rsmith added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticGroups.td:269 +def CXXPre2BCompatPedantic : + DiagGroup<"c++98-c++11-c++14-c++17-c++20-compat-pedantic", [CXXPre2BCompat]>; ---------------- rjmccall wrote: > Quuxplusone wrote: > > aaron.ballman wrote: > > > rjmccall wrote: > > > > Uh, I think we're a couple standard releases past the point at which we > > > > should have reconsidered this schema. I guess the problem is that we > > > > can't say `-Wpre-c++23-compat` without jumping the gun. Is there a > > > > problem with `-Wc++20-compat` and then having the earlier warning > > > > groups imply the later ones? That seems to be what we do with > > > > `-Wc++98-compat`; did we abandon that approach intentionally? > > > @rsmith may have more background here. I was following the pattern > > > already in the file, but I tend to agree that this pattern is not leading > > > us somewhere good. FWIW, I ran into a similar situation with this on the > > > C side of things in D95396, so we should probably be consistent there too. > > My understanding is that the //command-line user// is expected to pass > > - `clang++ -std=c++20 -Wc++11-compat` to indicate "I want //actually// to > > compile in C++20 mode, but give me warnings about anything that would > > prevent compiling in C++11 mode" > > - `clang++ -std=c++17 -Wc++14-compat` to indicate "I want //actually// to > > compile in C++17 mode, but give me warnings about anything that would > > prevent compiling in C++14 mode" > > - `clang++ -std=c++14 -Wc++20-compat` to indicate "I want //actually// to > > compile in C++14 mode, but give me warnings about anything that would > > prevent compiling in C++20 mode" — EXCEPT that I think this is not > > supported. My impression is that forward-compatibility warnings are > > generally just rolled into `-Wall` and not handled separately beyond that? > > > > I don't think any human user is expected to pass > > `-Wc++98-c++11-c++14-c++17-c++20-compat` by hand; it's just an internal > > name for a particular subset of `-Wc++98-compat`. > > > > IOW, we could choose a new naming scheme for it, but that would be a purely > > internal change that won't affect how command-line users interact with > > Clang at all (for better and for worse). > Diagnostic groups can both directly contain diagnostics and imply other > diagnostic groups, so I don't think there's any reason to make a dedicated > group just to contain the new diagnostics in e.g. `-Wc++14-compat` except to > allow someone turn on those warnings separately. And it does show up to > users as the warning group under `-fdiagnostics-show-option` (which is the > default). @Quuxplusone's comment describes the intent. `-std=c++20 -Wc++14-compat` should give a more or less complete list of reasons why the code would not compile in C++14 (at least on the language side; we don't check for stdlib compatibility). The other direction -- `-std=c++11 -Wc++14-compat` -- is more of a best-effort check for things that we've seen cause problems in practice and can easily detect. (As a consequence, I don't think there's any subset/superset relation between `-Wc++X-compat` and `-Wc++Y-compat`.) I'd be happy to see these groups renamed to `-Wpre-c++20-compat` or similar. Warning group synonyms are relatively cheap, so I wouldn't be worried about adding a `-Wpre-c++2b-compat` now and renaming it to `-Wpre-c++23-compat` flag later. (As an aside, it'd be handy if there were some way to mark a `DiagGroup` as existing only for grouping purposes, so that we could avoid exposing a `-W` flag for cases where groups are added for internal reasons.) ================ Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1023 +// earlier C++ versions. +def CXX2B : DiagGroup<"c++2b-extensions">; + ---------------- I think we generally use a lowercase letter here, so `CXX2b`. (For example, see the `C2x` group.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95691/new/ https://reviews.llvm.org/D95691 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
