[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#4197789 , @erichkeane 
wrote:

> In D131307#4197767 , @rsmith wrote:
>
>>> Based on feedback we will provide users to the ability to downgrade this 
>>> this diagnostic to a waring to allow for a transition period. We expect to 
>>> turn this diagnostic to an error in the next release.
>>
>> Can we revert this now?
>
> Seems appropriate.  Aaron, WDYT?

I think it's fine -- if we get significant user feedback after 16 goes out to 
the public which suggests we need to retain this for longer, it's easy enough 
to bring back from version control.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131307#4197767 , @rsmith wrote:

>> Based on feedback we will provide users to the ability to downgrade this 
>> this diagnostic to a waring to allow for a transition period. We expect to 
>> turn this diagnostic to an error in the next release.
>
> Can we revert this now?

Seems appropriate.  Aaron, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

> Based on feedback we will provide users to the ability to downgrade this this 
> diagnostic to a waring to allow for a transition period. We expect to turn 
> this diagnostic to an error in the next release.

Can we revert this now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D131307#3726643 , @erichkeane 
wrote:

> In D131307#3726631 , @smeenai wrote:
>
>> Was it intended that the warning generated here isn't silenced by `-w`, only 
>> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
>> that `-Wno-error` doesn't downgrade the error? See 
>> https://godbolt.org/z/s9qPveTWG for an example.
>
> Yes, we've discussed that on this thread before: Clang's behavior for 
> warnings-as-default-error require explicit suppression of the warning, and 
> isn't effected by global warning/error settings.



In D131307#3726644 , @aaron.ballman 
wrote:

> In D131307#3726631 , @smeenai wrote:
>
>> Was it intended that the warning generated here isn't silenced by `-w`, only 
>> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
>> that `-Wno-error` doesn't downgrade the error? See 
>> https://godbolt.org/z/s9qPveTWG for an example.
>
> Yes. That is the behavior of warnings which default to an error. The idea is: 
> these aren't really *warnings*, they're errors that we let users downgrade 
> for . So `-w` shouldn't blanket disable them or users will be very 
> surprised when that warning turns into a hard error in a future version of 
> the compiler. So you have to explicitly disable warnings that default to an 
> error. The same is true for `-Wno-error` behavior.

Yup, all of that makes sense; I just missed it earlier. Thank you both :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#3726631 , @smeenai wrote:

> Was it intended that the warning generated here isn't silenced by `-w`, only 
> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
> that `-Wno-error` doesn't downgrade the error? See 
> https://godbolt.org/z/s9qPveTWG for an example.

Yes. That is the behavior of warnings which default to an error. The idea is: 
these aren't really *warnings*, they're errors that we let users downgrade for 
. So `-w` shouldn't blanket disable them or users will be very 
surprised when that warning turns into a hard error in a future version of the 
compiler. So you have to explicitly disable warnings that default to an error. 
The same is true for `-Wno-error` behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131307#3726631 , @smeenai wrote:

> Was it intended that the warning generated here isn't silenced by `-w`, only 
> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
> that `-Wno-error` doesn't downgrade the error? See 
> https://godbolt.org/z/s9qPveTWG for an example.

Yes, we've discussed that on this thread before: Clang's behavior for 
warnings-as-default-error require explicit suppression of the warning, and 
isn't effected by global warning/error settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Was it intended that the warning generated here isn't silenced by `-w`, only by 
an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and that 
`-Wno-error` doesn't downgrade the error? See https://godbolt.org/z/s9qPveTWG 
for an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D131307#3714629 , @shafik wrote:

> In D131307#3713011 , @sberg wrote:
>
>> With this commit,
>>
>>   $ cat test.cc
>>   #include "boost/numeric/conversion/cast.hpp"
>>   int main() { return boost::numeric_cast(0L); }
>>   
>>   $ clang++ test.cc
>>
>> succeeds without any diagnostic, while with its parent commit 
>> https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
>>  " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
>> enum to a value outside the range of the enumeration values" it had started 
>> to fail with
>
> Yes, that is intended. When modifying the change to allow it to be turned 
> into a warning it started applying outside of constant expression contexts 
> and that broke a lot more stuff.

I think we might be talking past each other here.  There are three commits:

1. D130058  "[Clang] Diagnose ill-formed 
constant expression when setting a non fixed enum to a value outside the range 
of the enumeration values"
2. D131307  "[Clang] Allow downgrading to a 
warning the diagnostic for setting a non fixed enum to a value outside the 
range of the enumeration values"
3. D131528  "[Clang] Restrict non fixed enum 
to a value outside the range of the enumeration values warning to context 
requiring a constant expression"

(1) had started to diagnose my reproducer as an error (and I assume it did 
rightly so).  But then (2) stopped diagnosing it at all, letting the reproducer 
compile successfully without any diagnostic (and I assume wrongly so).  (3) 
didn't make any further change regarding that behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Alexander Yermolovich via Phabricator via cfe-commits]

ayermolo added a comment.

In D131307#3714629 , @shafik wrote:

> In D131307#3713011 , @sberg wrote:
>
>> With this commit,
>>
>>   $ cat test.cc
>>   #include "boost/numeric/conversion/cast.hpp"
>>   int main() { return boost::numeric_cast(0L); }
>>   
>>   $ clang++ test.cc
>>
>> succeeds without any diagnostic, while with its parent commit 
>> https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
>>  " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
>> enum to a value outside the range of the enumeration values" it had started 
>> to fail with
>
> Yes, that is intended. When modifying the change to allow it to be turned 
> into a warning it started applying outside of constant expression contexts 
> and that broke a lot more stuff.
>
> I am planning on adding a default to a warning diagnostic for the 
> non-constant expression cases but that will be done separately. I wanted to 
> help folks unbreak their builds first.

I think after this change we started to see
protobuf/protobuf/src/google/protobuf/wire_format_lite.cc:103:9: error: integer 
value -1 is outside the valid range of values [0, 7] for this enumeration type 
[-Wenum-constexpr-conversion]

  static_cast(-1),  // invalid

Can I add a flag to turn this back in to a note?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131307#3713011 , @sberg wrote:

> With this commit,
>
>   $ cat test.cc
>   #include "boost/numeric/conversion/cast.hpp"
>   int main() { return boost::numeric_cast(0L); }
>   
>   $ clang++ test.cc
>
> succeeds without any diagnostic, while with its parent commit 
> https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
>  " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
> enum to a value outside the range of the enumeration values" it had started 
> to fail with

Yes, that is intended. When modifying the change to allow it to be turned into 
a warning it started applying outside of constant expression contexts and that 
broke a lot more stuff.

I am planning on adding a default to a warning diagnostic for the non-constant 
expression cases but that will be done separately. I wanted to help folks 
unbreak their builds first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

With this commit,

  $ cat test.cc
  #include "boost/numeric/conversion/cast.hpp"
  int main() { return boost::numeric_cast(0L); }
  
  $ clang++ test.cc

succeeds without any diagnostic, while with its parent commit 
https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
 " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
enum to a value outside the range of the enumeration values" it had started to 
fail with

  In file included from test.cc:1:
  In file included from /usr/include/boost/numeric/conversion/cast.hpp:33:
  In file included from /usr/include/boost/numeric/conversion/converter.hpp:13:
  In file included from 
/usr/include/boost/numeric/conversion/conversion_traits.hpp:13:
  In file included from 
/usr/include/boost/numeric/conversion/detail/conversion_traits.hpp:18:
  In file included from 
/usr/include/boost/numeric/conversion/detail/int_float_mixture.hpp:19:
  In file included from /usr/include/boost/mpl/integral_c.hpp:32:
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: non-type 
template argument is not a constant expression
  typedef AUX_WRAPPER_INST( 
BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;
  
~~^~~~
  /usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 
'BOOST_MPL_AUX_STATIC_CAST'
  #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast(expr)
^
  /usr/include/boost/mpl/integral_c.hpp:31:54: note: expanded from macro 
'AUX_WRAPPER_INST'
  #define AUX_WRAPPER_INST(value) AUX_WRAPPER_NAME< T, value >
   ^
  /usr/include/boost/numeric/conversion/detail/meta.hpp:30:46: note: in 
instantiation of template class 
'mpl_::integral_c' requested here
 enum { x = ( BOOST_MPL_AUX_VALUE_WKND(T1)::value == 
BOOST_MPL_AUX_VALUE_WKND(T2)::value ) };
   ^
  /usr/include/boost/mpl/if.hpp:63:68: note: in instantiation of template class 
'boost::numeric::convdetail::equal_to, 
mpl_::integral_c>' requested here
BOOST_MPL_AUX_STATIC_CAST(bool, BOOST_MPL_AUX_VALUE_WKND(T1)::value)
 ^
  /usr/include/boost/mpl/eval_if.hpp:37:22: note: in instantiation of template 
class 
'boost::mpl::if_, 
mpl_::integral_c>, 
boost::mpl::identity>, 
boost::mpl::eval_if, 
mpl_::integral_c>, 
boost::mpl::identity>>, 
boost::mpl::if_, 
mpl_::integral_c>, 
boost::mpl::identity>, boost::mpl::identity' requested here
  typedef typename if_::type f_;
   ^
  /usr/include/boost/numeric/conversion/detail/meta.hpp:81:12: note: in 
instantiation of template class 
'boost::mpl::eval_if, 
mpl_::integral_c>, 
boost::mpl::identity>, 
boost::mpl::eval_if, 
mpl_::integral_c>, 
boost::mpl::identity>>, 
boost::mpl::if_, 
mpl_::integral_c>, 
boost::mpl::identity>, boost::mpl::identity' requested here
mpl::eval_if::type
 ^
  /usr/include/boost/numeric/conversion/detail/udt_builtin_mixture.hpp:41:7: 
note: in instantiation of template class 
'boost::numeric::convdetail::ct_switch4, 
mpl_::integral_c, 
mpl_::integral_c, 
mpl_::integral_c, 
boost::numeric::convdetail::get_subranged_BuiltIn2BuiltIn, 
boost::mpl::identity>, 
boost::mpl::identity>, boost::mpl::identity>>' requested here
ct_switch4::type
  ^
  /usr/include/boost/numeric/conversion/conversion_traits.hpp:22:7: note: in 
instantiation of template class 
'boost::numeric::convdetail::non_trivial_traits_impl' requested here
  : convdetail::get_conversion_traits::type
^
  /usr/include/boost/numeric/conversion/detail/converter.hpp:584:22: note: in 
instantiation of template class 'boost::numeric::conversion_traits' 
requested here
  typedef typename Traits::trivial trivial ;
   ^
  /usr/include/boost/numeric/conversion/converter.hpp:29:32: note: in 
instantiation of template class 
'boost::numeric::convdetail::get_converter_impl, boost::numeric::def_overflow_handler, boost::numeric::Trunc, 
boost::numeric::raw_converter>, 
boost::numeric::UseInternalRangeChecker>' requested here
  struct converter : convdetail::get_converter_impl, 
boost::numeric::def_overflow_handler, boost::numeric::Trunc>' requested 
here
  return converter::convert(arg);
 ^
  test.cc:2:28: note: in instantiation of function template specialization 
'boost::numeric_cast' requested here
  int main() { return boost::numeric_cast(0L); }
 ^
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: note: integer value 
-1 is outside the valid range of values [0, 3] for this enumeration type
  typedef AUX_WRAPPER_INST( 
BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

@zatrazz please see D131528  which should 
prevent the diagnostic for triggering for the example you showed us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131307#3709644 , @erichkeane 
wrote:

> 



> That IS Undefined Behavior, but I think was unintended by this patch.  The 
> intent of this patch (and its parent) was to diagnose this UB during 
> `constexpr` evaluation.  HOWEVER, this patch seems to have extended it to 
> non-`constexpr` constant evaluation.  So while that _IS_ undefined behavior, 
> I don't think it should be covered by this error (likely a normal 'warning' 
> is fine here, but not this diagnostic).  Hopefully @shafik can correct this 
> when he starts again today.

That was not totally intended. I am working on some changes to narrow down the 
scope of the diagnostic and hopefully that will reduce the fallout.

Although Erich is correct, that is undefined behavior and should be eventually 
be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131307#3709595 , @zatrazz wrote:

> This commit seems to have broken test-suite on aarch64 [1]. The warning shows:
>
>   FAILED: MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o
>   /home/adhemerval.zanella/projects/llvm/test-suite-build/tools/timeit 
> --summary MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.time 
> /home/adhemerval.zanella/projects/llvm/llvm-src-build-stage1-buildbot/bin/clang++
>  -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time -DNOASM -DLLVM -MD -MT 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -MF 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.d -o 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -c 
> /home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp
>   
> /home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp:3606:24:
>  error: integer value -1 is outside the valid range of values [0, 15] for 
> this enumeration type [-Wenum-constexpr-conversion]
>   if (c==EOF) return (Filetype)(-1);
>  ^
>   1 error generated.
>
> I am not sure if this is an existing issue with test-suite.
>
> [1] https://lab.llvm.org/staging/#/builders/158/builds/344

That IS Undefined Behavior, but I think was unintended by this patch.  The 
intent of this patch (and its parent) was to diagnose this UB during 
`constexpr` evaluation.  HOWEVER, this patch seems to have extended it to 
non-`constexpr` constant evaluation.  So while that _IS_ undefined behavior, I 
don't think it should be covered by this error (likely a normal 'warning' is 
fine here, but not this diagnostic).  Hopefully @shafik can correct this when 
he starts again today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz added a comment.

This commit seems to have broken test-suite on aarch64 [1]. The warning shows:

  FAILED: MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o
  /home/adhemerval.zanella/projects/llvm/test-suite-build/tools/timeit 
--summary MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.time 
/home/adhemerval.zanella/projects/llvm/llvm-src-build-stage1-buildbot/bin/clang++
 -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time -DNOASM -DLLVM -MD -MT 
MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -MF 
MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.d -o 
MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -c 
/home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp
  
/home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp:3606:24:
 error: integer value -1 is outside the valid range of values [0, 15] for this 
enumeration type [-Wenum-constexpr-conversion]
  if (c==EOF) return (Filetype)(-1);
 ^
  1 error generated.

I am not sure if this is an existing issue with test-suite.

[1] https://lab.llvm.org/staging/#/builders/158/builds/344


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#3709494 , @JonChesterfield 
wrote:

> Did some digging here. The function hsa_agent_get_info takes an argument of 
> type hsa_agent_info_t, which has declared values in the range [0 24]. The 
> implementation of that (in amd_gpu_agent fwiw) casts that argument to a 
> size_t and then switches on it, checking those declared values and a bunch of 
> extensions. This is used to provide vendor extensions through a 
> vendor-agnostic interface.
>
> This seems to be a case where C and C++ have diverged.

Yes, they've always been divergent in this area, it's only thanks to the magic 
of constexpr that anyone really notices now.

> As far as I can tell, C thinks an enum is an int, and anything that fits in 
> an int can be stored in one and retrieved later.

It's a bit more complicated than that, unfortunately. But this is kind of the 
gist of it.

> C23 lets one specify the underlying type. C++ evidently thinks the value 
> stored must be within [min max] of the declaration, which is at least more 
> flexible than requiring it be one in the declaration.

Correct, C++ uses the minimum-sized bit-field that can hold all of the values 
of the enumeration.

> So I think the fix here is to change hsa_agent_info_t to include 
> `HSA_AGENT_INFO_UNUSED_INCREASE_RANGE_OF_TYPE = INT32_MAX` so the vendor 
> extensions remain accessible. It's a header that is usable from C and C++ so 
> it needs to do something conforming to both. Does that sound right?

Yes, that's how I'd solve the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Did some digging here. The function hsa_agent_get_info takes an argument of 
type hsa_agent_info_t, which has declared values in the range [0 24]. The 
implementation of that (in amd_gpu_agent fwiw) casts that argument to a size_t 
and then switches on it, checking those declared values and a bunch of 
extensions. This is used to provide vendor extensions through a vendor-agnostic 
interface.

This seems to be a case where C and C++ have diverged. As far as I can tell, C 
thinks an enum is an int, and anything that fits in an int can be stored in one 
and retrieved later. C23 lets one specify the underlying type. C++ evidently 
thinks the value stored must be within [min max] of the declaration, which is 
at least more flexible than requiring it be one in the declaration.

So I think the fix here is to change hsa_agent_info_t to include 
`HSA_AGENT_INFO_UNUSED_INCREASE_RANGE_OF_TYPE = INT32_MAX` so the vendor 
extensions remain accessible. It's a header that is usable from C and C++ so it 
needs to do something conforming to both. Does that sound right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#3709083 , @sylvestre.ledru 
wrote:

> We have this error in OpenMP:
> https://github.com/llvm/llvm-project/issues/57022

That bug looks to be a true positive from this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

We have this error in OpenMP:
https://github.com/llvm/llvm-project/issues/57022


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.

thanks for the analysis , i agree its a coding error in our library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131307#3708413 , @ronlieb wrote:

> hi, latest commit seems to have broken buildbot  for amdgpu
> https://lab.llvm.org/buildbot/#/builders/193/builds/16651

This looks like a real bug, the enum which has a typedef `hsa_agent_info_t` has 
a max enumerator with a value of `24` and therefore because it does not have a 
fixed underlying type the value range is indeed `[0, 31]`.

I am not sure what the right fix is here since it looks like the enum is meant 
to be used with C as well as C++. In C++ one could give a fixed underlying type 
if it was meant to have any `int` value let's say.

`HSA_AMD_AGENT_INFO_COMPUTE_UNIT_COUNT` is way outside the values of 
`hsa_agent_info_t` so maybe this was not intended at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.

hi, latest commit seems to have broken buildbot  for amdgpu
https://lab.llvm.org/buildbot/#/builders/193/builds/16651


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc104113ddec: [Clang] Allow downgrading to a warning the 
diagnostic for setting a non fixed… (authored by shafik).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D131307?vs=450455=450996#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp
  clang/test/SemaCXX/compare.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2418,37 +2418,37 @@
 
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast(-8);
-  constexpr E1 x2 = static_cast(8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 8 is outside the valid range of values [-8, 7] for this enumeration type}}
+  constexpr E1 x2 = static_cast(8);
+  // expected-error@-1 {{integer value 8 is outside the valid range of values [-8, 7] for this enumeration type}}
 
