Fix parsing of new and refactor parsing of with and other things
#313
Conversation
wow, i thought in other classic languages like C++ and Java they require explicit parentheses here |
|
Wow are you new to programming? 😛 |
FlandiaYingman
left a comment
FlandiaYingman
left a comment
There was a problem hiding this comment.
Wow amazing. I just have some questions about the new new parsing strategy
|
|
||
| def isModuleModifier: Bool = this match | ||
| case Tree.TypeDef(Mod, _, N, N) => true | ||
| case td @ Tree.TypeDef(Mod, _, rhs) => rhs.isEmpty && td.extension.isEmpty && td.withPart.isEmpty |
There was a problem hiding this comment.
Why not
| case td @ Tree.TypeDef(Mod, _, rhs) => rhs.isEmpty && td.extension.isEmpty && td.withPart.isEmpty | |
| case td @ Tree.TypeDef(Mod, _, N) => td.extension.isEmpty && td.withPart.isEmpty |
| //│ ═══[WARNING] Modules are not yet lifted. | ||
| //│ ╔══[COMPILATION ERROR] No definition found in scope for 'M' | ||
| //│ ║ l.154: module M with | ||
| //│ ╙── ^ | ||
| //│ ║ ^^^^^^ | ||
| //│ ║ l.155: fun foo() = | ||
| //│ ║ ^^^^^^^^^^^^^^^ | ||
| //│ ║ l.156: set y = 2 | ||
| //│ ║ ^^^^^^^^^^^^^^^ | ||
| //│ ║ l.157: x + y | ||
| //│ ╙── ^^^^^^^^^^^ |
There was a problem hiding this comment.
Do you think the location is too... long? 🤔
There was a problem hiding this comment.
Yeah, in other prototypes we had logic to abbreviate long locations. We should port that over at some point.
| // val td = DummyTypeDef(syntax.Cls) | ||
| val td = TypeDef(syntax.Cls, App(id, Tup(Ident("captures") :: Nil)), N) |
There was a problem hiding this comment.
| // val td = DummyTypeDef(syntax.Cls) | |
| val td = TypeDef(syntax.Cls, App(id, Tup(Ident("captures") :: Nil)), N) | |
| val td = TypeDef(syntax.Cls, App(id, Tup(Ident("captures") :: Nil)), N) |
| // case New(c, rfto) => | ||
| // assert(rfto.isEmpty) | ||
| // Term.New(cls(subterm(c), inAppPrefix = inAppPrefix), params.map(subterm(_)), bodo).withLocOf(tree) | ||
| case ProperNew(body, rfto) => // TODO handle Under |
There was a problem hiding this comment.
| // case New(c, rfto) => | |
| // assert(rfto.isEmpty) | |
| // Term.New(cls(subterm(c), inAppPrefix = inAppPrefix), params.map(subterm(_)), bodo).withLocOf(tree) | |
| case ProperNew(body, rfto) => // TODO handle Under | |
| case ProperNew(body, rfto) => // TODO handle Under |
There was a problem hiding this comment.
Actually I think this is good now:
| // case New(c, rfto) => | |
| // assert(rfto.isEmpty) | |
| // Term.New(cls(subterm(c), inAppPrefix = inAppPrefix), params.map(subterm(_)), bodo).withLocOf(tree) | |
| case ProperNew(body, rfto) => // TODO handle Under | |
| case ProperNew(body, rfto) => |
| // * `new` is a strange keyword: | ||
| // * it has a very high precedence that sits between that of selection and that of application. | ||
| // * Indeed, `new Foo().bar` should parse as `(new Foo()).bar`, not `new (Foo().bar)`, | ||
| // * but `new Foo.Bar` should parse as `new (Foo.Bar)`. | ||
| val newRightPrec = S(maxPrec.get + charPrecList.length - 1) | ||
| // * ^ maxPrec.get + charPrecList.length is the precedence of selection | ||
| val `new` = Keyword("new", N, newRightPrec) |
There was a problem hiding this comment.
Wow, how does this mysterious precedence definition make it possible to parse new before application but after selection 🤔
There was a problem hiding this comment.
This logic could be improved for clarity, for sure.
Chec out how charPrecList reserves a slot for new that sits between the slot for "." and that for applications.
2 participants
newis a strange keyword: it has a very high precedence that sits between that of selection and that of application. Indeed,new Foo().barshould parse as(new Foo()).bar, notnew (Foo().bar), butnew Foo.Barshould parse asnew (Foo.Bar).I just wanted to fix the precedence of
new, but this lead to a formidable yak shave. Notably ended up having to makewithparse as an infix keyword (because of thenew ... with ...syntax and the incomparable precedences) and having to temporarily refactor theconstructordesugaring hack (which will be changed again by #309).