[PATCH] D131683: Diagnosing the Future Keywords

Thu Aug 11 07:48:51 PDT 2022

erichkeane added a comment.

I believe you need tests here desperately.  I also believe the `define` logic that you have here isn't going to be sufficient for the C future flags thanks to it getting some C keywords.

================
Comment at: clang/include/clang/Basic/TokenKinds.def:387
 // C++11 keywords
-CXX11_KEYWORD(alignas               , 0)
+CXX11_KEYWORD(alignas               , KEYC23)
 // alignof and _Alignof return the required ABI alignment
----------------
Hmm... this looks like it is going to be troublesome for the 'future' feature.  Can you make sure you have tests for all of these?  

================
Comment at: clang/lib/Lex/Preprocessor.cpp:799
+#define C23_KEYWORD(NAME, FLAGS) .Case(#NAME, diag::warn_c23_keyword)
+#define CXX11_KEYWORD(NAME, FLAGS) .Case(#NAME, diag::warn_c23_keyword)
+#include "clang/Basic/TokenKinds.def"
----------------
This isn't right at all.  We should be looking through the list of flags instead of trying to assume that cxx11 keywords here are all 'future' C keywords.  First, this isn't true.  Second, even if it was, it is really fragile.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683