-  constexpr E2 x3 = static_cast(-8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value -8 is outside the valid range of values [0, 7] for this enumeration type}}
+  constexpr E2 x3 = static_cast(-8);
+  // expected-error@-1 {{integer value -8 is outside the valid range of values [0, 7] for this enumeration type}}
   constexpr E2 x4 = static_cast(0);
-  constexpr E2 x5 = static_cast(8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 8 is outside the valid range of values [0, 7] for this enumeration type}}
+  constexpr E2 x5 = static_cast(8);
+  // expected-error@-1 {{integer value 8 is outside the valid range of values [0, 7] for this enumeration type}}
 
   constexpr E3 x6 = static_cast(-2048);
   constexpr E3 x7 = static_cast(-8);
   constexpr E3 x8 = static_cast(0);
   constexpr E3 x9 = static_cast(8);
-  constexpr E3 x10 = static_cast(2048); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2048 is outside the valid range of values [-2048, 2047] for this enumeration type}}
+  constexpr E3 x10 = static_cast(2048);
+  // expected-error@-1 {{integer value 2048 is outside the valid range of values [-2048, 2047] for this enumeration type}}
 
   constexpr E4 x11 = static_cast(0);
   constexpr E4 x12 = static_cast(1);
-  constexpr E4 x13 = static_cast(2); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
+  constexpr E4 x13 = static_cast(2);
+  // expected-error@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
 
   constexpr EEmpty x14 = static_cast(0);
   constexpr EEmpty x15 = static_cast(1);
