aaron.ballman 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]>; ---------------- rsmith wrote: > 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. In terms of what's reasonable for this patch, what's our path forward? It sounds like we'd like to see `CXXPre2bCompat` that's spelled `-Wpre-c++2b-compat` (and same for pedantic), and then we'll add aliases for the other language standard modes in a follow-up? 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
