Nim: proc `[]`(s: string; i: BackwardsIndex): char {...} doesn't return var char

[geekrelief]

Summary

Like
proc []T: var T {...}
proc []Idx, T: var T {...}

can
proc [](s: string; i: BackwardsIndex): char {...} return var char?

Description

I was trying to get this to work doing exercisim.io

func reverse*(a: string):string =
  result = a
  for i in 0 ..< a.len div 2:
    #swap(result[i], result[^(i+1)]) # result[BackwardsIndex(i+1)] is immutable, not var!
    swap(result[i], result[a.high - i]) # had to do this

[Yardanico]

func reverse*(a: string):string =
  result = a
  for i in 0 ..< a.len div 2:
    #swap(result[i], result[^(i+1)]) # result[BackwardsIndex(i+1)] is immutable, not var!
    swap(result[i], result[a.high - i]) # had to do this


echo reverse("hello")

works fine for me on devel and on 1.2.0

[solo989]

All these procs should be probably replaced with templates. So
proc [](s: string; i: BackwardsIndex): char {.inline.} =
becomes
template [](s: string; i: BackwardsIndex): char
whose body already calls
template []*(s: string; i: int): char = arrGet(s, i)

[geekrelief]

@Yardanico Just to be clear it's:

func reverse*(a: string):string =
  result = a
  for i in 0 ..< a.len div 2:
    swap(result[i], result[^(i+1)]) # result[BackwardsIndex(i+1)] is immutable, not var!

That causes the error.

[Yardanico]

@geekrelief I understood that and that's why I hid my comment :)

[timotheecour]

I still think BackwardsIndex is.... a backwards idea and D's opDollar is preferable.
Simple math: it reduces number of overloads needed; and it's also strictly more flexible.

[Araq]

Oh not that again... we used to do D's opDollar idea. It was much worse than BackwardsIndex. Special case in the language, a terrible mess to support in the compiler.

[solo989]

Let's not derail this conversation.
Converting the procs into templates would fix the issue.
I would make a pull request but everytime I do it just clutters up my request with old commits.

[Yardanico]

@solo989 it's not an issue since people can just squash merge your PR

[solo989]

Ok I reopened my pull request.

[Araq]

Turning it into templates re-introduces the "Multi evaluation" problem.

[mratsim]

The correct solution is to implement

proc `[]`(s: var string; i: BackwardsIndex): var char

to accompany

proc `[]`(s: string; i: BackwardsIndex): char

Not use template and hide mutability

[solo989]

If someone else wants to take a crack at fixing this problem, go ahead. My solution didn't seem to work as Backwards Index is a bit buggy. It would work with a template but that introduces double evaluation bugs.