-  constexpr EEmpty x16 = static_cast(2); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
+  constexpr EEmpty x16 = static_cast(2);
+  // expected-error@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
 
   constexpr EFixed x17 = static_cast(100);
   constexpr EScoped x18 = static_cast(100);
 
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
-  constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
+  constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
+  // expected-error@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
 }
 }
Index: clang/test/SemaCXX/compare.cpp
===
--- clang/test/SemaCXX/compare.cpp
+++ clang/test/SemaCXX/compare.cpp
@@ -428,11 +428,6 @@
 namespace tautological_enum {
   enum E { a, b, c } e;
 
-  // FIXME: We should warn about constructing this out-of-range numeration value.
-  const E invalid = (E)-1;
-  // ... but we should not warn about comparing against it.
-  bool x = e == invalid;
-
   // We should not warn about relational comparisons for enumerators, even if
   // they're tautological.
   bool y = e >= a && e <= b;
Index: clang/test/SemaCXX/MicrosoftCompatibility.cpp

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

All right, SGTM


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D131307#3705362 , @smeenai wrote:

> In D131307#3704709 , @thakis wrote:
>
>> It's already an error, but it's a warning default-mapped to an error. You 
>> can -Wno-error=name to downgrade it into a warning, but that requires an 
>> explicit action. So people are unlikely to miss it.
>>
>> This is how we usually handle these breaking changes.
>>
>> Maybe there could be a test for the -Wno-error= case? But this looks roughly 
>> right to me overall. I haven't looked in detail.
>
> Right, but we also want people to understand that the downgrade possibility 
> will be going away in the next release (i.e. it'll always be an error and 
> there's nothing you can do about that), so they really do want to deal with 
> this as a priority.

FWIW, we had a similar situation (but not quite the same) during Clang 15 where 
we had some diagnostics that were strengthened to be an error but that 
allowance only exists in some language modes:

  - The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
an error in C99 and later. Prior to C2x, it may be downgraded to a warning
with ``-Wno-error=implicit-function-declaration``, or disabled entirely with
``-Wno-implicit-function-declaration``. As of C2x, support for implicit
function declarations has been removed, and the warning options will have no
effect.
  - The ``-Wimplicit-int`` warning diagnostic now defaults to an error in C99 
and
later. Prior to C2x, it may be downgraded to a warning with
``-Wno-error=implicit-int``, or disabled entirely with 
``-Wno-implicit-int``.
As of C2x, support for implicit int has been removed, and the warning 
options
will have no effect. Specifying ``-Wimplicit-int`` in C89 mode will now 
issue
warnings instead of being a noop.

I think the release note we have in this patch is reasonable.

This LGTM!


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Default-mapped-to-error and release notes seems like a pretty strong signal to 
me. I think this is fine as is.


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131307#3704709 , @thakis wrote:

> It's already an error, but it's a warning default-mapped to an error. You can 
> -Wno-error=name to downgrade it into a warning, but that requires an explicit 
> action. So people are unlikely to miss it.
>
> This is how we usually handle these breaking changes.
>
> Maybe there could be a test for the -Wno-error= case? But this looks roughly 
> right to me overall. I haven't looked in detail.

We don't typically test versions of `-Wno-error=`, we tend to trust that the 
diagnostics system 'works' in this case.  Unless there is a situation that 
you're concerned about?

In D131307#3705362 , @smeenai wrote:

> In D131307#3704709 , @thakis wrote:
>
>> It's already an error, but it's a warning default-mapped to an error. You 
>> can -Wno-error=name to downgrade it into a warning, but that requires an 
>> explicit action. So people are unlikely to miss it.
>>
>> This is how we usually handle these breaking changes.
>>
>> Maybe there could be a test for the -Wno-error= case? But this looks roughly 
>> right to me overall. I haven't looked in detail.
>
> Right, but we also want people to understand that the downgrade possibility 
> will be going away in the next release (i.e. it'll always be an error and 
> there's nothing you can do about that), so they really do want to deal with 
> this as a priority.

While I'm sympathetic to this, I don't think there is precedent for doing 
something like that.  I think I'd be OK tacking an extra note onto this (and 
starting said precedent), but I think we'd need to hear from a code-owner to 
make a change like that.


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D131307#3704709 , @thakis wrote:

> It's already an error, but it's a warning default-mapped to an error. You can 
> -Wno-error=name to downgrade it into a warning, but that requires an explicit 
> action. So people are unlikely to miss it.
>
> This is how we usually handle these breaking changes.
>
> Maybe there could be a test for the -Wno-error= case? But this looks roughly 
> right to me overall. I haven't looked in detail.

Right, but we also want people to understand that the downgrade possibility 
will be going away in the next release (i.e. it'll always be an error and 
there's nothing you can do about that), so they really do want to deal with 
this as a priority.


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

It's already an error, but it's a warning default-mapped to an error. You can 
-Wno-error=name to downgrade it into a warning, but that requires an explicit 
action. So people are unlikely to miss it.

This is how we usually handle these breaking changes.

Maybe there could be a test for the -Wno-error= case? But this looks roughly 
right to me overall. I haven't looked in detail.


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Thank you!

I'm worried that users might miss this if it's only in the release notes, and 
then we'd be in a similar situation again when we try converting it to an 
error. Maybe you could also include the bit about the diagnostic turning into 
error-only in the diagnostic message itself, to make it more likely for people 
to notice it?


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13532
 
-if (DestType->isEnumeralType()) {
+if (Info.Ctx.getLangOpts().CPlusPlus && DestType->isEnumeralType()) {
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());

Changing this to a warning meaning it applied in more cases, so I need to 
explicitly restrict this to C++ contexts.



Comment at: clang/test/SemaCXX/MicrosoftCompatibility.cpp:242
 ENUM *var = 0; 
-ENUM var2 = (ENUM)3;
+ENUM var2 = (ENUM)0;
 enum ENUM1* var3 = 0;// expected-warning {{forward references to 'enum' types 
are a Microsoft extension}}

This is undefined behavior, using `0` avoids the diagnostic and I believe still 
sticks to the intent of the test. Same below.



Comment at: clang/test/SemaCXX/compare.cpp:432
-  // FIXME: We should warn about constructing this out-of-range numeration 
value.
-  const E invalid = (E)-1;
-  // ... but we should not warn about comparing against it.

This is UB and I don't think this applies anymore.



Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2422
+  constexpr E1 x2 = static_cast(8);
+  // expected-error@-1 {{integer value 8 is outside the valid range of values 
[-8, 7] for this enumeration type}}
 

