ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land.
The libc++ changes are reasonable. We could discuss more around how to handle C++03 in `__libcpp_is_constant_evaluated` but I don't think it's worth delaying this patch for that. ================ Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31 + return false; +#endif } ---------------- hazohelet wrote: > philnik wrote: > > Mordante wrote: > > > hazohelet wrote: > > > > Mordante wrote: > > > > > Why is this needed? Does this mean the builtin will fail in C++03 > > > > > mode? > > > > `__libcpp_is_constant_evaluated` always returns false under C++03 or > > > > earlier, where constexpr isn't available. This is a fix to run libc++ > > > > tests without seeing warnings from clang with this patch. > > > > (Live demo: https://godbolt.org/z/oszebM5o5) > > > I see can you add a comment in that regard? To me it looks like the > > > builtin fails in C++03, where returning false indeed is always the right > > > answer. > > https://godbolt.org/z/bafeeY7b7 shows that > > `__builtin_is_constant_evaluated()` doesn't always return false in C++03, > > so this change seems wrong to me. > This is NFC. > The problem is that `__libcpp_is_constant_evaluated()` has different > semantics from `__builtin_is_constant_evaluated()` in C++03. > The cause of this tautological-falsity here is whether > `__libcpp_is_constant_evaluated()` is `constexpr`-qualified or not. > `_LIBCPP_CONSTEXPR` macro expands to `constexpr` since C++11, but in > pre-C++11 mode, it expands to nothing. > So in C++03 mode this function is something like `bool > __libcpp_is_constant_evaluated() { return __builtin_is_constant_evaluated(); > }`, where it is tautologically-false because it is not constexpr-qualified. > > As you mentioned, `__builtin_is_constant_evaluated` does not change its > semantics depending on C++ versions. > However, `__libcpp_is_constant_evaluated()` always returns false in C++03 as > in https://godbolt.org/z/oszebM5o5 This explanation makes sense to me. I think it's reasonable to return `false` unconditionally in C++03 mode. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
