[PATCH] D125006: [pseudo] Support parsing variant target symbols.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.


================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Grammar.h:188
+// Name must be a valid nonterminal symbol name in the grammar.
+SymbolID findNonterminal(llvm::StringRef Name,
+                         const clang::pseudo::GrammarTable &GT);
----------------
hokein wrote:
> it is unfortunate that we put this function in the Grammar.h. This could be avoided when we have the generated enum grammar type, but we are not there yet...
This doesn't seem so bad to me, but I'd prefer it to be a method on Grammar I think - callers like the ones in the benchmark shouldn't have to worry about the GrammarTable object.


================
Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:118
+          *ParseableStream, clang::pseudo::ParseParams{*G, LRTable, Arena, GSS},
+          clang::pseudo::findNonterminal(StartSymbol, G->table()));
       if (PrintForest)
----------------
this means if you pass --start-symbol=typo we'll hit an assertion

I think findNonterminal should probably return Optional instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125006