[pseudo][wip] Enforce the C++ rule for the optional init-declarator-list.

[hokein]

Summary

Basically it implements the https://eel.is/c++draft/dcl.pre#5 rule.

The motivation is to eliminate a false parse (simple declaration) for the enum
declaration.

// A simple declaration without a declarator
// or an opaque-enum-declaration
enum a;

However, it has some negative effects on broken code (with the single-type
decl-specifier-seq guard):

• break the const int; case (which we used to parse as a declaration without declarator), now we fails to parse, because it is invalid code per the C++ spec.
• regress the const Foo; case, now we parse it as a declaration with a declarator named Foo and const as the decl-specifier-seq, vs we used to parsed as a declaration without declarator (which is better IMO);

Just post here to get thoughts about it:
My feeling is that this degrades the parser a lot for the little benefit we gain,
we probably don't move forward with this approach.

[sammccall]

Basically it implements the https://eel.is/c++draft/dcl.pre#5 rule.

Just to make sure we're on the same page, my understanding of that rule is roughly "declarations must declare something", and it has two parts:

• the declarators can only be omitted if a named class/enum is declared (this patch)
• "shall (re)introduce one or more names": if the declarators are omitted, either the class/enum must be named or the enum must provide enumerators (not in this patch)

This corresponds to clang's -Wmissing-declarations, which is a warning rather than an error! I think this is an argument to allow these parses at least in cases when there's no correct parse.
Ideally we'd eliminate the wrong one when there's actual ambiguity, this isn't really doable with guards, the closest mechanism we have is "soft" disambiguation.

regress the const Foo; case, now we parse it as a declaration with a declarator named Foo and const as the decl-specifier-seq, vs we used to parsed as a declaration without declarator (which is better IMO);