I kept this on the next line since when we turn this back into an error we will 
have two diagnostics again.


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

https://reviews.llvm.org/D131307

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: aaron.ballman, erichkeane, thakis, smeenai, tahonermann.
Herald added a project: All.
shafik requested review of this revision.

In D130058  we diagnose the undefined 
behavior of setting the value outside the range of the enumerations values for 
an enum without a fixed underlying type.

Based on feedback we will provide users to the ability to downgrade this this 
diagnostic to a waring to allow for a transition period. We expect to turn this 
diagnostic to an error in the next release.


https://reviews.llvm.org/D131307

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp
  clang/test/SemaCXX/compare.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2418,37 +2418,37 @@
 
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast(-8);
-  constexpr E1 x2 = static_cast(8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 8 is outside the valid range of values [-8, 7] for this enumeration type}}
+  constexpr E1 x2 = static_cast(8);
+  // expected-error@-1 {{integer value 8 is outside the valid range of values [-8, 7] for this enumeration type}}
 
-  constexpr E2 x3 = static_cast(-8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value -8 is outside the valid range of values [0, 7] for this enumeration type}}
+  constexpr E2 x3 = static_cast(-8);
+  // expected-error@-1 {{integer value -8 is outside the valid range of values [0, 7] for this enumeration type}}
   constexpr E2 x4 = static_cast(0);
-  constexpr E2 x5 = static_cast(8); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 8 is outside the valid range of values [0, 7] for this enumeration type}}
+  constexpr E2 x5 = static_cast(8);
+  // expected-error@-1 {{integer value 8 is outside the valid range of values [0, 7] for this enumeration type}}
 
   constexpr E3 x6 = static_cast(-2048);
   constexpr E3 x7 = static_cast(-8);
   constexpr E3 x8 = static_cast(0);
   constexpr E3 x9 = static_cast(8);
-  constexpr E3 x10 = static_cast(2048); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2048 is outside the valid range of values [-2048, 2047] for this enumeration type}}
+  constexpr E3 x10 = static_cast(2048);
+  // expected-error@-1 {{integer value 2048 is outside the valid range of values [-2048, 2047] for this enumeration type}}
 
   constexpr E4 x11 = static_cast(0);
   constexpr E4 x12 = static_cast(1);
-  constexpr E4 x13 = static_cast(2); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
+  constexpr E4 x13 = static_cast(2);
+  // expected-error@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
 
   constexpr EEmpty x14 = static_cast(0);
   constexpr EEmpty x15 = static_cast(1);
-  constexpr EEmpty x16 = static_cast(2); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
+  constexpr EEmpty x16 = static_cast(2);
+  // expected-error@-1 {{integer value 2 is outside the valid range of values [0, 1] for this enumeration type}}
 
   constexpr EFixed x17 = static_cast(100);
   constexpr EScoped x18 = static_cast(100);
 
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
-  constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1); // expected-error {{must be initialized by a constant expression}}
-  // expected-note@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
+  constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
+  // expected-error@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
 }
 }
Index: clang/test/SemaCXX/compare.cpp
===
--- clang/test/SemaCXX/compare.cpp
+++ clang/test/SemaCXX/compare.cpp
@@ -428,11 +428,6 @@
 namespace tautological_enum {
   enum E { a, b, c } e;
 
-  // FIXME: We should warn about constructing this out-of-range numeration value.
-  const E invalid = (E)-1;
-  // ... but we should not warn about comparing against it.
-  bool x = e == invalid;
-
   // We should not warn about relational comparisons for enumerators, even if
   // they're