fix: rework translation-unit top-level items to disallow binary expressions #146
Conversation
amaanq
changed the title
|
@amaanq no additional testcases, sorry, but this one should be sufficient. |
amaanq
force-pushed
the
decl-binop-conflict
branch
from
414cc4d
to
6a11836
Compare
|
@amaanq actually, I might have one - |
amaanq
force-pushed
the
decl-binop-conflict
branch
from
6a11836
to
0a76efd
Compare
|
That one seems to work on master, but good to add |
amaanq
force-pushed
the
decl-binop-conflict
branch
2 times, most recently
from
8a57c83
to
5897d88
Compare
| @@ -649,6 +680,36 @@ module.exports = grammar({ | |||
| $._statement, | |||
| ), | |||
|
|
|||
| // This is missing binary expressions, others were kept so that macro code can be parsed better and code examples | |||
There was a problem hiding this comment.
would be good to say why we skipped binary expressions. what's the problem with binary expression at the top?
There was a problem hiding this comment.
If it's because of this ambiguity with pointer expressions, could have been solved in another way? by playing with dynamic priorities?
There was a problem hiding this comment.
Yeah I did try a lot w/ dynamic precendences, but that caused other issues elsewhere mainly and sometimes didn't do anything as well
amaanq
force-pushed
the
decl-binop-conflict
branch
from
5897d88
to
b1b0fad
Compare
amaanq
force-pushed
the
decl-binop-conflict
branch
from
b1b0fad
to
1bb93aa
Compare
|
reworked |
|
I'm not sure about that one, I think it is correct to associate the pointer declarator with a function name/identifier |
This resolves conflicts with declarations that can be interpreted as binary_expressions
Previously, pointers were associated with the function identifier, now they're associated with the return type
amaanq
force-pushed
the
decl-binop-conflict
branch
from
1bb93aa
to
0df358a
Compare
|
I reworked it anyways just for top level functions..if that proves to be incorrect/disputed I will revert @XVilka |
|
Actually, that breaks error recovery a bit in upstream tests and still doesn't fit fully right w/ me, so I reverted it, sorry. That needs a better solution than what I had |
Closes #48
I don't like how this looks with the nearly repeated rule, but it's the best option I could think of that didn't impact state count too much, maybe some JS-magic could make it look prettier with manipulating top_level_item
@XVilka if you have some more test cases I could add them
cc @ahlinc