[pseudo] Reorganize CXX.h enums

[sammccall]

Summary

• Place rules under rule::lhs::rhsrhsrhs
• Change mangling of keywords to ALL_CAPS (needed to turn keywords that appear alone on RHS into valid identifiers)
• Make enums implicitly convertible to underlying type (though still scoped, using alias tricks)

In principle this lets us exhaustively write a switch over all rules of a NT:

switch ((rule::declarator)N->rule()) {
  case rule::declarator::noptr_declarator:
    ...
}

In practice we don't do this anywhere yet as we're often switching over multiple
nonterminal kinds at once.

[hokein]

Thanks for exploring the idea, this looks like a good start to me.

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

54

why there is a dummy namespace here?

clang-tools-extra/pseudo/lib/cxx/CXX.cpp

121–122

The code of applying the pattern doesn't look much worse to me and it is easier to verify the exhaustiveness by human as well. We might loose some performance (I hope not a lot), but I think it is a good tradeoff (not saying we need do it in this patch).

switch (LHSNonterminal(Declarator->rule(), *G)) {
   case cxx::Symbol::noptr_declarator: {
      switch ((rule::noptr_declarator)Declarator->rule())  {
         case rule::noptr_declarator::declarator_id:
             ....
         case rule::noptr_declarator::noptr_declarator__parameters_and_qualifiers:
             ....
      }
   ...
   }
}

276

A nice improvement

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp

47–48

IMO, this improves the readability, and also aligns with the text in the cxx.grammar.

47–48

nit: we don't allow nullable nonterminals, so the RHS should never be empty. we could just initialize MangleName with mangleSymbol(R.seq()[0]), and iterate from 1 (then the trailing two pop_back is not needed).

47–48

nit: update the doc comment in .h file.

48

ok, now we're dropping the index for all RHS symbols. Just want to know your thought about this. Personally, I'd like to keep this information (noptr_declarator__l_square__constant_expression__r_square vs noptr_declarator0_l_square1_constant_expression2_r_square3) though it makes the name a bit uglier.

Change mangling of keywords to ALL_CAPS (needed to turn keywords that appear alone on RHS into valid identifiers)

if we add index number to the name, then this change is not required.

[sammccall]

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

54

for each NT, we close the previous ns+enum and open new ones.
For this to work for the first NT, we have to open an ns+enum.

I can add a comment, but would prefer to do this some other way?

clang-tools-extra/pseudo/lib/cxx/CXX.cpp

121–122

I guess this is a question of taste, but I find that style very hard to read.

(Note that it's incorrectly indented, and the indentation rules around switch/case are one of the main reasons I find nested switches confusing)

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp

47–48

Thanks. I like this change too. We still have semi vs ; (should we use SEMI?) but it feels clearer

47–48

Honestly I would rather just move the impl back into gen - this change seems to demonstrate why

48

Short version: I can deal with the numbers at the front (there's a *little* bit of value), but at the back of the symbol name there's no value, just noise. I find the double-underscore version much more readable (than either variant with numbers).

---

I always found the indexes aesthetically ugly but OK when you could read them as

lhs _   0 rhsa_ 1 rhsb
lhs := [0]rhsa [1]rhsb

But an identifier can't start with a number (rule::empty_statement::0semi) this isn't possible. I think you suggested shifting the numbers to the end, but I can't find a way to read that, it adds too much visual noise. (e.g. because the number "decorations" don't line up in a column in code completion or a switch statement).

I also think the double-underscore version is significantly less cryptic for new readers.

[usaxena95]

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

54–62

I would include this block in the clang-format off block to show these are for the generated code.

//clang-format off
namespace dummy { enum Rule {
...
};}
//clang-format on

[sammccall]

tweak formatting

[hokein]

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

54

ah, I get it (until I read the preprocessed CXX.h code) -- I found it really hard to understand it by literally reading the code here.

To make it clear, I think we can probably add two additional RULE_BEGIN, RULE_END macros?

in CXXSymbols.inc, we emit something like

RULE_BEGIN(_)
RULE(_, translation_unit, 135)
RULE(_, statement_seq, 136)
RULE(_, declaration_seq, 137))
RULE_END(_)

In CXX.h, we write

#define RULE_BEGIN(LHS) namespace LHS { enum Rule : RULE ID {
#define RULE_END()  }; }

clang-tools-extra/pseudo/lib/cxx/CXX.cpp

121–122

