[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 9 02:19:56 PDT 2022


hokein 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) {
----------------
sammccall wrote:
> 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
Fair enough. I underestimated the effort of doing this stuff (we need to adjust a few existing tests in GrammarTests as well).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125677/new/

https://reviews.llvm.org/D125677



More information about the cfe-commits mailing list