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

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131762

Files:
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf


Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===================================================================
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -334,7 +334,7 @@
 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 ) ;
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===================================================================
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -161,6 +161,59 @@
   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.
 //
@@ -320,6 +373,15 @@
       {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,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131762.452144.patch
Type: text/x-patch
Size: 4374 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220812/8a4042d5/attachment.bin>