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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D131528#3719414 <https://reviews.llvm.org/D131528#3719414>, @shafik wrote:

> 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`

Ok, but however, before D131307 <https://reviews.llvm.org/D131307> which allowed to downgrade the error to a warning, these two source examples didn't produce any single warning or error (relating to the enums) at all - and AFAIK the intent of D131307 <https://reviews.llvm.org/D131307> was to lower the impact of the enum-conversion error, not widen the scope of what it covers - right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528