[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

[Sam McCall via Phabricator via cfe-commits]

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