sammccall added inline comments.
================ Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule &R = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; ---------------- so the dollar signs are a practical problem: at least on my machine the warning is on by default, so we emit thousands of warnings and it's impossible to find the error among them. At *minimum* we need to use pragmas or something to suppress the warning. But using nonstandard C++ seems likely to cause other problems and limit portability for not-great reasons. Some ideas: -`Rule::ptr_declarator__EQUALS__ptr_operator__ptr_declarator` (this violates the internal `__` rule, though that one is widely ignored) - `Rule::ptr_declarator_0ptr_operator_1ptr_declarator` - `Rule::PtrDeclarator_EQ_PtrOperator_PtrDeclarator` (yet another spelling variation, but quite readable. We could even change the spellings in the BNF file...) ================ Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Language.h:29 // Return true if the guard is satisfied. using RuleGuard = llvm::function_ref<bool( + RuleID RID, llvm::ArrayRef<const ForestNode *> RHS, const TokenStream &)>; ---------------- sammccall wrote: > This allows/encourages guards that dynamically consider multiple rules, which > increases the amount of coupling between rules and grammar. > > Can we express this as > function-declarator := declarator [guard=FunctionDeclarator]? > > This does mangle the grammar a bit to our implementation, and introduces two > names for the same concept (guard & nonterminal). > If this feels ugly and redundant, another option would be to change the guard > to be specified by the rule ID instead of an extension ID: > ``` > function-declarator := declarator [guard] > > DenseMap<RuleID, RuleGuard> Guards; > ``` Having played with this idea a bit, I do think we should strongly consider dispatching on the rule ID rather than a named extension. I went to add some more guards (on NUMERIC_CONSTANT etc) and they really are 1:1 with rules. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.llvm.org/D129222 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
