[PATCH] D127448: [wip][pseudo] Implement guard extension.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 1 06:46:32 PDT 2022

hokein added inline comments.

Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:298
   auto Pop = [&](const GSS::Node *Head, RuleID RID) {
-    LLVM_DEBUG(llvm::dbgs() << "  Pop " << Params.G.dumpRule(RID) << "\n");
-    const auto &Rule = Params.G.lookupRule(RID);
+    LLVM_DEBUG(llvm::dbgs() << "  Pop " << Params.Lang.G.dumpRule(RID) << "\n");
+    const auto &Rule = Params.Lang.G.lookupRule(RID);
sammccall wrote:
> I'm not sure exactly what the best thing to do about this is, but `Params.Lang.G` is hard to read - it's a long complicated name that doesn't even name the thing.
> I think probably we should split ParseParams up instead of nesting ParseLang in it, e.g.
> ```
> struct GLRStorage { ForestArena&, GSS }
> glrReduce(vector<ParseStep>, const ParseLang&, GLStorage&, NewHeadCallback)
> ```
> Or even put a forest pointer in the GSS just for convenience.
Changed the signature of glrShift/Reduce/Parse.

Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:306
+          auto It = Params.Lang.Guards.find(Rule.Guard);
+          assert(It != Params.Lang.Guards.end() && "missing guard!");
+          if (!It->getSecond()(TempSequence, Tokens))
sammccall wrote:
> I think diagnosing missing guards but then treating them as always-passing is less surprising and more ergonomic while modifying a grammar
I wanted to make this as a contract  (guard in each rule must refer to a valid extension function) in GLR parser.

I think loosing the restriction is also fine -- the only downside I can see is that if we have a typo guard in the grammar file, which we don't have a corresponding function, the parser will run and emit confusing results. We can fix it by adding "missing guard function" diagnostics.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list