sammccall added a comment.
Some notes before our meeting.
It does appear that it's possible to generate duplicate forest nodes in this
way, and AFAIK any method other that explicitly deduplicating creating using a
map<(rule, rhsnodes), sequencenode> is going to have this problem.
The good news
sammccall added inline comments.
Comment at: clang/lib/Tooling/Syntax/Pseudo/GLRParser.cpp:285
+ForestChildren.push_back(PN->Parsed);
+ const ForestNode &ForestNode = ParsedForest.createSequence(
+ ReduceSymbolID, RA.ReduceRuleID, ForestChildren.front()->sta
sammccall added a comment.
A bunch of comments, but probably the most important one is that the inner loop
feels uncomfortably chaotic:
- there's a huge function with a lot of stuff in scope and referenced, it's
unclear what the data flow is
- there are very many (hopefully, unneccesarily many)
alextsao1999 added inline comments.
Comment at: clang/lib/Tooling/Syntax/Pseudo/GLRParser.cpp:318
Bases.push_back(
-{Base, ParsingTable.getGoToState(Base->State, ReduceRule.Target)});
+{Base, ParsingTable.getGoToState(Base->State, ReduceSymbolID)}
hokein added a comment.
The implementation (`performReduction`) is awkward at the moment, but it is
completed, should give a global view of the algorithm.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121368/new/
https://reviews.llvm.org/D121368
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang.
Forest node by design is unmutable. To create an ambiguous node, we have
to know all alte
