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

[Haojian Wu via Phabricator via cfe-commits]

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127448