[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Louis Dionne via Phabricator via cfe-commits]

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