bugfix: require relative names in import{Wordy,Symboly}Id #5568
Conversation
mitchellwrosen
changed the title
| I got confused here: | ||
|
|
||
| 3 | use Nat .+ | ||
|
|
||
|
|
||
| I was surprised to find a '.' here. | ||
| I was expecting one of these instead: | ||
|
|
||
| * bang | ||
| * binding | ||
| * do | ||
| * false | ||
| * force | ||
| * handle | ||
| * if | ||
| * lambda | ||
| * let | ||
| * newline or semicolon | ||
| * pattern | ||
| * quote | ||
| * termLink | ||
| * true | ||
| * tuple | ||
| * typeLink |
There was a problem hiding this comment.
Side note, I'm not sure which one of these it wants.
There was a problem hiding this comment.
Yeah I know, the error messages are atrocious :|
| I was expecting something after the use keyword | ||
|
|
||
| 3 | use .lib.builtin.Nat + | ||
|
|
||
| Here's a few examples of valid `use` statements: | ||
|
|
||
| use math sqrt Introduces `sqrt` as a local alias for | ||
| `math.sqrt` | ||
| use List :+ Introduces `:+` as a local alias for | ||
| `List.:+`. | ||
| use .foo bar.baz Introduces `bar.baz` as a local alias for | ||
| the absolute name `.foo.bar.baz` |
There was a problem hiding this comment.
Do we not want to allow absolute names for the first one? The example on line 65 suggests it as correct syntax.
There was a problem hiding this comment.
I don't see a problem with allowing absolute names in certain places, I can make that change. My instinct was to simplify everything down to relative names, but that could be wrong.
|
Tentatively approved and seems okay for now, but I could see wanting to distinguish with absolute names in Can you also address the second message I commented on, that suggests the now-unsupported syntax? |
|
Conclusion: will revert part of this to allow |
|
For reference the current user docs on |
|
Ok, this is ready to go. The macOS CI errors look superfluous: |
Overview
This PR fixes #4536 by changing this from a runtime crash to a parse error:
During implementation, I surveyed other uses of the combinator that parsed ".bar" here (where we want to only allow relative "bar"), and determined that none of them need to allow both absolute and relative names, so for consistency and simplicity, I switched them all to require relative names, even when an absolute name has a sensible interpretation (e.g. "namespace .blah")
The full list of now-disallowed absolute dots (also see transcript where all of these are captured):
Test coverage
PR includes transcript