[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131528#3718405 <https://reviews.llvm.org/D131528#3718405>, @mstorsjo wrote:

> In D131528#3716876 <https://reviews.llvm.org/D131528#3716876>, @shafik wrote:
>
>> In D131528#3715841 <https://reviews.llvm.org/D131528#3715841>, @thakis wrote:
>>
>>> We're also still seeing the diag fire after this: https://ci.chromium.org/p/chromium/builders/ci/ToTLinux
>>>
>>> (And we cleaned up our codebase back when it was still an error.)
>>>
>>> Our bots have been red since the change to turn this into a warning landed.
>>
>> Apologies, my condition was not strict enough, I have put up D131704 <https://reviews.llvm.org/D131704>
>
> Unfortunately, even after D131704 <https://reviews.llvm.org/D131704> (with both commits), I'm still hitting a couple cases that weren't an error before this. With https://martin.st/temp/qt-jit-enum.cpp and https://martin.st/temp/protobuf-wire-format.cpp, built with `clang -target i686-w64-mingw32 -c qt-jit-enum.cpp -std=c++17 -Wno-user-defined-literals -Wno-ignored-attributes` (and same for the other file, although `-std=c++17` can be omitted for the protobuf source) I'm still hitting errors for things that weren't even warned about before.

Thank you for those examples, it is super helpful to receive this feedback especially with simple reproducers.

This is expected, it is constant initialization and I verified that they can be turned into warning using `-Wno-error=enum-constexpr-conversion`

Both of these cases are undefined behavior and they can fixed a number of ways. One approach would be to give them a fixed underlying type either like `enum WireType : int` for example of making them a scoped enum like `enum class WireType`. Another solution would be to add the value you want to use an enumerator explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528