Skip to content

Resolving issue #119101 #119143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

rkmaxhero
Copy link

CLANG parser previously allowed for invalid C/C++ auto classes declaration due to a lack of logic addressing this case. Logic comparing with a valid case was added to the ParseDeclarationOrFunctionDefinition() function to account for this. Test cases where added to address possible scenarios of auto class declaration. the parser diagnostic file will now detect invalid auto class definitions and output appropriately.

Copy link

github-actions bot commented Dec 8, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 8, 2024

@llvm/pr-subscribers-clang

Author: Rounaq Khan (rkmaxhero)

Changes

CLANG parser previously allowed for invalid C/C++ auto classes declaration due to a lack of logic addressing this case. Logic comparing with a valid case was added to the ParseDeclarationOrFunctionDefinition() function to account for this. Test cases where added to address possible scenarios of auto class declaration. the parser diagnostic file will now detect invalid auto class definitions and output appropriately.


Full diff: https://github.com/llvm/llvm-project/pull/119143.diff

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+2)
  • (modified) clang/lib/Parse/Parser.cpp (+12)
  • (added) clang/test/SemaCXX/invalid-storage-class.cpp (+7)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 0da509280068ad..99d698f57982fc 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -449,6 +449,8 @@ def err_typename_invalid_constexpr : Error<
   "to be specified">;
 def err_typename_identifiers_only : Error<
   "typename is allowed for identifiers only">;
+def err_storage_class_before_class_decl : Error<
+  "'%0' is not allowed before a class declaration">;
 
 def err_friend_invalid_in_context : Error<
   "'friend' used outside of class">;
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 04c2f1d380bc48..2145f78f4b2749 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1254,6 +1254,18 @@ Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition(
         Actions.getASTContext().getSourceManager());
   });
 
+  if (DS->getStorageClassSpec() != DeclSpec::SCS_unspecified) {
+  // Check if the next token starts a class/struct/union/enum declaration
+    if (Tok.isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union, tok::kw_enum)) {
+      // Emit an error: storage class specifiers are not allowed before class declarations
+      Diag(DS->getStorageClassSpecLoc(), diag::err_storage_class_before_class_decl)
+          << DeclSpec::getSpecifierName(DS->getStorageClassSpec());
+
+      // Optionally, consume the storage class specifier to continue parsing
+      DS->ClearStorageClassSpecs();
+    }
+  }
+
   if (DS) {
     return ParseDeclOrFunctionDefInternal(Attrs, DeclSpecAttrs, *DS, AS);
   } else {
diff --git a/clang/test/SemaCXX/invalid-storage-class.cpp b/clang/test/SemaCXX/invalid-storage-class.cpp
new file mode 100644
index 00000000000000..ce0dee3711ac03
--- /dev/null
+++ b/clang/test/SemaCXX/invalid-storage-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+auto class X1 {}; // expected-error {{'auto' is not allowed before a class declaration}}
+
+static struct X2 {}; // expected-error {{'static' is not allowed before a class declaration}}
+
+register union X3 {}; // expected-error {{'register' is not allowed before a class declaration}}
\ No newline at end of file

@zygoloid
Copy link
Collaborator

zygoloid commented Dec 8, 2024

Did you mean a different issue than #119101? It's not clear to me what connection this PR has to that issue.

It's not clear to me that there's an issue to be solved here. Under -pedantic-errors, clang already produces an error on examples like the ones here, and by default clang produces a warning on such cases, so we are already diagnosing this. And we need to continue to accept things like static struct A {} a;, which are valid but would be rejected by this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants