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

2022-08-16 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In D131528#3723231 , @ayermolo wrote: > In D131528#3722395 , @shafik wrote: > >> In D131528#3719430 , @ayermolo >> wrote: >> >>> Was this and

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

2022-08-15 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo added a comment. In D131528#3722395 , @shafik wrote: > In D131528#3719430 , @ayermolo > wrote: > >> Was this and previous change intended to capture this case? >> const enum NumberType neg_one = (enum

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

2022-08-14 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. In D131528#3719561 , @mstorsjo wrote: > In D131528#3719414 , @shafik wrote: > >> In D131528#3718405 , @mstorsjo >> wrote: >> >>> In

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

2022-08-14 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. In D131528#3719430 , @ayermolo wrote: > Was this and previous change intended to capture this case? > const enum NumberType neg_one = (enum NumberType) ((enum NumberType) 0 - > (enum NumberType) 1); That was not intended and I

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

2022-08-12 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D131528#3719414 , @shafik wrote: > In D131528#3718405 , @mstorsjo > wrote: > >> In D131528#3716876 , @shafik wrote: >> >>> In

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

2022-08-12 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo added a comment. Was this and previous change intended to capture this case? const enum NumberType neg_one = (enum NumberType) ((enum NumberType) 0 - (enum NumberType) 1); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/

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

2022-08-12 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. In D131528#3718405 , @mstorsjo wrote: > In D131528#3716876 , @shafik wrote: > >> In D131528#3715841 , @thakis wrote: >> >>> We're also still

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

2022-08-12 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D131528#3716876 , @shafik wrote: > In D131528#3715841 , @thakis wrote: > >> We're also still seeing the diag fire after this: >>

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

2022-08-11 Thread Gulfem Savrun Yeniceri via Phabricator via cfe-commits
gulfem added a comment. In D131528#3717509 , @shafik wrote: > In D131528#3717141 , @gulfem wrote: > >> Can somebody please clarify this? Is the following code results in >> `undefined` behavior? >> >> enum E1

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

2022-08-11 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. In D131528#3717141 , @gulfem wrote: > Can somebody please clarify this? Is the following code results in > `undefined` behavior? > > enum E1 {e11=-4, e12=4}; > E1 x2b = static_cast(8); > > `constexpr E1 x2 = static_cast(8)`

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

2022-08-11 Thread Gulfem Savrun Yeniceri via Phabricator via cfe-commits
gulfem added a comment. Can somebody please clarify this? Is the following code results `undefined` behavior? enum E1 {e11=-4, e12=4}; E1 x2b = static_cast(8); `constexpr E1 x2 = static_cast(8)` seems like `undefined`, so `-Wenum-constexpr-conversion` is triggered. We started seeing

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

2022-08-11 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. In 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

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

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D131528#3715217 , @mstorsjo wrote: > There are still some cases that were broken by D131307 > , that aren't fixed by this patch. Building > https://martin.st/temp/qt-enum.cpp with `clang

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

2022-08-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In 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

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

2022-08-11 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. 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. Repository: rG

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

2022-08-11 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. There are still some cases that were broken by D131307 , that aren't fixed by this patch. Building https://martin.st/temp/qt-enum.cpp with `clang -target i686-w64-mingw32 -c -std=c++17 qt-enum.cpp -Wno-ignored-attributes

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

2022-08-10 Thread Shafik Yaghmour via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4e458765aaef: [Clang] Restrict non fixed enum to a value outside the range of the enumeration… (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE

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

2022-08-10 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Can this land? Our bots have been red since the default-error-mapped-warning patch landed... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits mailing list

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

2022-08-10 Thread Adhemerval Zanella via Phabricator via cfe-commits
zatrazz added a comment. It does help on test-suite, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2022-08-10 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. Yep, looks fine to me. We might consider adding an additional warning-as-warning for the alternative case (could you put a comment on this 'if' statement to that effect?). CHANGES SINCE LAST ACTION

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

2022-08-09 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Lg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits mailing list

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

2022-08-09 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik updated this revision to Diff 451318. shafik added a comment. - Adding test to confirm the warning does not trigger outside of a constant expression context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 Files:

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

2022-08-09 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Should there be a test case to verify that the warning isn't triggered in a non-constexpr context anymore? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits

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

2022-08-09 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman, zatrazz. Herald added a project: All. shafik requested review of this revision. In D131307 we allowed the diagnostic to be turned into a warning for a transition period. This had