(Note that it's incorrectly indented, and the indentation rules around switch/case are one of the main reasons I find nested switches confusing)

Ah the indentation is weird (it is not what I originally written..). There are ways to make the indentation correct (turn off the clang-format etc).

If nested switch is hard to read, maybe we can wrap one with a macro to improve the code readability (happy to explore ideas, I think there is value on this pattern).

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp

47–48

yeah, that looks good (that means lowercase is always referring to nonterminals)

47–48

ok, feel free to move it.

48

I find the double-underscore version much more readable (than either variant with numbers)

I agree that the double-underscore is more readable, but I don't have a much-better feeling. I'm ok with the double-underscore one if you feel strong about it.

The reason why I value the index is that it is easier to spot the index and its corresponding RHS element, and it could potentially avoid writing bugs (specially in the guard function implementation,
thinking about accessing the RHS element in the looong rule l_paren__r_paren__decl_specifier_seq__noexcept_specifier__trailing_return_type__requires_clause).

[sammccall]

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

54

That would make the callsite more clear, but at the cost of adding ad-hoc bits to the CXXSymbols.inc.
I'd prefer to keep it generic if we can (otherwise we might as well just have Gen generate the enum definitions directly, right?)

Added a comment to explain the dummy.

54–62

Oops, they're not for the generated code, just for the macro definition (clang-format gets very confused)

Restricted the scope of the formatting-disabled block and tried to improve the hand-formatting (though I don't think any formatting of this particular preprocessor trick is very readable)

clang-tools-extra/pseudo/lib/cxx/CXX.cpp

121–122

Maybe - I think nonstandard formatting and macros cost a lot of readability in their own right.

I think we should probably discuss this separately - I think this change is worth having even if we don't change control flow to exploit in in all cases, WDYT?

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp

47–48

I go back and forth: I think the use-mention distinction is important to readability too:

• initializer_list means an initializer list goes here
• INT does *not* mean an integer goes here, but rather the *word* "int"
• by this test, comma should be lowercase, because it's a comma, not the word "comma" that goes here

(Now I'm actually wondering if we should go back and lowercase IDENTIFIER, NUMERIC_CONSTANT etc in the bnf file...)

In any case, let's uppercase SEMI for now because anything else leaves ugly inconsistency somewhere.

48

The reason why I value the index is that it is easier to spot the index and its corresponding RHS element

Right, I agree, but at least for me this benefit vanishes entirely if the index has to come at the end of the name instead of the start.

I don't feel strongly about double-underscore over 0foo_1bar (which isn't a valid identifier), but I do feel strongly about it over foo0_bar1.

Losing the indices is a tradeoff, but I don't feel like it's terrible (it's bad in the example you gave, but it's not the common case: median RHS tokens is 1, the average is 2, and 85% have <=3)

[sammccall]

address comments

[hokein]

I think this patch is in a good state.

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

54

That would make the callsite more clear, but at the cost of adding ad-hoc bits to the CXXSymbols.inc.

I think this is the point. We read this cxx.h more often than the CXXSymbols.inc, this seems to be a right tradeoff to me.

I'd prefer to keep it generic if we can (otherwise we might as well just have Gen generate the enum definitions directly, right?)

And yeah, generating the whole definitions from the Gen tool is another option.

OK, if you prefer to do it in this way, could you at least rename the enum Rule { to something like enum PlaceHolder {? It is very confusing to use the meaningful name Rule for this dummy enum.

clang-tools-extra/pseudo/lib/cxx/CXX.cpp

121–122

yeah, it should not block this change.

clang-tools-extra/pseudo/unittests/GrammarTest.cpp

112

nit: I think the test is not valid now because we don't have mangleRule in the Grammar.

[dyung]

Hi @sammccall, your change seems to be causing a build failure on the PS4 linux bot, can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/25678

FAILED: tools/clang/tools/extra/pseudo/gen/CMakeFiles/clang-pseudo-gen.dir/Main.cpp.o 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/g++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/pseudo/gen -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/pseudo/gen -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/include -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/include -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/include -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/include -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/pseudo/include -I/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/clang/tools/extra/pseudo/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/clang/tools/extra/pseudo/gen/CMakeFiles/clang-pseudo-gen.dir/Main.cpp.o -MF tools/clang/tools/extra/pseudo/gen/CMakeFiles/clang-pseudo-gen.dir/Main.cpp.o.d -o tools/clang/tools/extra/pseudo/gen/CMakeFiles/clang-pseudo-gen.dir/Main.cpp.o -c /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/pseudo/gen/Main.cpp
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/pseudo/gen/Main.cpp: In function ‘std::string clang::pseudo::{anonymous}::mangleSymbol(clang::pseudo::SymbolID, const clang::pseudo::Grammar&)’:
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/pseudo/gen/Main.cpp:73:50: error: expected primary-expression before ‘]’ token
   73 |   static std::string *TokNames = new std::string[]{
      |                                                  ^
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang-tools-extra/pseudo/gen/Main.cpp:77:7: error: too many initializers for ‘std::string [1]’ {aka ‘std::__cxx11::basic_string<char> [1]’}
   77 |       };
      |       ^

[sammccall]

Sorry Doug!
7b70c2e75cd6b21cf9c2ea2f2ce5f7b9c4202549 should fix, not sure why my local
clang accepts it.

clang-tools-extra/pseudo/gen/Main.cpp

if (std::error_code EC = Text.getError()) {
     llvm::errs() << "Error: can't read grammar file '" << Path
                  << "': " << EC.message() << "\n";
     ::exit(1);
   }
   return Text.get()->getBuffer().str();
 }
 } // namespace
 
+namespace clang {
+namespace pseudo {
+namespace {
+
+// Mangles a symbol name into a valid identifier.
+//
+// These follow names in the grammar fairly closely:
+//   nonterminal: `ptr-declartor` becomes `ptr_declarator`;
+//   punctuator: `,` becomes `COMMA`;
+//   keyword: `INT` becomes `INT`;
+//   terminal: `IDENTIFIER` becomes `IDENTIFIER`;
+std::string mangleSymbol(SymbolID SID, const Grammar &G) {
+  static std::string *TokNames = new std::string[]{
+#define TOK(X) llvm::StringRef(#X).upper(),
+#define KEYWORD(Keyword, Condition) llvm::StringRef(#Keyword).upper(),
+#include "clang/Basic/TokenKinds.def"
+      };
+  if (isToken(SID))
+    return TokNames[symbolToToken(SID)];
+  std::string Name = G.symbolName(SID).str();
+  // translation-unit -> translation_unit
+  std::replace(Name.begin(), Name.end(), '-', '_');
+  return Name;
+}
+
+// Mangles the RHS of a rule definition into a valid identifier.
+// 
+// These are unique only for a fixed LHS.
+// e.g. for the grammar rule `ptr-declarator := ptr-operator ptr-declarator`,
+// it is `ptr_operator__ptr_declarator`.
+std::string mangleRule(RuleID RID, const Grammar &G) {
+  const auto &R = G.lookupRule(RID);
+  std::string MangleName = mangleSymbol(R.seq().front(), G);
+  for (SymbolID S : R.seq().drop_front()) {
+    MangleName.append("__");
+    MangleName.append(mangleSymbol(S, G));
+  }
+  return MangleName;
+}
+
+} // namespace
+} // namespace pseudo
+} // namespace clang
+
 int main(int argc, char *argv[]) {
   llvm::cl::ParseCommandLineOptions(argc, argv, "");
 
   std::string GrammarText = readOrDie(Grammar);
   std::vector<std::string> Diags;
   auto G = clang::pseudo::Grammar::parseBNF(GrammarText, Diags);
 
   if (!Diags.empty()) {
     llvm::errs() << llvm::join(Diags, "\n");
     return 1;
   }
 
   std::error_code EC;
   llvm::ToolOutputFile Out{OutputFilename, EC, llvm::sys::fs::OF_None};
   if (EC) {
     llvm::errs() << EC.message() << '\n';
     return 1;
   }
 
   switch (Emit) {
   case EmitSymbolList:
     Out.os() << R"cpp(
 #ifndef NONTERMINAL
-#define NONTERMINAL(X, Y)
+#define NONTERMINAL(NAME, ID)
 #endif
 #ifndef RULE
-#define RULE(X, Y)
+#define RULE(LHS, RHS, ID)
 #endif
 #ifndef EXTENSION
-#define EXTENSION(X, Y)
+#define EXTENSION(NAME, ID)
 #endif
 )cpp";
     for (clang::pseudo::SymbolID ID = 0; ID < G.table().Nonterminals.size();
-         ++ID)
-      Out.os() << llvm::formatv("NONTERMINAL({0}, {1})\n", G.mangleSymbol(ID),
-                                ID);
-    for (clang::pseudo::RuleID RID = 0; RID < G.table().Rules.size(); ++RID)
-      Out.os() << llvm::formatv("RULE({0}, {1})\n", G.mangleRule(RID), RID);
+         ++ID) {
+      Out.os() << llvm::formatv("NONTERMINAL({0}, {1})\n",
+                                clang::pseudo::mangleSymbol(ID, G), ID);
+      for (const clang::pseudo::Rule &R : G.rulesFor(ID)) {
+        clang::pseudo::RuleID RID = &R - G.table().Rules.data();
+        Out.os() << llvm::formatv("RULE({0}, {1}, {2})\n",
+                                  clang::pseudo::mangleSymbol(R.Target, G),
+                                  clang::pseudo::mangleRule(RID, G), RID);
+      }
+    }
     for (clang::pseudo::ExtensionID EID = 1 /*skip the sentinel 0 value*/;
          EID < G.table().AttributeValues.size(); ++EID) {
       llvm::StringRef Name = G.table().AttributeValues[EID];
       assert(!Name.empty());
       Out.os() << llvm::formatv("EXTENSION({0}, {1})\n", Name, EID);
     }
     Out.os() << R"cpp(
 #undef NONTERMINAL

clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h

 #define CLANG_PSEUDO_CXX_CXX_H
 
 #include "clang-pseudo/Language.h"
 #include "clang-pseudo/grammar/Grammar.h"
 
 namespace clang {
 namespace pseudo {
 namespace cxx {
-// Symbol represents nonterminal symbols in the C++ grammar.
-// It provides a simple uniform way to access a particular nonterminal.
-enum class Symbol : SymbolID {
+
+// We want enums to be scoped but implicitly convertible to RuleID etc.
+// So create regular (unscoped) enums inside subnamespaces of `detail`.
+// Then add aliases for them outside `detail`.
+namespace detail {
+namespace symbols {
+enum Symbol : SymbolID {
 #define NONTERMINAL(X, Y) X = Y,
 #include "CXXSymbols.inc"
 #undef NONTERMINAL
 };
+} // namespace symbols
 
-enum class Rule : RuleID {
-#define RULE(X, Y) X = Y,
+namespace extensions {
+enum Extension : ExtensionID {
+#define EXTENSION(X, Y) X = Y,
 #include "CXXSymbols.inc"
-#undef RULE
+#undef EXTENSION
 };
+} // namespace extensions
 
-enum class Extension : ExtensionID {
-#define EXTENSION(X, Y) X = Y,
+namespace rules {
+// For each symbol we close the last symbol's enum+namespace and open new ones.
+// We need a dummy namespace+enum so that this works for the first rule.
+namespace dummy {
+enum Dummy {
+//clang-format off
+#define NONTERMINAL(NAME, ID) \
+};                            \
+}                             \
+namespace NAME {              \
+enum Rule : RuleID {
+//clang-format on
+#define RULE(LHS, RHS, ID) RHS = ID,
 #include "CXXSymbols.inc"
-#undef EXTENSION
 };
+}
+} // namespace rules
+} // namespace detail
+
+// Symbol represents nonterminal symbols in the C++ grammar.
+// It provides a simple uniform way to access a particular nonterminal.
+using Symbol = detail::symbols::Symbol;
+
+using Extension = detail::extensions::Extension;
+
+namespace rule {
+#define NONTERMINAL(NAME, ID) using NAME = detail::rules::NAME::Rule;
+#include "CXXSymbols.inc"
+} // namespace rule
 
 // Returns the Language for the cxx.bnf grammar.
 const Language &getLanguage();
 
 } // namespace cxx
 
 } // namespace pseudo
 } // namespace clang
 
 #endif // CLANG_PSEUDO_CXX_CXX_H

clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h

public:
   // Returns all rules of the given nonterminal symbol.
   llvm::ArrayRef<Rule> rulesFor(SymbolID SID) const;
   const Rule &lookupRule(RuleID RID) const;
 
   // Gets symbol (terminal or nonterminal) name.
   // Terminals have names like "," (kw_comma) or "OPERATOR" (kw_operator).
   llvm::StringRef symbolName(SymbolID) const;
 
-  // Gets the mangled name for a terminal/nonterminal.
-  // Compared to names in the grammar,
-  //   nonterminals `ptr-declartor` becomes `ptr_declarator`;
-  //   terminal `,` becomes `comma`;
-  //   terminal `IDENTIFIER` becomes `identifier`;
-  //   terminal `INT` becomes `int`;
-  // NOTE: for nonterminals, the mangled name is the same as the cxx::Symbol
-  // enum class; for terminals, we deliberately stripped the `kw_` prefix in
-  // favor of the simplicity.
-  std::string mangleSymbol(SymbolID) const;
-  // Gets the mangled name for the rule.
-  // E.g. for the grammar rule `ptr-declarator := ptr-operator ptr-declarator`,
-  // it is `ptr_declarator_0ptr_operator_1ptr_declarator`.
-  std::string mangleRule(RuleID) const;
-
   // Lookup the SymbolID of the nonterminal symbol by Name.
   llvm::Optional<SymbolID> findNonterminal(llvm::StringRef Name) const;
 
   // Dumps the whole grammar.
   std::string dump() const;
   // Dumps a particular rule.
   std::string dumpRule(RuleID) const;
   // Dumps all rules of the given nonterminal symbol.

clang-tools-extra/pseudo/lib/cxx/CXX.cpp

 const ForestNode &onlySymbol(SymbolID Kind,
                              const ArrayRef<const ForestNode *> RHS,
                              const TokenStream &Tokens) {
   assert(RHS.size() == 1 && RHS.front()->symbol() == Kind);
   return *RHS.front();
 }
 
 bool isFunctionDeclarator(const ForestNode *Declarator) {
-  assert(Declarator->symbol() == (SymbolID)(cxx::Symbol::declarator));
+  assert(Declarator->symbol() == cxx::Symbol::declarator);
   bool IsFunction = false;
-  using cxx::Rule;
   while (true) {
     // not well-formed code, return the best guess.
     if (Declarator->kind() != ForestNode::Sequence)
       return IsFunction;
 
-    switch ((cxx::Rule)Declarator->rule()) {
-    case Rule::noptr_declarator_0declarator_id: // reached the bottom
+    switch (Declarator->rule()) {
+    case rule::noptr_declarator::declarator_id: // reached the bottom
       return IsFunction;
     // *X is a nonfunction (unless X is a function).
-    case Rule::ptr_declarator_0ptr_operator_1ptr_declarator:
+    case rule::ptr_declarator::ptr_operator__ptr_declarator:
       Declarator = Declarator->elements()[1];
       IsFunction = false;
       continue;
     // X() is a function (unless X is a pointer or similar).
-    case Rule::
-        declarator_0noptr_declarator_1parameters_and_qualifiers_2trailing_return_type:
-    case Rule::noptr_declarator_0noptr_declarator_1parameters_and_qualifiers:
+    case rule::declarator::
+        noptr_declarator__parameters_and_qualifiers__trailing_return_type:
+    case rule::noptr_declarator::noptr_declarator__parameters_and_qualifiers:
       Declarator = Declarator->elements()[0];
       IsFunction = true;
       continue;
     // X[] is an array (unless X is a pointer or function).
-    case Rule::
-        noptr_declarator_0noptr_declarator_1l_square_2constant_expression_3r_square:
-    case Rule::noptr_declarator_0noptr_declarator_1l_square_2r_square:
+    case rule::noptr_declarator::
+        noptr_declarator__L_SQUARE__constant_expression__R_SQUARE:
+    case rule::noptr_declarator::noptr_declarator__L_SQUARE__R_SQUARE:
       Declarator = Declarator->elements()[0];
       IsFunction = false;
       continue;
     // (X) is whatever X is.
-    case Rule::noptr_declarator_0l_paren_1ptr_declarator_2r_paren:
+    case rule::noptr_declarator::L_PAREN__ptr_declarator__R_PAREN:
       Declarator = Declarator->elements()[1];
       continue;
-    case Rule::ptr_declarator_0noptr_declarator:
-    case Rule::declarator_0ptr_declarator:
+    case rule::ptr_declarator::noptr_declarator:
+    case rule::declarator::ptr_declarator:
       Declarator = Declarator->elements()[0];
       continue;
 
     default:
       assert(false && "unhandled declarator for IsFunction");
       return IsFunction;
     }
   }

 // Returns false for
 //  - non-types
 //  - "unsigned" etc that may suffice as types but may modify others
 //  - cases of uncertainty (e.g. due to ambiguity)
 bool hasExclusiveType(const ForestNode *N) {
   // FIXME: every time we apply this check, we walk the whole subtree.
   // Add per-node caching instead.
   while (true) {
-    assert(N->symbol() == (SymbolID)Symbol::decl_specifier_seq ||
-           N->symbol() == (SymbolID)Symbol::type_specifier_seq ||
-           N->symbol() == (SymbolID)Symbol::defining_type_specifier_seq ||
-           N->symbol() == (SymbolID)Symbol::decl_specifier ||
-           N->symbol() == (SymbolID)Symbol::type_specifier ||
-           N->symbol() == (SymbolID)Symbol::defining_type_specifier ||
-           N->symbol() == (SymbolID)Symbol::simple_type_specifier);
+    assert(N->symbol() == Symbol::decl_specifier_seq ||
+           N->symbol() == Symbol::type_specifier_seq ||
+           N->symbol() == Symbol::defining_type_specifier_seq ||
+           N->symbol() == Symbol::decl_specifier ||
+           N->symbol() == Symbol::type_specifier ||
+           N->symbol() == Symbol::defining_type_specifier ||
+           N->symbol() == Symbol::simple_type_specifier);
     if (N->kind() == ForestNode::Opaque)
       return false; // conservative
     if (N->kind() == ForestNode::Ambiguous)
       return llvm::all_of(N->alternatives(), hasExclusiveType); // conservative
     // All supported symbols are nonterminals.
     assert(N->kind() == ForestNode::Sequence);
     switch (N->rule()) {
       // seq := element seq: check element then continue into seq
-      case (RuleID)Rule::decl_specifier_seq_0decl_specifier_1decl_specifier_seq:
-      case (RuleID)Rule::defining_type_specifier_seq_0defining_type_specifier_1defining_type_specifier_seq:
-      case (RuleID)Rule::type_specifier_seq_0type_specifier_1type_specifier_seq:
+      case rule::decl_specifier_seq::decl_specifier__decl_specifier_seq:
+      case rule::defining_type_specifier_seq::defining_type_specifier__defining_type_specifier_seq:
+      case rule::type_specifier_seq::type_specifier__type_specifier_seq:
         if (hasExclusiveType(N->children()[0]))
           return true;
         N = N->children()[1];
         continue;
       // seq := element: continue into element
-      case (RuleID)Rule::decl_specifier_seq_0decl_specifier:
-      case (RuleID)Rule::type_specifier_seq_0type_specifier:
-      case (RuleID)Rule::defining_type_specifier_seq_0defining_type_specifier:
+      case rule::decl_specifier_seq::decl_specifier:
+      case rule::type_specifier_seq::type_specifier:
+      case rule::defining_type_specifier_seq::defining_type_specifier:
         N = N->children()[0];
         continue;
 
       // defining-type-specifier
-      case (RuleID)Rule::defining_type_specifier_0type_specifier:
+      case rule::defining_type_specifier::type_specifier:
         N = N->children()[0];
         continue;
-      case (RuleID)Rule::defining_type_specifier_0class_specifier:
-      case (RuleID)Rule::defining_type_specifier_0enum_specifier:
+      case rule::defining_type_specifier::class_specifier:
+      case rule::defining_type_specifier::enum_specifier:
         return true;
 
       // decl-specifier
-      case (RuleID)Rule::decl_specifier_0defining_type_specifier:
+      case rule::decl_specifier::defining_type_specifier:
         N = N->children()[0];
         continue;
-      case (RuleID)Rule::decl_specifier_0consteval:
-      case (RuleID)Rule::decl_specifier_0constexpr:
-      case (RuleID)Rule::decl_specifier_0constinit:
-      case (RuleID)Rule::decl_specifier_0inline:
-      case (RuleID)Rule::decl_specifier_0friend:
-      case (RuleID)Rule::decl_specifier_0storage_class_specifier:
-      case (RuleID)Rule::decl_specifier_0typedef:
-      case (RuleID)Rule::decl_specifier_0function_specifier:
+      case rule::decl_specifier::CONSTEVAL:
+      case rule::decl_specifier::CONSTEXPR:
+      case rule::decl_specifier::CONSTINIT:
+      case rule::decl_specifier::INLINE:
+      case rule::decl_specifier::FRIEND:
+      case rule::decl_specifier::storage_class_specifier:
+      case rule::decl_specifier::TYPEDEF:
+      case rule::decl_specifier::function_specifier:
         return false;
 
       // type-specifier
-      case (RuleID)Rule::type_specifier_0elaborated_type_specifier:
-      case (RuleID)Rule::type_specifier_0typename_specifier:
+      case rule::type_specifier::elaborated_type_specifier:
+      case rule::type_specifier::typename_specifier:
         return true;
-      case (RuleID)Rule::type_specifier_0simple_type_specifier:
+      case rule::type_specifier::simple_type_specifier:
         N = N->children()[0];
         continue;
-      case (RuleID)Rule::type_specifier_0cv_qualifier:
+      case rule::type_specifier::cv_qualifier:
         return false;
 
       // simple-type-specifier
-      case (RuleID)Rule::simple_type_specifier_0type_name:
-      case (RuleID)Rule::simple_type_specifier_0template_name:
-      case (RuleID)Rule::simple_type_specifier_0builtin_type:
-      case (RuleID)Rule::simple_type_specifier_0nested_name_specifier_1template_2simple_template_id:
-      case (RuleID)Rule::simple_type_specifier_0nested_name_specifier_1template_name:
-      case (RuleID)Rule::simple_type_specifier_0nested_name_specifier_1type_name:
-      case (RuleID)Rule::simple_type_specifier_0decltype_specifier:
-      case (RuleID)Rule::simple_type_specifier_0placeholder_type_specifier:
+      case rule::simple_type_specifier::type_name:
+      case rule::simple_type_specifier::template_name:
+      case rule::simple_type_specifier::builtin_type:
+      case rule::simple_type_specifier::nested_name_specifier__TEMPLATE__simple_template_id:
+      case rule::simple_type_specifier::nested_name_specifier__template_name:
+      case rule::simple_type_specifier::nested_name_specifier__type_name:
+      case rule::simple_type_specifier::decltype_specifier:
+      case rule::simple_type_specifier::placeholder_type_specifier:
         return true;
-      case (RuleID)Rule::simple_type_specifier_0long:
-      case (RuleID)Rule::simple_type_specifier_0short:
-      case (RuleID)Rule::simple_type_specifier_0signed:
-      case (RuleID)Rule::simple_type_specifier_0unsigned:
+      case rule::simple_type_specifier::LONG:
+      case rule::simple_type_specifier::SHORT:
+      case rule::simple_type_specifier::SIGNED:
+      case rule::simple_type_specifier::UNSIGNED:
         return false;
 
       default:
         LLVM_DEBUG(llvm::errs() << "Unhandled rule " << N->rule() << "\n");
         llvm_unreachable("hasExclusiveType be exhaustive!");
     }
   }
 }
 
 llvm::DenseMap<ExtensionID, RuleGuard> buildGuards() {
 #define GUARD(cond)                                                            \
   {                                                                            \
     [](const GuardParams &P) { return cond; }                                  \
   }
 #define TOKEN_GUARD(kind, cond)                                                \
   [](const GuardParams& P) {                                                   \
     const Token &Tok = onlyToken(tok::kind, P.RHS, P.Tokens);                  \
     return cond;                                                               \
   }
 #define SYMBOL_GUARD(kind, cond)                                               \
   [](const GuardParams& P) {                                                   \
-    const ForestNode &N = onlySymbol((SymbolID)Symbol::kind, P.RHS, P.Tokens); \
+    const ForestNode &N = onlySymbol(Symbol::kind, P.RHS, P.Tokens); \
     return cond;                                                               \
   }
   return {
-      {(RuleID)Rule::function_declarator_0declarator,
+      {rule::function_declarator::declarator,
        SYMBOL_GUARD(declarator, isFunctionDeclarator(&N))},
-      {(RuleID)Rule::non_function_declarator_0declarator,
+      {rule::non_function_declarator::declarator,
        SYMBOL_GUARD(declarator, !isFunctionDeclarator(&N))},
 
       // A {decl,type,defining-type}-specifier-sequence cannot have multiple
       // "exclusive" types (like class names): a value has only one type.
-      {(RuleID)Rule::
-           defining_type_specifier_seq_0defining_type_specifier_1defining_type_specifier_seq,
+      {rule::defining_type_specifier_seq::
+           defining_type_specifier__defining_type_specifier_seq,
        GUARD(!hasExclusiveType(P.RHS[0]) || !hasExclusiveType(P.RHS[1]))},
-      {(RuleID)Rule::type_specifier_seq_0type_specifier_1type_specifier_seq,
+      {rule::type_specifier_seq::type_specifier__type_specifier_seq,
        GUARD(!hasExclusiveType(P.RHS[0]) || !hasExclusiveType(P.RHS[1]))},
-      {(RuleID)Rule::decl_specifier_seq_0decl_specifier_1decl_specifier_seq,
+      {rule::decl_specifier_seq::decl_specifier__decl_specifier_seq,
        GUARD(!hasExclusiveType(P.RHS[0]) || !hasExclusiveType(P.RHS[1]))},
 
-      {(RuleID)Rule::contextual_override_0identifier,
+      {rule::contextual_override::IDENTIFIER,
        TOKEN_GUARD(identifier, Tok.text() == "override")},
-      {(RuleID)Rule::contextual_final_0identifier,
+      {rule::contextual_final::IDENTIFIER,
        TOKEN_GUARD(identifier, Tok.text() == "final")},
-      {(RuleID)Rule::import_keyword_0identifier,
+      {rule::import_keyword::IDENTIFIER,
        TOKEN_GUARD(identifier, Tok.text() == "import")},
-      {(RuleID)Rule::export_keyword_0identifier,
+      {rule::export_keyword::IDENTIFIER,
        TOKEN_GUARD(identifier, Tok.text() == "export")},
-      {(RuleID)Rule::module_keyword_0identifier,
+      {rule::module_keyword::IDENTIFIER,
        TOKEN_GUARD(identifier, Tok.text() == "module")},
-      {(RuleID)Rule::contextual_zero_0numeric_constant,
+      {rule::contextual_zero::NUMERIC_CONSTANT,
        TOKEN_GUARD(numeric_constant, Tok.text() == "0")},
 
-      {(RuleID)Rule::
-           selection_statement_0if_1l_paren_2condition_3r_paren_4statement,
+      // FIXME: the init-statement variants are missing?
+      {rule::selection_statement::IF__L_PAREN__condition__R_PAREN__statement,
        guardNextTokenNotElse},
-      {(RuleID)Rule::
-           selection_statement_0if_1constexpr_2l_paren_3condition_4r_paren_5statement,
+      {rule::selection_statement::
+           IF__CONSTEXPR__L_PAREN__condition__R_PAREN__statement,
        guardNextTokenNotElse},
 
       // The grammar distinguishes (only) user-defined vs plain string literals,
       // where the clang lexer distinguishes (only) encoding types.
-      {(RuleID)Rule::user_defined_string_literal_chunk_0string_literal,
+      {rule::user_defined_string_literal_chunk::STRING_LITERAL,
        TOKEN_GUARD(string_literal, isStringUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_string_literal_chunk_0utf8_string_literal,
+      {rule::user_defined_string_literal_chunk::UTF8_STRING_LITERAL,
        TOKEN_GUARD(utf8_string_literal, isStringUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_string_literal_chunk_0utf16_string_literal,
+      {rule::user_defined_string_literal_chunk::UTF16_STRING_LITERAL,
        TOKEN_GUARD(utf16_string_literal, isStringUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_string_literal_chunk_0utf32_string_literal,
+      {rule::user_defined_string_literal_chunk::UTF32_STRING_LITERAL,
        TOKEN_GUARD(utf32_string_literal, isStringUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_string_literal_chunk_0wide_string_literal,
+      {rule::user_defined_string_literal_chunk::WIDE_STRING_LITERAL,
        TOKEN_GUARD(wide_string_literal, isStringUserDefined(Tok))},
-      {(RuleID)Rule::string_literal_chunk_0string_literal,
+      {rule::string_literal_chunk::STRING_LITERAL,
        TOKEN_GUARD(string_literal, !isStringUserDefined(Tok))},
-      {(RuleID)Rule::string_literal_chunk_0utf8_string_literal,
+      {rule::string_literal_chunk::UTF8_STRING_LITERAL,
        TOKEN_GUARD(utf8_string_literal, !isStringUserDefined(Tok))},
-      {(RuleID)Rule::string_literal_chunk_0utf16_string_literal,
+      {rule::string_literal_chunk::UTF16_STRING_LITERAL,
        TOKEN_GUARD(utf16_string_literal, !isStringUserDefined(Tok))},
-      {(RuleID)Rule::string_literal_chunk_0utf32_string_literal,
+      {rule::string_literal_chunk::UTF32_STRING_LITERAL,
        TOKEN_GUARD(utf32_string_literal, !isStringUserDefined(Tok))},
-      {(RuleID)Rule::string_literal_chunk_0wide_string_literal,
+      {rule::string_literal_chunk::WIDE_STRING_LITERAL,
        TOKEN_GUARD(wide_string_literal, !isStringUserDefined(Tok))},
       // And the same for chars.
-      {(RuleID)Rule::user_defined_character_literal_0char_constant,
+      {rule::user_defined_character_literal::CHAR_CONSTANT,
        TOKEN_GUARD(char_constant, isCharUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_character_literal_0utf8_char_constant,
+      {rule::user_defined_character_literal::UTF8_CHAR_CONSTANT,
        TOKEN_GUARD(utf8_char_constant, isCharUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_character_literal_0utf16_char_constant,
+      {rule::user_defined_character_literal::UTF16_CHAR_CONSTANT,
        TOKEN_GUARD(utf16_char_constant, isCharUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_character_literal_0utf32_char_constant,
+      {rule::user_defined_character_literal::UTF32_CHAR_CONSTANT,
        TOKEN_GUARD(utf32_char_constant, isCharUserDefined(Tok))},
-      {(RuleID)Rule::user_defined_character_literal_0wide_char_constant,
+      {rule::user_defined_character_literal::WIDE_CHAR_CONSTANT,
        TOKEN_GUARD(wide_char_constant, isCharUserDefined(Tok))},
-      {(RuleID)Rule::character_literal_0char_constant,
+      {rule::character_literal::CHAR_CONSTANT,
        TOKEN_GUARD(char_constant, !isCharUserDefined(Tok))},
-      {(RuleID)Rule::character_literal_0utf8_char_constant,
+      {rule::character_literal::UTF8_CHAR_CONSTANT,
        TOKEN_GUARD(utf8_char_constant, !isCharUserDefined(Tok))},
-      {(RuleID)Rule::character_literal_0utf16_char_constant,
+      {rule::character_literal::UTF16_CHAR_CONSTANT,
        TOKEN_GUARD(utf16_char_constant, !isCharUserDefined(Tok))},
-      {(RuleID)Rule::character_literal_0utf32_char_constant,
+      {rule::character_literal::UTF32_CHAR_CONSTANT,
        TOKEN_GUARD(utf32_char_constant, !isCharUserDefined(Tok))},
-      {(RuleID)Rule::character_literal_0wide_char_constant,
+      {rule::character_literal::WIDE_CHAR_CONSTANT,
        TOKEN_GUARD(wide_char_constant, !isCharUserDefined(Tok))},
       // clang just has one NUMERIC_CONSTANT token for {ud,plain}x{float,int}
-      {(RuleID)Rule::user_defined_integer_literal_0numeric_constant,
+      {rule::user_defined_integer_literal::NUMERIC_CONSTANT,
        TOKEN_GUARD(numeric_constant, numKind(Tok) == (Integer | UserDefined))},
-      {(RuleID)Rule::user_defined_floating_point_literal_0numeric_constant,
+      {rule::user_defined_floating_point_literal::NUMERIC_CONSTANT,
        TOKEN_GUARD(numeric_constant, numKind(Tok) == (Floating | UserDefined))},
-      {(RuleID)Rule::integer_literal_0numeric_constant,
+      {rule::integer_literal::NUMERIC_CONSTANT,
        TOKEN_GUARD(numeric_constant, numKind(Tok) == Integer)},
-      {(RuleID)Rule::floating_point_literal_0numeric_constant,
+      {rule::floating_point_literal::NUMERIC_CONSTANT,
        TOKEN_GUARD(numeric_constant, numKind(Tok) == Floating)},
   };
 #undef TOKEN_GUARD
 #undef SYMBOL_GUARD
 }
 
 Token::Index recoverBrackets(Token::Index Begin, const TokenStream &Tokens) {
   assert(Begin > 0);
   const Token &Left = Tokens.tokens()[Begin - 1];
   assert(Left.Kind == tok::l_brace || Left.Kind == tok::l_paren ||
          Left.Kind == tok::l_square);
   if (const Token *Right = Left.pair()) {
     assert(Tokens.index(*Right) > Begin - 1);
     return Tokens.index(*Right);
   }
   return Token::Invalid;
 }
 
 llvm::DenseMap<ExtensionID, RecoveryStrategy> buildRecoveryStrategies() {
   return {
-      {(ExtensionID)Extension::Brackets, recoverBrackets},
+      {Extension::Brackets, recoverBrackets},
   };
 }
 
 } // namespace
 
 const Language &getLanguage() {
   static const auto &CXXLanguage = []() -> const Language & {
     std::vector<std::string> Diags;

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp

const Rule &Grammar::lookupRule(RuleID RID) const {
   return T->Rules[RID];
 }
 
 llvm::StringRef Grammar::symbolName(SymbolID SID) const {
   if (isToken(SID))
     return T->Terminals[symbolToToken(SID)];
   return T->Nonterminals[SID].Name;
 }
 
-std::string Grammar::mangleSymbol(SymbolID SID) const {
-  static const char *const TokNames[] = {
-#define TOK(X) #X,
-#define KEYWORD(X, Y) #X,
-#include "clang/Basic/TokenKinds.def"
-      nullptr};
-  if (clang::pseudo::isToken(SID))
-    return TokNames[clang::pseudo::symbolToToken(SID)];
-  std::string Name = symbolName(SID).str();
-  // translation-unit -> translation_unit
-  std::replace(Name.begin(), Name.end(), '-', '_');
-  return Name;
-}
-
-std::string Grammar::mangleRule(RuleID RID) const {
-  const auto &R = lookupRule(RID);
-  std::string MangleName = mangleSymbol(R.Target);
-  for (size_t I = 0; I < R.seq().size(); ++I)
-    MangleName += llvm::formatv("_{0}{1}", I, mangleSymbol(R.seq()[I]));
-  return MangleName;
-}
-
 llvm::Optional<SymbolID> Grammar::findNonterminal(llvm::StringRef Name) const {
   auto It = llvm::partition_point(
       T->Nonterminals,
       [&](const GrammarTable::Nonterminal &X) { return X.Name < Name; });
   if (It != T->Nonterminals.end() && It->Name == Name)
     return It - T->Nonterminals.begin();
   return llvm::None;
 }
 

clang-tools-extra/pseudo/unittests/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_custom_target(ClangPseudoUnitTests)
 add_unittest(ClangPseudoUnitTests ClangPseudoTests
   BracketTest.cpp
+  CXXTest.cpp
   DirectiveTreeTest.cpp
   ForestTest.cpp
   GLRTest.cpp
   GrammarTest.cpp
   LRTableTest.cpp
   TokenTest.cpp
 )
 
 clang_target_link_libraries(ClangPseudoTests
   PRIVATE
   clangBasic
   clangLex
   )
 
 target_link_libraries(ClangPseudoTests
   PRIVATE
   clangPseudo
+  clangPseudoCXX
   clangPseudoGrammar
   LLVMTestingSupport
   )

clang-tools-extra/pseudo/unittests/CXXTest.cpp

+//===--- CXXTest.cpp ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang-pseudo/cxx/CXX.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace pseudo {
+namespace cxx {
+namespace {
+
+TEST(CXX, GeneratedEnums) {
+  const auto &Lang = clang::pseudo::cxx::getLanguage();
+  EXPECT_EQ("iteration-statement",
+            Lang.G.symbolName(Symbol::iteration_statement));
+  EXPECT_EQ("iteration-statement := DO statement WHILE ( expression ) ;",
+            Lang.G.dumpRule(
+                rule::iteration_statement::
+                    DO__statement__WHILE__L_PAREN__expression__R_PAREN__SEMI));
+}
+
+} // namespace
+} // namespace cxx
+} // namespace pseudo
+} // namespace clang

clang-tools-extra/pseudo/unittests/GrammarTest.cpp

build(R"bnf(
     _ := x
     x := IDENTIFIER [guard]
   )bnf");
   ASSERT_THAT(Diags, IsEmpty());
   EXPECT_FALSE(G.lookupRule(ruleFor("_")).Guarded);
   EXPECT_TRUE(G.lookupRule(ruleFor("x")).Guarded);
 }
 
-TEST_F(GrammarTest, MangleName) {
-  build(R"bnf(
-    _ := declaration
-
-    declaration := ptr-declarator ;
-    ptr-declarator := * IDENTIFIER
-
-  )bnf");
-  ASSERT_TRUE(Diags.empty());
-  EXPECT_EQ(G.mangleRule(ruleFor("declaration")),
-            "declaration_0ptr_declarator_1semi");
-  EXPECT_EQ(G.mangleRule(ruleFor("ptr-declarator")),
-            "ptr_declarator_0star_1identifier");
-}
-
 TEST_F(GrammarTest, Diagnostics) {
   build(R"cpp(
     _ := ,_opt
     _ := undefined-sym
     null :=
     _ := IDENFIFIE # a typo of the terminal IDENFITIER
 
     invalid
     # cycle