[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

Jordan Rupprecht via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 7 18:01:10 PDT 2023


rupprecht added a comment.

In D150226#4401188 <https://reviews.llvm.org/D150226#4401188>, @jyknight wrote:

> In D150226#4400782 <https://reviews.llvm.org/D150226#4400782>, @rupprecht wrote:
>
>> As a general question/feature request: is there a way to have specific warnings apply even for system headers? It would be nice if I could check what breaks when by adding `-Wsystem-error=enum-constexpr-conversion` to the global build flags. Rebuilding clang w/ this patched in also works, but is a little more difficult/noisy.
>
> I think there's no per-warning flag (you can turn on _all_ of the enabled warnings in system headers with `-Wsystem-headers`), but by modifying the clang sources you can add `ShowInSystemHeader` to the diagnostic.
> E.g.
>
>    def warn_constexpr_unscoped_enum_out_of_range : Warning<
>      "integer value %0 is outside the valid range of values [%1, %2] for this "
>   -  "enumeration type">, DefaultError, InGroup<DiagGroup<"enum-constexpr-conversion">>;
>   +  "enumeration type">, DefaultError, InGroup<DiagGroup<"enum-constexpr-conversion">>, ShowInSystemHeader;

Thanks for that pointer. It's definitely a lot simpler/general to do that than to patch a change like this in, but still requires rebuilding clang. My most recent attempt didn't find anything `-Wenum-constexpr-conversion` related, but did find a bunch of other breakages that I assume are related to other recent clang changes (usually existing issues with the code that need to be cleaned up, but maybe a few are clang bugs). The fact that `ShowInSystemHeader` already exists makes me hopeful that it isn't too hard to support this, so I filed https://github.com/llvm/llvm-project/issues/63180.

I suppose including this warning in `ShowInSystemHeader` with your diff above would be a good first step anyway, right? If we're about to make it a hard error anyway, then making it a `ShowInSystemHeader` error first would ease that transition.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150226/new/

https://reviews.llvm.org/D150226



More information about the cfe-commits mailing list