aaron.ballman added a comment.

Thank you for working on this!



================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:696-698
+def ext_c2x_pp_directive : Extension<
+  "%select{#elifdef|#elifndef}0 is a C2x extension">,
+  InGroup<CPre2xCompatPedantic>;
----------------
I think we need two diagnostics, one for C2x and one for C++2b 
(https://wg21.link/p2334 was adopted for C++23). Each of these diagnostics 
should come in a pair:
```
def warn_cxx20_compat_pp_directive : Warning<
  "use of a '#%select{elifdef|elifndef}0' directive is incompatible with C++ 
standards before C++2b",
  InGroup<CXXPre2bCompat>, DefaultIgnore;
def ext_cxx20_pp_directive : ExtWarn<
  "use of a '#%select{elifdef|elifndef}0' directive is a C++2b extension",
  InGroup<CXX2b>;
```
and similar for C, except with wording about C standards and in the C warning 
groups.


================
Comment at: clang/lib/Lex/PPDirectives.cpp:3262-3263
+    case PED_Elifndef:
+      if (!getLangOpts().C2x)
+        Diag(ElifToken, diag::ext_c2x_pp_directive) << DirKind - 1;
+      break;
----------------
And then here, you would do something like:
```
unsigned DiagID;
if (getLangOpts().CPlusPlus)
  DiagID = getLangOpts().CPlusPlus2b ? diag::warn_cxx20_compat_pp_directive : 
diag::ext_cxx20_pp_directive;
else
  DiagID = getLangOpts().C2x ? diag::warn_c17_compat_pp_directive : 
diag::ext_c2b_pp_directive;

Diag(ElifToken, DiagID) << ...;
```


================
Comment at: clang/test/Preprocessor/ext-c2x-pp-directive.c:14
+
+// expected-warning {{ISO C requires a translation unit to contain at least 
one declaration}}
----------------
Might as well add a declaration so that we don't have to expect the diagnostic.

Also, this will need tests for both forms of the diagnostics, C++ tests, and 
it'd probably be good to verify the behavior when the previous conditional is 
skipped. e.g.,
```
#if 0
#elifdef WHATEVER // We should still warn on this; we wouldn't if we didn't 
recognize it as a conditional
#endif
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

Reply via email to