[PATCH] D131683: Diagnosing the Future Keywords

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Looks about right, please see if you can do more work on the static-assert test to make sure we don't lose other testing behavior, AND see the other comment.



================
Comment at: clang/lib/Lex/Preprocessor.cpp:847
   if (II.isFutureCompatKeyword() && !DisableMacroExpansion) {
-    Diag(Identifier, getFutureCompatDiagKind(II, getLangOpts()))
+    IdentifierTable IT;
+    Diag(Identifier, IT.getFutureCompatDiagKind(II, getLangOpts()))
----------------
Don't create a new one, do `getIdentifierTable()` to get the active identifier table.


================
Comment at: clang/test/Parser/static_assert.c:16
 // 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-error {{a type specifier is required for all declarations}} \
-                      // c2x-ms-warning {{use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension}}
+static_assert(1, ""); 
 #endif
----------------
So we want to make sure we still get the ms-warnings, so this also needs to be validated in C++17 mode as well, which should ALSO give us a 'future keyword' diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683