mirror of
https://github.com/llvm/llvm-project.git
synced 2025-04-19 09:06:40 +00:00
f01b56ffb3
This introduces some tablegen helpers for defining compatibility warnings. The main aim of this is to both simplify adding new compatibility warnings as well as to unify the naming of compatibility warnings. I’ve refactored ~half of the compatiblity warnings (that follow the usual scheme) in `DiagnosticSemaKinds.td` for illustration purposes and also to simplify/unify the wording of some of them (I also corrected a typo in one of them as a drive-by fix). I haven’t (yet) migrated *all* warnings even in that one file, and there are some more specialised ones for which the scheme I’ve established here doesn’t work (e.g. because they’re warning+error instead of warning+extwarn; however, warning+extension *is* supported), but the point of this isn’t to implement *all* compatibility-related warnings this way, only to make the common case a bit easier to handle. This currently also only handles C++ compatibility warnings, but it should be fairly straight-forward to extend the tablegen code so it can also be used for C compatibility warnings (if this gets merged, I’m planning to do that in a follow-up pr). The vast majority of compatibility warnings are emitted by writing ```c++ Diag(Loc, getLangOpts().CPlusPlusYZ ? diag::ext_... : diag::warn_...) ``` in accordance with which I’ve chosen the following naming scheme: ```c++ Diag(Loc, getLangOpts().CPlusPlusYZ ? diag::compat_cxxyz_foo : diag::compat_pre_cxxyz_foo) ``` That is, for a warning about a C++20 feature—i.e. C++≤17 compatibility—we get: ```c++ Diag(Loc, getLangOpts().CPlusPlus20 ? diag::compat_cxx20_foo : diag::compat_pre_cxx20_foo) ``` While there is an argument to be made against writing ‘`compat_cxx20`’ here since is technically a case of ‘C++17 compatibility’ and not ‘C++20 compatibility’, I at least find this easier to reason about, because I can just write the same number 3 times instead of having to use `ext_cxx20_foo` but `warn_cxx17_foo`. Instead, I like to read this as a warning about the ‘compatibility *of* a C++20 feature’ rather than ‘*with* C++17’. I also experimented with moving all compatibility warnings to a separate file, but 1. I don’t think it’s worth the effort, and 2. I think it hurts compile times a bit because at least in my testing I felt that I had to recompile more code than if we just keep e.g. Sema-specific compat warnings in the Sema diagnostics file. Instead, I’ve opted to put them all in the same place within any one file; currently this is a the very top but I don’t really have strong opinions about this.