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: > rsmith wrote: > > 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.) > Okay. It looks like `-Wc++X-compat` is consistently (1) all the > this-feature-used-not-to-exist diagnostics from C++X and later plus (2) > warnings about deprecation and semantic changes introduced by exactly version > X. This seems like an unfortunate pairing, basically caused by the option > names not being very clear about what kind of compatibility they mean. If we > want @Quuxplusone's interpretation, which I agree is a natural human > interpretation of those command lines, we'll need special support for it in > diagnostic-option handling, so that we include specific diagnostics based on > the relationship between the option and the language version. > > There is a natural subset relationship between the > this-feature-used-not-to-exist groups; we're just not taking advantage of it > at all. (2) sounds like a bug. Maybe we should add `CXXPostXYCompat` groups, symmetric to the `CXXPreXYCompat` groups, to better handle that? I'm not sure about the need for special support in diagnostic option handling -- we don't ever produce a "you're using a feature that wasn't in an older standard mode" warning unless we're in the newer mode, and we don't ever produce a "you're using a feature that will change / go away in a newer standard mode" warning unless we're in the older mode. I think it'd be reasonable to take advantage of the subset relationships. Back when there were only a couple of C++ language standards we cared about, the difference between linear and quadratic growth didn't really matter, but we're past that point now. 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
