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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
