[PATCH] D95396: Improve static_assert/_Static_assert diagnostics
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 18 15:03:54 PST 2021
rsmith added a comment.
@rnk / @thakis Can you take a look at this and see if you're happy with this "defining `assert` implicitly defines `static_assert`" approach?
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:427-429
+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">>;
----------------
Perhaps:
```
def ext_ms_static_assert : ExtWarn<
"use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension">,
"<assert.h>">, InGroup<MicrosoftStaticAssert>;
```
I think `MicrosoftStaticAssert` should also be a subgroup of the `Microsoft` (`-Wmicrosoft`) warning group.
================
Comment at: clang/lib/Lex/PPDirectives.cpp:2884
+ Tok.setKind(tok::kw__Static_assert);
+ Tok.setIdentifierInfo(getIdentifierInfo("_Static_assert"));
+ MI->AddTokenToBody(Tok);
----------------
Do we need to give the expansion token a source location? What do diagnostics pointing at this token look like?
================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:881
+ // MSVC's assert.h does not provide such a macro definition.
+ if (!getLangOpts().CPlusPlus && !PP.isMacroDefined("assert"))
+ Diag(Tok, diag::warn_cxx_static_assert_in_c)
----------------
I don't think we need the `isMacroDefined` check here; we will have a `static_assert` macro in that case, so we won't see a `kw_static_assert` token. Checking for `assert` being defined means we won't warn on this, and we should:
```
#include <assert.h>
#undef static_assert
static_assert(1, "");
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95396/new/
https://reviews.llvm.org/D95396
More information about the cfe-commits
mailing list