Skip to content

[C11] Diagnose C11 keywords as being incompatible w/earlier standards #82015

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

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

AaronBallman
Copy link
Collaborator

Our usual pattern when issuing an extension warning is to also issue a
default-off diagnostic about the keywords not being compatible with
standards before a certain point. This adds those diagnostics for C11
keywords.

Our usual pattern when issuing an extension warning is to also issue a
default-off diagnostic about the keywords not being compatible with
standards before a certain point. This adds those diagnostics for C11
keywords.
@AaronBallman AaronBallman added clang Clang issues not falling into any other category c11 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Feb 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

Our usual pattern when issuing an extension warning is to also issue a
default-off diagnostic about the keywords not being compatible with
standards before a certain point. This adds those diagnostics for C11
keywords.


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

9 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+3)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+3)
  • (modified) clang/include/clang/Parse/Parser.h (+2)
  • (modified) clang/lib/Parse/ParseDecl.cpp (+5-11)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+3-3)
  • (modified) clang/lib/Parse/ParseExpr.cpp (+3-4)
  • (modified) clang/lib/Parse/Parser.cpp (+13)
  • (added) clang/test/Sema/c11-keywords.c (+37)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 16f79a349c20c8..06c7d57d73ca70 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -185,6 +185,9 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses friend declarations with an ``enum`` elaborated-type-specifier in language modes after C++98.
 
+- Added diagnostics for C11 keywords being incompatible with language standards
+  before C11, under a new warning group: ``-Wpre-c11-compat``.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 7679b8528a4197..e8b4139d7893ce 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -290,6 +290,9 @@ def : DiagGroup<"c++1z-compat-mangling", [CXX17CompatMangling]>;
 def NoexceptType : DiagGroup<"noexcept-type", [CXX17CompatMangling]>;
 
 // Warnings for C code which is not compatible with previous C standards.
+def CPre11Compat : DiagGroup<"pre-c11-compat">;
+def CPre11CompatPedantic : DiagGroup<"pre-c11-compat-pedantic",
+                                     [CPre11Compat]>;
 def CPre23Compat : DiagGroup<"pre-c23-compat">;
 def CPre23CompatPedantic : DiagGroup<"pre-c23-compat-pedantic",
                                      [CPre23Compat]>;
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 11b490a0928e60..c0dbc25a0c3265 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -165,6 +165,9 @@ def ext_c99_feature : Extension<
   "'%0' is a C99 extension">, InGroup<C99>;
 def ext_c11_feature : Extension<
   "'%0' is a C11 extension">, InGroup<C11>;
+def warn_c11_compat_keyword : Warning<
+  "'%0' is incompatible with C standards before C11">,
+  InGroup<CPre11Compat>, DefaultIgnore;
 def warn_c23_compat_keyword : Warning<
  "'%0' is incompatible with C standards before C23">,
  InGroup<CPre23Compat>, DefaultIgnore;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 69b9e837fe8bef..071520f535bc95 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1122,6 +1122,8 @@ class Parser : public CodeCompletionHandler {
   void checkCompoundToken(SourceLocation FirstTokLoc,
                           tok::TokenKind FirstTokKind, CompoundToken Op);
 
+  void diagnoseUseOfC11Keyword(const Token &Tok);
+
 public:
   //===--------------------------------------------------------------------===//
   // Scope manipulation
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 9640d7ee70d27f..0728113ba7c936 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3520,8 +3520,7 @@ void Parser::ParseDeclarationSpecifiers(
 
     // alignment-specifier
     case tok::kw__Alignas:
-      if (!getLangOpts().C11)
-        Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+      diagnoseUseOfC11Keyword(Tok);
       [[fallthrough]];
     case tok::kw_alignas:
       // _Alignas and alignas (C23, not C++) should parse the same way. The C++
@@ -4184,8 +4183,7 @@ void Parser::ParseDeclarationSpecifiers(
       isStorageClass = true;
       break;
     case tok::kw__Thread_local:
-      if (!getLangOpts().C11)
-        Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+      diagnoseUseOfC11Keyword(Tok);
       isInvalid = DS.SetStorageClassSpecThread(DeclSpec::TSCS__Thread_local,
                                                Loc, PrevSpec, DiagID);
       isStorageClass = true;
@@ -4245,8 +4243,7 @@ void Parser::ParseDeclarationSpecifiers(
       break;
     }
     case tok::kw__Noreturn:
-      if (!getLangOpts().C11)
-        Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+      diagnoseUseOfC11Keyword(Tok);
       isInvalid = DS.setFunctionSpecNoreturn(Loc, PrevSpec, DiagID);
       break;
 
@@ -4576,9 +4573,7 @@ void Parser::ParseDeclarationSpecifiers(
       //   If the _Atomic keyword is immediately followed by a left parenthesis,
       //   it is interpreted as a type specifier (with a type name), not as a
       //   type qualifier.
-      if (!getLangOpts().C11)
-        Diag(Tok, diag::ext_c11_feature) << Tok.getName();
-
+      diagnoseUseOfC11Keyword(Tok);
       if (NextToken().is(tok::l_paren)) {
         ParseAtomicSpecifier(DS);
         continue;
@@ -6154,8 +6149,7 @@ void Parser::ParseTypeQualifierListOpt(
     case tok::kw__Atomic:
       if (!AtomicAllowed)
         goto DoneWithTypeQuals;
-      if (!getLangOpts().C11)
-        Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+      diagnoseUseOfC11Keyword(Tok);
       isInvalid = DS.SetTypeQual(DeclSpec::TQ_atomic, Loc, PrevSpec, DiagID,
                                  getLangOpts());
       break;
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 7d0dbc4ac69490..62632b2d79792e 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -968,9 +968,9 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) {
   // Save the token name used for static assertion.
   const char *TokName = Tok.getName();
 
-  if (Tok.is(tok::kw__Static_assert) && !getLangOpts().C11)
-    Diag(Tok, diag::ext_c11_feature) << Tok.getName();
-  if (Tok.is(tok::kw_static_assert)) {
+  if (Tok.is(tok::kw__Static_assert))
+    diagnoseUseOfC11Keyword(Tok);
+  else if (Tok.is(tok::kw_static_assert)) {
     if (!getLangOpts().CPlusPlus) {
       if (getLangOpts().C23)
         Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 52cebdb6f64bac..f1417456ffe510 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1462,8 +1462,7 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind,
     return Res;
   }
   case tok::kw__Alignof:   // unary-expression: '_Alignof' '(' type-name ')'
-    if (!getLangOpts().C11)
-      Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+    diagnoseUseOfC11Keyword(Tok);
     [[fallthrough]];
   case tok::kw_alignof:    // unary-expression: 'alignof' '(' type-id ')'
   case tok::kw___alignof:  // unary-expression: '__alignof' unary-expression
@@ -3389,8 +3388,8 @@ ExprResult Parser::ParseStringLiteralExpression(bool AllowUserDefinedLiteral,
 /// \endverbatim
 ExprResult Parser::ParseGenericSelectionExpression() {
   assert(Tok.is(tok::kw__Generic) && "_Generic keyword expected");
-  if (!getLangOpts().C11)
-    Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+
+  diagnoseUseOfC11Keyword(Tok);
 
   SourceLocation KeyLoc = ConsumeToken();
   BalancedDelimiterTracker T(*this, tok::l_paren);
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 2dd4a73bfbc262..abc06468fc9d61 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2742,6 +2742,19 @@ bool Parser::parseMisplacedModuleImport() {
   return false;
 }
 
+void Parser::diagnoseUseOfC11Keyword(const Token& Tok) {
+  // Warn that this is a C11 extension if in an older mode or if in C++.
+  // Otherwise, warn that it is incompatible with standards before C11 if in
+  // C11 or later.
+  unsigned DiagId;
+  if (!getLangOpts().C11) {
+    DiagId = diag::ext_c11_feature;
+  } else {
+    DiagId = diag::warn_c11_compat_keyword;
+  }
+  Diag(Tok, DiagId) << Tok.getName();
+}
+
 bool BalancedDelimiterTracker::diagnoseOverflow() {
   P.Diag(P.Tok, diag::err_bracket_depth_exceeded)
     << P.getLangOpts().BracketDepth;
diff --git a/clang/test/Sema/c11-keywords.c b/clang/test/Sema/c11-keywords.c
new file mode 100644
index 00000000000000..2621e1a1a8595d
--- /dev/null
+++ b/clang/test/Sema/c11-keywords.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 %s -std=c11 -fsyntax-only -verify=compat -Wpre-c11-compat
+// RUN: %clang_cc1 %s -std=c99 -fsyntax-only -verify=ext -pedantic
+// RUN: %clang_cc1 %s -std=c11 -fsyntax-only -verify=good
+// RUN: %clang_cc1 -x c++ %s -fsyntax-only -verify=ext -pedantic
+
+// good-no-diagnostics
+
+extern _Noreturn void exit(int); /* compat-warning {{'_Noreturn' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Noreturn' is a C11 extension}}
+                                  */
+
+void func(void) {
+  static _Thread_local int tl;   /* compat-warning {{'_Thread_local' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Thread_local' is a C11 extension}}
+                                  */
+  _Alignas(8) char c;            /* compat-warning {{'_Alignas' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Alignas' is a C11 extension}}
+                                  */
+  _Atomic int i1;                /* compat-warning {{'_Atomic' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Atomic' is a C11 extension}}
+                                  */
+  _Atomic(int) i2;               /* compat-warning {{'_Atomic' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Atomic' is a C11 extension}}
+                                  */
+
+  _Static_assert(1, "");         /* compat-warning {{'_Static_assert' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Static_assert' is a C11 extension}}
+                                  */
+
+  (void)_Generic(1, int : 1);    /* compat-warning {{'_Generic' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Generic' is a C11 extension}}
+                                  */
+  (void)_Alignof(int);           /* compat-warning {{'_Alignof' is incompatible with C standards before C11}}
+                                    ext-warning {{'_Alignof' is a C11 extension}}
+                                  */
+}
+

@llvmbot llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Feb 16, 2024
Copy link

github-actions bot commented Feb 16, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

// Otherwise, warn that it is incompatible with standards before C11 if in
// C11 or later.
unsigned DiagId;
if (!getLangOpts().C11) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use curleys on 1 liners

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

} else {
DiagId = diag::warn_c11_compat_keyword;
}
Diag(Tok, DiagId) << Tok.getName();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think it is worth the DiagId pattern here, I think just a Diag + Ternary is probably fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had originally structured it this way because I thought we might want to give a different diagnostic in C++ mode (where we call these C11 extensions), but then I realized that diagnostic is reasonable as-is. I'll change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator Author

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made both changes (thanks) and moved the test file to a more appropriate location.

// Otherwise, warn that it is incompatible with standards before C11 if in
// C11 or later.
unsigned DiagId;
if (!getLangOpts().C11) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

} else {
DiagId = diag::warn_c11_compat_keyword;
}
Diag(Tok, DiagId) << Tok.getName();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@AaronBallman AaronBallman merged commit 97434cb into llvm:main Feb 16, 2024
@AaronBallman AaronBallman deleted the aballman-c11-keywords branch February 16, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c11 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer 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