[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

aaron.ballman added inline comments. Comment at: clang/lib/Tooling/Syntax/Pseudo/LRTable.cpp:119 +++End; + return llvm::makeArrayRef(&Actions[Start], &Actions[End]); +} hokein wrote: > aaron.ballman wrote: > > This is causing an assertion with debug builds o

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein added inline comments. Comment at: clang/lib/Tooling/Syntax/Pseudo/LRTable.cpp:119 +++End; + return llvm::makeArrayRef(&Actions[Start], &Actions[End]); +} aaron.ballman wrote: > This is causing an assertion with debug builds on Windows because > `Act

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

aaron.ballman added a comment. In D118196#3341159 , @hokein wrote: > In D118196#3341110 , @aaron.ballman > wrote: > >> Hi, this commit is causing runtime failures on Windows in debug builds. Can >> you please co

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein added a comment. In D118196#3341110 , @aaron.ballman wrote: > Hi, this commit is causing runtime failures on Windows in debug builds. Can > you please correct or revert? Thanks! sorry for the trouble. I'm mostly running out of time today, I will

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

aaron.ballman added a comment. Hi, this commit is causing runtime failures on Windows in debug builds. Can you please correct or revert? Thanks! Comment at: clang/lib/Tooling/Syntax/Pseudo/LRTable.cpp:119 +++End; + return llvm::makeArrayRef(&Actions[Start], &Actions[End])

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa2fab82f33bb: [pseudo] Implement LRTable. (authored by hokein). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein updated this revision to Diff 410725. hokein marked 3 inline comments as done. hokein added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118196/new/ https://reviews.llvm.org/D118196 Files: clang/include/clang/

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks, I really like the way the tests look now! Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:78 + Shift, + // Reduce by a rule, the value

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:177 }; +void initTerminals(std::vector &Terminals); sammccall wrote: > why is this exposed/required rather than being initialized by the > GrammarTable constructor?

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein updated this revision to Diff 409893. hokein added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118196/new/ https://reviews.llvm.org/D118196 Files: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h clang/include/cla

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein updated this revision to Diff 409892. hokein marked 14 inline comments as done. hokein added a comment. - address comments - more api polishments - LRGraph unittest => lit tests - hide LRTable::Builder, move to LRTableBuild.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

sammccall added a comment. This is really nice, mostly nits. Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:177 }; +void initTerminals(std::vector &Terminals); why is this exposed/required rather than being initialized by the GrammarTable c

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:84 + +Kind K = Kind::Error; +// Value sammccall wrote: > This action struct can be squeezed to 16 bits. > > Kind only needs 3 bits (I suspect it can be 2: Error

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein updated this revision to Diff 407862. hokein marked 10 inline comments as done. hokein added a comment. Herald added a subscriber: mgrang. - rebase, rescope the patch to LRTable - refine the Action class interfaces, and nameing; - use a more compact Index, reduce LRTable from 660KB => 335KB

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein marked 11 inline comments as done. hokein added a comment. comments around LRAutomaton should be addressed in the separate patch, https://reviews.llvm.org/D119172. Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRAutomaton.h:32 +public: + Item(RuleID ID, uint8_t

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein marked 7 inline comments as done. hokein added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:130 + + // Returns the SymbolID of the augmented symbol '_' + SymbolID augmentedSymbol() const { return 0; } sammccall wrote:

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

sammccall added a comment. OK, I think I understand it now! Sorry for so many comments, happy to chat offline about which ones to address/where to start. Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:39 +public: + struct Action { +enum class Kind : uint8

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

sammccall added a comment. OK, a bunch of random notes but I'm getting a higher level picture. I'm not sure about the split between LRAutomaton and LRTable. I suppose doing it in two stages simplifies the implementation, but I don't think the first stage particularly needs to be public. Also I

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

sammccall added a comment. A few initial comments just on the "easy parts" - changes to grammar and the first/follow set computation. I need to study the rest. Generally I have a feeling some of the impl is very inefficient but efficiency may not matter much in this context, and it's unclear h

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein updated this revision to Diff 404442. hokein added a comment. Polish the LRTable implementation, using two flat vectors memory-size (bytes): 1380908 => 664600 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118196/new/ https://reviews.llvm.or

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

hokein created this revision. hokein added a reviewer: sammccall. Herald added a subscriber: mgorny. hokein requested review of this revision. Herald added a project: clang. This patch introduces a dense implementation of the LR parsing table, which is used by LR parsers. We implement a SLR(1) par