icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Spans sometimes start a line above their expected position #603

Closed externl closed 1 year ago

externl commented 1 year ago

The issue is this:

[ab::bar("s")]
enum Foo : uint8 {}

The enum's span here starts at the "last character" of line 1 (col 15in this case), instead of the first character of line 2. This is an issue before my PR. We end up with an extra line being printed without anything to underline.

_Originally posted by @externl in https://github.com/icerpc/slicec/pull/602#discussion_r1218140882_

error [E010]: invalid enum 'E': enums must contain at least one enumerator
 --> foo.slice:3:15
  |
3 | [ab::bar("s")]
  |
4 | enum E : uint8 {
  | ------
  |

Failed: Compilation failed with 1 error(s)
externl commented 1 year ago

Note that to see "correct" better behavior, just remove the attribute.

InsertCreativityHere commented 1 year ago

It seems like it has to do with optional tokens not being present. For enums, this is the unchecked keyword:

[ab::bar("s")]
enum Foo : uint8 {}    // Span wrongly starts right after `]`

[ab::bar("s")]
unchecked enum Foo : uint8 {}    // Span correctly starts right before `unchecked`.

And for something that doesn't have any optional tokens, it always works fine:

[ab::bar("s")]
interface Foo {}    // Span correctly starts right before `interface`.
externl commented 1 year ago

What about:

❯ cat foo.slice
module Foo

enum E: uint8 {}
error [E010]: invalid enum 'E': enums must contain at least one enumerator
 --> foo.slice:3:1
  |
3 | enum E: uint8 {}
  | ------
  |

Failed: Compilation failed with 1 error(s)

No attribute and it's location is correct.

InsertCreativityHere commented 1 year ago

Because enum is the very first token it sees. The rule it's matching here is:

<p: Prelude> <l: @L> <uk: unchecked_keyword?> enum_keyword

Where Prelude is optional comments and attributes and stuff.

So if there's no comments, attributes, or unchecked, the first location it will get is for the enum keyword, so it uses that one. Maybe. Still looking at it.