sammccall accepted this revision. sammccall added inline comments.
================ Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:86 + const ForestNode *Result = nullptr; + StateID AcceptState = Params.Table.getGoToState(StartState, StartSymbol); + glrReduce(PendingReduce, Params, [&](const GSS::Node *NewHead) { ---------------- hokein wrote: > sammccall wrote: > > this line introduces a requirement that the start symbol be a nonterminal - > > if this is new, can we doc it? > this is not a new requirement, it is rather implicit previously (we use > `G.findNonterminal` to pass the start-symbol argument of the `glrParse`). I > added an assertion in the glrParse. > > It would be nice to diagnose it in the bnf grammar parsing time (will add a > followup patch). FWIW I think an assertion (and possibly a comment) is the right tradeoff here, maintaining diagnostics for all this stuff seems like overkill Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125677/new/ https://reviews.llvm.org/D125677 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits