aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, jyknight.
aaron.ballman requested review of this revision.

I noticed that our diagnostics relating to static assertions are a bit 
confused. For instance, when in MS compatibility mode in C (where we accept 
`static_assert` even without including `<assert.h>`), we would fail to warn the 
user that they were using the wrong spelling (even in pedantic mode), we were 
missing a compatibility warning about using `_Static_assert` in earlier 
standards modes, diagnostics for the optional message were not reflected in C 
as they were in C++, etc.

This patch improves the diagnostic functionality in both C and C++ mode. It 
adds `-Wc89-c99-c11-c17-compat` and `-Wc89-c99-c11-c17-compat-pedantic` 
diagnostics groups and adds new C-specific diagnostics for a static assertion 
without a diagnostic message.


https://reviews.llvm.org/D95396

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/static_assert.c
  clang/test/SemaCXX/static-assert.cpp

Index: clang/test/SemaCXX/static-assert.cpp
===================================================================
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -pedantic -triple=x86_64-linux-gnu
 
 int f(); // expected-note {{declared here}}
 
Index: clang/test/Parser/static_assert.c
===================================================================
--- /dev/null
+++ clang/test/Parser/static_assert.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -Wc89-c99-c11-c17-compat -verify=c2x-compat %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -verify=c99-pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify=cxx17 -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -pedantic -verify=cxx17-pedantic -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -verify=cxx98 -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -pedantic -verify=cxx98-pedantic -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -Wc++98-c++11-c++14-compat -verify=cxx17-compat -x c++ %s
+
+// c99-no-diagnostics
+// cxx17-no-diagnostics
+// cxx98-no-diagnostics
+
+#ifdef TEST_SPELLING
+// Only test the C++ spelling in C mode in some of the tests, to reduce the
+// amount of diagnostics to have to check. This spelling is allowed in MS-
+// compatibility mode in C, but otherwise produces errors.
+static_assert(1, ""); // c2x-error {{expected parameter declarator}} \
+                      // c2x-error {{expected ')'}} \
+                      // c2x-note {{to match this '('}} \
+                      // c2x-warning {{type specifier missing, defaults to 'int'}} \
+                      // c2x-ms-warning {{'static_assert' is spelled '_Static_assert' in C; consider including <assert.h>}}
+#endif
+
+// We support _Static_assert as an extension in older C modes and in all C++
+// modes, but only as a pedantic warning.
+_Static_assert(1, ""); // c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+                       // cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+                       // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
+
+// _Static_assert without a message has more complex diagnostic logic:
+//   * In C++17 or C2x mode, it's supported by default.
+//   * But there is a special compat warning flag to warn about portability to
+//     older standards.
+//   * In older standard pedantic modes, warn about supporting without a
+//     message as an extension.
+_Static_assert(1); // c99-pedantic-warning {{'_Static_assert' with no message is a C2x extension}} \
+                   // cxx98-pedantic-warning {{'static_assert' with no message is a C++17 extension}} \
+                   // c2x-compat-warning {{'_Static_assert' with no message is incompatible with C standards before C2x}} \
+                   // cxx17-compat-warning {{'static_assert' with no message is incompatible with C++ standards before C++17}} \
+                   // c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+                   // cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+                   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -871,8 +871,13 @@
 
   if (Tok.is(tok::kw__Static_assert) && !getLangOpts().C11)
     Diag(Tok, diag::ext_c11_feature) << Tok.getName();
-  if (Tok.is(tok::kw_static_assert))
-    Diag(Tok, diag::warn_cxx98_compat_static_assert);
+  if (Tok.is(tok::kw_static_assert)) {
+    if (!getLangOpts().CPlusPlus)
+      Diag(Tok, diag::warn_cxx_static_assert_in_c)
+          << FixItHint::CreateReplacement(Tok.getLocation(), "_Static_assert");
+    else
+      Diag(Tok, diag::warn_cxx98_compat_static_assert);
+  }
 
   SourceLocation StaticAssertLoc = ConsumeToken();
 
@@ -893,12 +898,19 @@
 
   ExprResult AssertMessage;
   if (Tok.is(tok::r_paren)) {
-    Diag(Tok, getLangOpts().CPlusPlus17
-                  ? diag::warn_cxx14_compat_static_assert_no_message
-                  : diag::ext_static_assert_no_message)
-      << (getLangOpts().CPlusPlus17
-              ? FixItHint()
-              : FixItHint::CreateInsertion(Tok.getLocation(), ", \"\""));
+    unsigned DiagVal;
+    if (getLangOpts().CPlusPlus17)
+      DiagVal = diag::warn_cxx14_compat_static_assert_no_message;
+    else if (getLangOpts().CPlusPlus)
+      DiagVal = diag::ext_cxx_static_assert_no_message;
+    else if (getLangOpts().C2x)
+      DiagVal = diag::warn_c17_compat_static_assert_no_message;
+    else
+      DiagVal = diag::ext_c_static_assert_no_message;
+    Diag(Tok, DiagVal) << ((getLangOpts().CPlusPlus17 || getLangOpts().C2x)
+                               ? FixItHint()
+                               : FixItHint::CreateInsertion(Tok.getLocation(),
+                                                            ", \"\""));
   } else {
     if (ExpectAndConsume(tok::comma)) {
       SkipUntil(tok::semi);
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -424,12 +424,21 @@
 def warn_cxx98_compat_static_assert : Warning<
   "static_assert declarations are incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
-def ext_static_assert_no_message : ExtWarn<
-  "static_assert with no message is a C++17 extension">, InGroup<CXX17>;
+def warn_cxx_static_assert_in_c : Warning<
+  "'static_assert' is spelled '_Static_assert' in C; consider including "
+  "<assert.h>">, InGroup<DiagGroup<"static-assert-extension">>;
+def ext_cxx_static_assert_no_message : Extension<
+  "'static_assert' with no message is a C++17 extension">, InGroup<CXX17>;
+def ext_c_static_assert_no_message : Extension<
+  "'_Static_assert' with no message is a C2x extension">, InGroup<C2x>;
 def warn_cxx14_compat_static_assert_no_message : Warning<
-  "static_assert with no message is incompatible with C++ standards before "
+  "'static_assert' with no message is incompatible with C++ standards before "
   "C++17">,
   DefaultIgnore, InGroup<CXXPre17Compat>;
+def warn_c17_compat_static_assert_no_message : Warning<
+  "'_Static_assert' with no message is incompatible with C standards before "
+  "C2x">,
+  DefaultIgnore, InGroup<CPre2xCompat>;
 def err_function_definition_not_allowed : Error<
   "function definition is not allowed here">;
 def err_expected_end_of_enumerator : Error<
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -253,6 +253,11 @@
 // Name of this warning in GCC.
 def NoexceptType : DiagGroup<"noexcept-type", [CXX17CompatMangling]>;
 
+// Warnings for C2x code which is not compatible with prior C standards.
+def CPre2xCompat : DiagGroup<"c89-c99-c11-c17-compat">;
+def CPre2xCompatPedantic : DiagGroup<"c89-c99-c11-c17-compat-pedantic",
+                                     [CPre2xCompat]>;
+
 // Warnings for C++1y code which is not compatible with prior C++ standards.
 def CXXPre14Compat : DiagGroup<"c++98-c++11-compat">;
 def CXXPre14CompatPedantic : DiagGroup<"c++98-c++11-compat-pedantic",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to