[clang] 8da0903 - Improve static_assert/_Static_assert diagnostics
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 3 05:48:43 PST 2021
Author: Aaron Ballman
Date: 2021-03-03T08:48:27-05:00
New Revision: 8da090381d567d0ec555840f6b2a651d2997e4b3
URL: https://github.com/llvm/llvm-project/commit/8da090381d567d0ec555840f6b2a651d2997e4b3
DIFF: https://github.com/llvm/llvm-project/commit/8da090381d567d0ec555840f6b2a651d2997e4b3.diff
LOG: Improve static_assert/_Static_assert diagnostics
Our diagnostics relating to static assertions were 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.
Added:
clang/test/Parser/static_assert.c
clang/test/Preprocessor/static_assert-already-defined.c
clang/test/Preprocessor/static_assert.c
Modified:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/lib/Lex/PPDirectives.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/test/Sema/static-assert.c
clang/test/SemaCXX/static-assert.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 81d78c69cc44..14632210c818 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -253,6 +253,11 @@ def : DiagGroup<"c++1z-compat-mangling", [CXX17CompatMangling]>;
// 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<"pre-c2x-compat">;
+def CPre2xCompatPedantic : DiagGroup<"pre-c2x-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",
@@ -1074,6 +1079,8 @@ def MicrosoftAnonTag : DiagGroup<"microsoft-anon-tag">;
def MicrosoftCommentPaste : DiagGroup<"microsoft-comment-paste">;
def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">;
def MicrosoftInaccessibleBase : DiagGroup<"microsoft-inaccessible-base">;
+def MicrosoftStaticAssert : DiagGroup<"microsoft-static-assert">;
+
// Aliases.
def : DiagGroup<"msvc-include", [MicrosoftInclude]>;
// -Wmsvc-include = -Wmicrosoft-include
@@ -1089,7 +1096,7 @@ def Microsoft : DiagGroup<"microsoft",
MicrosoftRedeclareStatic, MicrosoftEnumForwardReference, MicrosoftGoto,
MicrosoftFlexibleArray, MicrosoftExtraQualification, MicrosoftCast,
MicrosoftConstInit, MicrosoftVoidPseudoDtor, MicrosoftAnonTag,
- MicrosoftCommentPaste, MicrosoftEndOfFile,
+ MicrosoftCommentPaste, MicrosoftEndOfFile, MicrosoftStaticAssert,
MicrosoftInconsistentDllImport]>;
def ClangClPch : DiagGroup<"clang-cl-pch">;
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index da228cc4be7a..ac7eb8e0764e 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -424,12 +424,21 @@ def err_bool_redeclaration : Error<
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 ext_ms_static_assert : ExtWarn<
+ "use of 'static_assert' without inclusion of <assert.h> is a Microsoft "
+ "extension">, InGroup<MicrosoftStaticAssert>;
+def ext_cxx_static_assert_no_message : ExtWarn<
+ "'static_assert' with no message is a C++17 extension">, InGroup<CXX17>;
+def ext_c_static_assert_no_message : ExtWarn<
+ "'_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<
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index d6b03d85913d..c854d3e9c02b 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2870,6 +2870,23 @@ void Preprocessor::HandleDefineDirective(
// If the callbacks want to know, tell them about the macro definition.
if (Callbacks)
Callbacks->MacroDefined(MacroNameTok, MD);
+
+ // If we're in MS compatibility mode and the macro being defined is the
+ // assert macro, implicitly add a macro definition for static_assert to work
+ // around their broken assert.h header file in C. Only do so if there isn't
+ // already a static_assert macro defined.
+ if (!getLangOpts().CPlusPlus && getLangOpts().MSVCCompat &&
+ MacroNameTok.getIdentifierInfo()->isStr("assert") &&
+ !isMacroDefined("static_assert")) {
+ MacroInfo *MI = AllocateMacroInfo(SourceLocation());
+
+ Token Tok;
+ Tok.startToken();
+ Tok.setKind(tok::kw__Static_assert);
+ Tok.setIdentifierInfo(getIdentifierInfo("_Static_assert"));
+ MI->AddTokenToBody(Tok);
+ (void)appendDefMacroDirective(getIdentifierInfo("static_assert"), MI);
+ }
}
/// HandleUndefDirective - Implements \#undef.
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 44c072b8987f..df71ba34cdd9 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -880,8 +880,13 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd){
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::ext_ms_static_assert)
+ << FixItHint::CreateReplacement(Tok.getLocation(), "_Static_assert");
+ else
+ Diag(Tok, diag::warn_cxx98_compat_static_assert);
+ }
SourceLocation StaticAssertLoc = ConsumeToken();
@@ -902,11 +907,17 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd){
ExprResult AssertMessage;
if (Tok.is(tok::r_paren)) {
+ unsigned DiagVal;
if (getLangOpts().CPlusPlus17)
- Diag(Tok, diag::warn_cxx14_compat_static_assert_no_message);
+ 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
- Diag(Tok, diag::ext_static_assert_no_message)
- << getStaticAssertNoMessageFixIt(AssertExpr.get(), Tok.getLocation());
+ DiagVal = diag::ext_c_static_assert_no_message;
+ Diag(Tok, DiagVal) << getStaticAssertNoMessageFixIt(AssertExpr.get(),
+ Tok.getLocation());
} else {
if (ExpectAndConsume(tok::comma)) {
SkipUntil(tok::semi);
diff --git a/clang/test/Parser/static_assert.c b/clang/test/Parser/static_assert.c
new file mode 100644
index 000000000000..eaae2adbe380
--- /dev/null
+++ b/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 -Wpre-c2x-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
+
+// cxx17-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 {{use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension}}
+#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}} \
+ // c99-warning {{'_Static_assert' with no message is a C2x extension}} \
+ // cxx98-pedantic-warning {{'static_assert' with no message is a C++17 extension}} \
+ // cxx98-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}}
diff --git a/clang/test/Preprocessor/static_assert-already-defined.c b/clang/test/Preprocessor/static_assert-already-defined.c
new file mode 100644
index 000000000000..1308e123100c
--- /dev/null
+++ b/clang/test/Preprocessor/static_assert-already-defined.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -DFIRST_WAY -E -dM %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -DFIRST_WAY -fms-compatibility -E -dM %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -dM %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -fms-compatibility -E -dM %s | FileCheck --strict-whitespace %s
+
+// If the assert macro is defined in MS compatibility mode in C, we
+// automatically inject a macro definition for static_assert. Test that the
+// macro is not added if there is already a definition of static_assert to
+// ensure that we don't re-define the macro in the event the Microsoft assert.h
+// header starts to define the macro some day (or the user defined their own
+// macro with the same name). Test that the order of the macro definitions does
+// not matter to the behavior.
+
+#ifdef FIRST_WAY
+#define static_assert 12
+#define assert
+#else
+#define assert
+#define static_assert 12
+#endif
+
+CHECK: #define static_assert 12
+
diff --git a/clang/test/Preprocessor/static_assert.c b/clang/test/Preprocessor/static_assert.c
new file mode 100644
index 000000000000..7fa9975e1d46
--- /dev/null
+++ b/clang/test/Preprocessor/static_assert.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -E -dM %s | FileCheck --strict-whitespace --check-prefix=NOMS %s
+// RUN: %clang_cc1 -fms-compatibility -E -dM %s | FileCheck --strict-whitespace --check-prefix=MS %s
+
+// If the assert macro is defined in MS compatibility mode in C, we
+// automatically inject a macro definition for static_assert. Test that the
+// macro is properly added to the preprocessed output. This allows us to
+// diagonse use of the static_assert keyword when <assert.h> has not been
+// included while still being able to compile preprocessed code.
+#define assert
+
+MS: #define static_assert _Static_assert
+NOMS-NOT: #define static_assert _Static_assert
diff --git a/clang/test/Sema/static-assert.c b/clang/test/Sema/static-assert.c
index 9105f2366985..5630bde64f93 100644
--- a/clang/test/Sema/static-assert.c
+++ b/clang/test/Sema/static-assert.c
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fms-compatibility -DMS -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fms-compatibility -DMS -fsyntax-only -verify=expected,ms %s
// RUN: %clang_cc1 -std=c99 -pedantic -fsyntax-only -verify=expected,ext %s
// RUN: %clang_cc1 -xc++ -std=c++11 -pedantic -fsyntax-only -verify=expected,ext,cxx %s
@@ -13,7 +13,7 @@ _Static_assert(0, "0 is nonzero"); // expected-error {{static_assert failed "0 i
// ext-warning {{'_Static_assert' is a C11 extension}}
#ifdef MS
-static_assert(1, "1 is nonzero");
+static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension}}
#endif
void foo(void) {
@@ -21,7 +21,7 @@ void foo(void) {
_Static_assert(0, "0 is nonzero"); // expected-error {{static_assert failed "0 is nonzero"}} \
// ext-warning {{'_Static_assert' is a C11 extension}}
#ifdef MS
- static_assert(1, "1 is nonzero");
+ static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
#endif
}
@@ -34,7 +34,7 @@ struct A {
_Static_assert(0, "0 is nonzero"); // expected-error {{static_assert failed "0 is nonzero"}} \
// ext-warning {{'_Static_assert' is a C11 extension}}
#ifdef MS
- static_assert(1, "1 is nonzero");
+ static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
#endif
};
@@ -58,3 +58,15 @@ typedef UNION(char, short) U3; // expected-error {{static_assert failed due to r
// ext-warning 3 {{'_Static_assert' is a C11 extension}}
typedef UNION(float, 0.5f) U4; // expected-error {{expected a type}} \
// ext-warning 3 {{'_Static_assert' is a C11 extension}}
+
+// After defining the assert macro in MS-compatibility mode, we should
+// no longer warn about including <assert.h> under the assumption the
+// user already did that.
+#ifdef MS
+#define assert(expr)
+static_assert(1, "1 is nonzero"); // ok
+
+// But we should still warn if the user did something bonkers.
+#undef static_assert
+static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
+#endif
diff --git a/clang/test/SemaCXX/static-assert.cpp b/clang/test/SemaCXX/static-assert.cpp
index a21cc874b566..dfc06331e51f 100644
--- a/clang/test/SemaCXX/static-assert.cpp
+++ b/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}}
More information about the cfe-commits
mailing list