I don't think it's clear. Foo could easily be a easily type with missing variable name or a variable name with missing type. A human would interpret this based on the other occurrences of Foo, based on case, etc. I think this is an argument in favor of solving this "softly" with ranking (though not a strong one: we can't let 'correctly' parsing broken code drive the parser *too* much).

My feeling is that this degrades the parser a lot for the little benefit we gain, we probably don't move forward with this approach.

I agree, I think we should solve this by downranking e.g. declarator-less declarations (so they're only used if needed). It's not ideal (I'd like to completely drop the misparse of enum foo; and similar cases where there's a valid alternative), but this isn't an *incredibly* common pattern that demands complete early removal from the forest.

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

bool isFunctionDeclarator(const ForestNode *Declarator) {
   }
   llvm_unreachable("unreachable");
 }
 
 bool guardNextTokenNotElse(const GuardParams &P) {
   return symbolToToken(P.Lookahead) != tok::kw_else;
 }
 
+bool canOmitDeclarator(const ForestNode* N) {
+  // Returns true if a decl-specifier is
+  //   a class-specifier
+  //   or an elaborated-type-specifier with class-key, or an enum_specifier.
+  auto hasTargetSpecifier = [](const ForestNode *N, auto &hasTargetSpecifier) {
+    if (N->kind() == ForestNode::Opaque)
+      return false;
+    if (N->kind() == ForestNode::Ambiguous)
+      return llvm::all_of(N->alternatives(),
+                          std::bind(hasTargetSpecifier, std::placeholders::_1,
+                                    hasTargetSpecifier));
+
+    switch (N->rule()) {
+    case rule::defining_type_specifier::type_specifier:
+    case rule::defining_type_specifier::enum_specifier:
+    case rule::type_specifier::elaborated_type_specifier:
+    case rule::decl_specifier::defining_type_specifier:
+      return hasTargetSpecifier(N->elements()[0], hasTargetSpecifier);
+
+    case rule::enum_specifier::enum_head__L_BRACE__R_BRACE:
+    case rule::enum_specifier::
+        enum_head__L_BRACE__enumerator_list__COMMA__R_BRACE:
+    case rule::enum_specifier::enum_head__L_BRACE__enumerator_list__R_BRACE:
+    case rule::elaborated_type_specifier::
+        class_key__nested_name_specifier__IDENTIFIER:
+    case rule::elaborated_type_specifier::
+        class_key__nested_name_specifier__simple_template_id:
+    case rule::elaborated_type_specifier::
+        class_key__nested_name_specifier__TEMPLATE__simple_template_id:
+    case rule::elaborated_type_specifier::class_key__simple_template_id:
+    case rule::elaborated_type_specifier::class_key__IDENTIFIER:
+    case rule::defining_type_specifier::class_specifier:
+      return true;
+    default:
+      return false;
+    }
+  };
+  while (true) {
+    switch (N->rule()) {
+    case cxx::rule::decl_specifier_seq::decl_specifier:
+      return hasTargetSpecifier(N->elements()[0], hasTargetSpecifier);
+    case cxx::rule::decl_specifier_seq::decl_specifier__decl_specifier_seq:
+      if (hasTargetSpecifier(N->elements()[0], hasTargetSpecifier))
+        return true;
+      N = N->elements()[1];
+      break;
+    default:
+      LLVM_DEBUG(llvm::errs() << "Unhandled rule " << N->rule() << "\n");
+      llvm_unreachable("decl_specifier_seq be exhaustive!");
+    }
+  }
+  return false;
+}
 // Whether this e.g. decl-specifier contains an "exclusive" type such as a class
 // name, and thus can't combine with a second exclusive type.
 //
 // 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) {
return {
       // Implement C++ [basic.lookup.qual.general]:
       //   If a name, template-id, or decltype-specifier is followed by a
       //   !::!, it shall designate a namespace, class, enumeration, or
       //   dependent type, and the !::! is never interpreted as a complete
       //   nested-name-specifier.
       {rule::nested_name_specifier::COLONCOLON,
        TOKEN_GUARD(coloncolon, Tok.prev().Kind != tok::identifier)},
 
+      // Implement C++ [dcl.pre#5]:
+      //   In a simple-declaration, the optional init-declarator-list can be
+      //   omitted only when declaring a class ([class.pre]) or enumeration
+      //   ([dcl.enum]), that is, when the decl-specifier-seq contains either a
+      //   class-specifier, an elaborated-type-specifier with a class-key
+      //   ([class.name]), or an enum-specifier.
+      {rule::simple_declaration::decl_specifier_seq__SEMI,
+       GUARD(canOmitDeclarator(P.RHS[0]))},
+
       // The grammar distinguishes (only) user-defined vs plain string literals,
       // where the clang lexer distinguishes (only) encoding types.
       {rule::user_defined_string_literal_chunk::STRING_LITERAL,
        TOKEN_GUARD(string_literal, isStringUserDefined(Tok))},
       {rule::user_defined_string_literal_chunk::UTF8_STRING_LITERAL,
        TOKEN_GUARD(utf8_string_literal, isStringUserDefined(Tok))},
       {rule::user_defined_string_literal_chunk::UTF16_STRING_LITERAL,
        TOKEN_GUARD(utf16_string_literal, isStringUserDefined(Tok))},

clang-tools-extra/pseudo/lib/cxx/cxx.bnf

 block-declaration := using-declaration
 block-declaration := using-enum-declaration
 block-declaration := using-directive
 block-declaration := static_assert-declaration
 block-declaration := alias-declaration
 block-declaration := opaque-enum-declaration
 nodeclspec-function-declaration := function-declarator ;
 alias-declaration := USING IDENTIFIER = defining-type-id ;
-simple-declaration := decl-specifier-seq init-declarator-list_opt ;
+simple-declaration := decl-specifier-seq init-declarator-list_opt ; [guard]
 simple-declaration := decl-specifier-seq ref-qualifier_opt [ identifier-list ] initializer ;
 static_assert-declaration := STATIC_ASSERT ( constant-expression ) ;
 static_assert-declaration := STATIC_ASSERT ( constant-expression , string-literal ) ;
 empty-declaration := ;
 # dcl.spec
 decl-specifier := storage-class-specifier
 decl-specifier := defining-type-specifier
 decl-specifier := function-specifier