hazohelet added a comment.
Thanks for the feedback.
================
Comment at: clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp:38
constexpr int fn5() {
- if constexpr (__builtin_is_constant_evaluated()) // expected-warning
{{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a
manifestly constant-evaluated expression}}
+ if constexpr (__builtin_is_constant_evaluated()) // expected-warning
{{'__builtin_is_constant_evaluated' will always evaluate to true in this
context}}
return 0;
----------------
cjdb wrote:
> This should also generate a fix-it hint, since we can automate the fix.
I think it's reasonable to show fix-it hint to remove `constexpr` here.
For that we need to store the source range of `constexpr` in evaluation context
record if it's constexpr-if condition.
================
Comment at: clang/test/SemaCXX/warn-tautological-meta-constant.cpp:18
+ } else {
+ if constexpr (std::is_constant_evaluated()) { // expected-warning {{always
evaluate to true}}
+ return 0;
----------------
cjdb wrote:
> I'm not a fan of this diagnostic text: it doesn't offer insight into what's
> gone wrong or provide actionable feedback on how to fix the code.
I agree that the current wording (`... will always evaluate to true in this
context`) is not great, but I'm not sure how to improve it because the reason
to see this diagnostic would mostly be a misunderstanding of the C++ const
semantics.
At least, if the appearance of `is_constant_evaluated` is in the condition of
constexpr-if, probably we can say they should remove `constexpr`.
CC @aaron.ballman
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
