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