rsmith added inline comments.

================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874-876
+    if (!getLangOpts().CPlusPlus)
+      Diag(Tok, diag::warn_cxx_static_assert_in_c)
+          << FixItHint::CreateReplacement(Tok.getLocation(), "_Static_assert");
----------------
I don't think this diagnostic is useful as-is: on Windows, including 
`<assert.h>` doesn't help because it doesn't `#define static_assert`. And 
people hitting this also can't switch to using `_Static_assert`, because MSVC 
doesn't provide it, only `static_assert`.

If we want to warn here, we could perhaps check whether `<assert.h>` has been 
included, but getting that check correct across PCH / modules is not 
straightforward. (If we knew what include guard the CRT's `assert.h` used (if 
any), I guess we could check whether that's defined, but that'd be a bit of a 
hack.) But I'm somewhat inclined to think we don't have a good way to 
distinguish between the good cases and the bad ones, so we shouldn't warn. 
Hopefully MS will fix their CRT at some point and we can stop providing this 
compatibility hack entirely (or start warning on it by default).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95396/new/

https://reviews.llvm.org/D95396

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to