[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

Thu Aug 11 05:05:33 PDT 2022

aaron.ballman added a comment.

In D130058#3714880 <https://reviews.llvm.org/D130058#3714880>, @glandium wrote:

> In D130058#3714672 <https://reviews.llvm.org/D130058#3714672>, @glandium wrote:
>
>> This catches https://searchfox.org/mozilla-central/rev/c77834ec635c523f2ba0092fcd1728c9b1c3005b/third_party/rust/glslopt/glsl-optimizer/src/compiler/glsl/ir_reader.cpp#732 but not https://searchfox.org/mozilla-central/rev/c77834ec635c523f2ba0092fcd1728c9b1c3005b/third_party/rust/glslopt/glsl-optimizer/src/compiler/glsl/ir.cpp#654. Is that expected?
>
> Maybe something changed? A couple days ago it was catching https://searchfox.org/mozilla-central/rev/c77834ec635c523f2ba0092fcd1728c9b1c3005b/gfx/cairo/cairo/src/win32/cairo-dwrite-font.cpp#238 but apparently not anymore.

Correct, the diagnostic was previously over-eager, but that's been fixed.

In D130058#3714882 <https://reviews.llvm.org/D130058#3714882>, @glandium wrote:

> Also not caught: a cast of 0 when 0 is not a valid value in the enum.

I don't think that situation will ever be UB. When the underlying type of the enum is not fixed, the range of values it can represent is whatever values fit into a hypothetical bit-field that is large enough to cover the full range of stated values (https://eel.is/c++draft/enum#dcl.enum-8.sentence-2). `0` is something that can always be represented in such a bit-field (there's a special provision for empty enumerations or one that can only store 0).

In D130058#3715183 <https://reviews.llvm.org/D130058#3715183>, @glandium wrote:

> Having looked around in the Firefox codebase for cases that now fail to build because of this, and looking at the clarification in DR2338, I wonder if enums in `extern "C"` sections should be treated as if they had an explicit type of `int` as if it were e.g. `enum Foo: int { ... }`.

That doesn't match the C++ language rules for constant expression evaluation, so I don't think we should go that route unless WG21 decides they want to.

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

https://reviews.llvm.org/D130058