ken-matsui added inline comments.
================ 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>; ---------------- aaron.ballman wrote: > ken-matsui wrote: > > aaron.ballman wrote: > > > ken-matsui wrote: > > > > aaron.ballman wrote: > > > > > 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. > > > > I thought I had to use `Extension` here, but what is the difference > > > > between `Warning`, `ExtWarn`, and `Extension`? > > > Great question! > > > > > > `Extension` diagnostics are warnings that are enabled via `-pedantic` but > > > otherwise off by default. > > > `ExtWarn` diagnostics are warnings that are enabled via `-pedantic` but > > > are also on by default. > > > `Warning` diagnostics are warnings. You can use things like > > > `DefaultIgnore` or `DefaultError` to control the default behavior of the > > > warning. > > > `Error` diagnostics are errors. > > > > > > You'll typically use `Extension` or `ExtWarn` for things that are > > > extensions of some sort, but which one you use depends on how important > > > the warning is. We've traditionally handled "use of whatever is a C++NN > > > extension" warnings as `ExtWarn` whereas the "use of whatever is > > > incompatible with C++ standards before C++NN" warnings are usually > > > `Extension`. > > In this case, `compat_pp_directive` for C++2b & C2x uses `Warning`, but why > > did you choose it rather than `Extension`? > Oh shoot, I misspoke and said `ExtWarn` when I meant `Warning` that's default > ignored. Sorry about that, and thank you for asking for clarification! > > I picked that because it's the pattern used by many other diagnostic pairs: > ``` > def ext_inline_variable : ExtWarn< > "inline variables are a C++17 extension">, InGroup<CXX17>; > def warn_cxx14_compat_inline_variable : Warning< > "inline variables are incompatible with C++ standards before C++17">, > DefaultIgnore, InGroup<CXXPre17Compat>; > > def ext_for_range_begin_end_types_differ : ExtWarn< > "'begin' and 'end' returning different types (%0 and %1) is a C++17 > extension">, > InGroup<CXX17>; > def warn_for_range_begin_end_types_differ : Warning< > "'begin' and 'end' returning different types (%0 and %1) is incompatible " > "with C++ standards before C++17">, InGroup<CXXPre17Compat>, DefaultIgnore; > > def ext_constexpr_body_invalid_stmt : ExtWarn< > "use of this statement in a constexpr %select{function|constructor}0 " > "is a C++14 extension">, InGroup<CXX14>; > def warn_cxx11_compat_constexpr_body_invalid_stmt : Warning< > "use of this statement in a constexpr %select{function|constructor}0 " > "is incompatible with C++ standards before C++14">, > InGroup<CXXPre14Compat>, DefaultIgnore; > > def ext_constexpr_function_try_block_cxx20 : ExtWarn< > "function try block in constexpr %select{function|constructor}0 is " > "a C++20 extension">, InGroup<CXX20>; > def warn_cxx17_compat_constexpr_function_try_block : Warning< > "function try block in constexpr %select{function|constructor}0 is " > "incompatible with C++ standards before C++20">, > InGroup<CXXPre20Compat>, DefaultIgnore; > ``` Oh, that makes sense. Thank you for your explanation! 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