search

hadronized / glsl

GLSL parser for Rust
190 stars 28 forks source link

Better scheme for parenthesis generation #120

Closed alixinne closed 3 years ago

alixinne commented 4 years ago

Fixes #52

I tagged this PR as WIP until we're sure the failing tests come from #119 and there's enough coverage as well as docs (currently missing).

The main idea behind this is to use the precedence and associativity information from the GLSL language specification, section 5.1, to only generate parentheses if needed. There might be cases that need extra parentheses generation, which they didn't have before, and which are currently not covered by tests (I'm thinking assignement, ternary and comma most notably).

alixinne commented 4 years ago

This PR looks good on my side, aside from this one test case to fix I think all precedence/associativity cases are covered.

hadronized commented 4 years ago

Agreed. For the failing test, I think it shouldn’t be that hard to detect when parens are not needed but I’m not sure how we should do that elegantly.

hadronized commented 4 years ago

Also, it’s just a syntactic / comfort test. We could merge as is and comment that test until we have the code for it, I’m okay with it.

alixinne commented 3 years ago

@phaazon I rebased this branch onto the current master so it can now be merged.

alixinne commented 3 years ago

I changed the way precedence information is used for generating the output. Everything is now handled in the transpiler module, without any changes to the syntax module. This makes the interface cleaner and the change itself more atomic IMO.

hadronized commented 3 years ago

Sorry for the long delay. I’ll have a look at why it doesn’t build on macOS and try to merge it ASAP.

alixinne commented 3 years ago

I rebased the branch to the current state of the master branch, it seems the Mac OS failure was just the nested parentheses performance test which failed once.