fix for Regression that the introduction of BackwardsIndex introduced

[krux02]

Before the introduction of BackwardsIndex the following code worked. BackwardsIndex broke it and with this PR it works again:

type
  MyType = object

proc `[]`(arg: MyType; idx: int): int =
  idx

proc len(arg: MyType): int = 100

var mt: MyType
echo mt[^1] # 99

The idea is: When I have a type that supports both len and `[]`, then backwards indexing should just work out of the box. It did work, and now it works again. And while I was fixing this, it just happened to also allow for custom BackwardsIndex conversion functions:

type
  MyType = object

template `^^`(a: MyType, b: BackwardsIndex): int =
  ## my conversion of `BackwardsIndex` for `MyType`
  1000 + int(b)

proc `[]`(arg: MyType; idx: int): int =
  idx

var mt: MyType
echo mt[^1] # 1001

As you can see, I do not overload len for MyType here. And it just works out of the box. While it is still possible to overload the [] operator for BackwardsIndex to support it, it is not needed anymore.

And last but not least. The code in system.nim is shorter now.

[Araq]

Brilliant solution, fails for

• tests/compiles/trecursive_generic_in_compiles.nim
• tests/array/troof3.nim
• tests/misc/tslices.nim

[krux02]

that's what the tests are for, aren't they. I will see what I can do.

[krux02]

Well I would like to comment on your snorky remark. I think your sarcasm is inapropriate. I try to fix a bug and I did not get it right the first time. That is what test are there for. I could also have reacted the same way when the BackwardsIndex was introduced, but I did not.

But thanks for pointing out the tests that failed. Actually digging though the logs of Travis and AppVeyor is very tedious, because it is so slow.

And last but not least, I would like to write a comment on the troof3.nim test. My question is, why should it work? An array with enums as index don't even support a forward index, so why then should it support a backwards index?

var a: array['a'..'c', string] = ["a", "b", "c"]

echo a[0]      # <- this isn't allowed
echo a[^1]   # So then why should this be allowedS It doesn't make a lot of sense to me.

[krux02]

And on the test trecursive_generic_in_compiles.nim. I don't understand the error message.

this is the line that failes:

let lst = lc[$x | (x <- 'a'..'z'), string].asList

And this is the error message I get:

trecursive_generic_in_compiles.nim(95, 21) Error: undeclared identifier: '|'

I don't understand how that is related to the changes I made.

[Araq]

Well I would like to comment on your snorky remark. I think your sarcasm is inapropriate.

No sarcasm involved here! I really like this solution! :-) Sorry.

[Araq]

Once it works we need to decide if ^^ should really be the symbol to use. Previously it wasn't exported and I wanted a binary operator for my personal readability.

[Araq]

And last but not least, I would like to write a comment on the troof3.nim test. My question is, why should it work? An array with enums as index don't even support a forward index, so why then should it support a backwards index?

Good point, I agree. Well, apparently it used to work and ^1 means the "last element" regardless of the involved indexing type. Can be useful for generic code, no?

[krux02]

I write an entry in the changelog, that backwards index for non-int indexed types does not compile anymore. And I remove the test.

[krux02]

@Araq: No you can't really use it for generic programming either, because there is no generic way to access the first element.

[Araq]

Since this breaks tmapconcept.nim, I delay this PR until further concept-related bugs are fixed. Sorry.

[ghost]

As example here is a data structure that won't work with these changes.

[krux02]

@NotTito: It was very confusing to me that you used head and tail with that semantic. normally head is the first element of a list and tail everything but the first element. In your case both were indices. I think getIndex and putIndex would have been better names. But I will now take a deeper look into it.

[krux02]

@NotTito I just tried moved the backwards index to the newest version of Nim and tested it with your example. There was a bug in line 53 where a normal index and a backwards index get subtracted. That operator is not defined. But when line 52 and 53 are removed, the automatically generated backwards index will work just fine. I tested it.

[krux02]

well there is nothing I can do with the concepts part. It seems like the concepts are broken, because my changes should not affect concepts at all.

[Araq]

well there is nothing I can do with the concepts part. It seems like the concepts are broken, because my changes should not affect concepts at all.

Unfortunately, yes. :-(

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.