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

hazohelet added a comment. Thank you for the revert! This looks like a general issue of clang on template deduction in constant-evaluated context (https://godbolt.org/z/zTro7Ycfa). Pushing constant-evaluated context against initializer of instantiated constexpr variables made this bug appear

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

hazohelet added a comment. Thanks for the report about the errors in lambdas. The culprit is `clang/lib/Sema/SemaTemplateInstantiateDecl.cpp`. I am pushing constant-evaluated context if the instantiated variable is constexpr variable, and somehow it causes those errors. I'll take deeper look

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

hazohelet added a comment. In D155064#4651306 , @hans wrote: > We saw a new warning in Chromium after this, and I wasn't sure if that's > intentional: > > #include > > template float foo(T input) { > if (sizeof(T) > 2) { > return 42;

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

sammccall added a comment. Hi, I'm afraid this breaks valid code with false diagnostics, I need to revert. Specifically it complains about missing lambda captures for values that are in fact arguments to the lambda: template constexpr void inner(F f) { return f(0); } template

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

abrachet added a comment. struct S { template constexpr bool a(F&& f) const { // This works previously in clang and on gcc return f(1); // no matching function for call to object of type ... } }; template struct S1 { void f() { auto test = [] (auto)

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

hans added a comment. We saw a new warning in Chromium after this, and I wasn't sure if that's intentional: #include template float foo(T input) { if (sizeof(T) > 2) { return 42; } else { constexpr float inverseMax = 1.0f / std::numeric_limits::max(); return

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG491b2810fb7f: [clang][SemaCXX] Diagnose tautological uses of consteval if and… (authored by hazohelet). Changed prior to commit:

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

cor3ntin accepted this revision. cor3ntin added a comment. @hazohelet Nah, feel free to merge that first! Thanks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 ___ cfe-commits mailing list

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

hazohelet updated this revision to Diff 556946. hazohelet added a comment. After discussion in https://github.com/llvm/llvm-project/pull/66222, I noticed I had misunderstood the recursiveness of constant-evaluated context. I was pushing constant-evaluating on the lambda body when it appears

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

cor3ntin accepted this revision. cor3ntin added a comment. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

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.

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

cor3ntin added a comment. @philnik @Mordante were the libc++ concerns addressed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

hazohelet added a comment. Ping 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

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

hazohelet added a comment. Ping 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

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

hazohelet marked an inline comment as done. hazohelet added inline comments. Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31 + return false; +#endif } philnik wrote: > Mordante wrote: > > hazohelet wrote: > > > Mordante wrote: > > > > Why

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

philnik added inline comments. Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31 + return false; +#endif } Mordante wrote: > hazohelet wrote: > > Mordante wrote: > > > Why is this needed? Does this mean the builtin will fail in C++03 mode? >

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

hazohelet updated this revision to Diff 551744. hazohelet marked 2 inline comments as done. hazohelet added a comment. Address comments from Mark - Added comment about the need of macro use in libc++ include file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/

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

Mordante added inline comments. Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31 + return false; +#endif } hazohelet wrote: > Mordante wrote: > > Why is this needed? Does this mean the builtin will fail in C++03 mode? >

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

hazohelet added inline comments. Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31 + return false; +#endif } Mordante wrote: > Why is this needed? Does this mean the builtin will fail in C++03 mode? `__libcpp_is_constant_evaluated` always

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

Mordante added inline comments. Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31 + return false; +#endif } Why is this needed? Does this mean the builtin will fail in C++03 mode? Comment at:

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

hazohelet updated this revision to Diff 550899. hazohelet added a comment. rebase to upstream to run ci CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticASTKinds.td

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

hazohelet updated this revision to Diff 550881. hazohelet marked an inline comment as done. hazohelet added a comment. Address comments from Chris - Generate fix-it hint to remove `constexpr` in constexpr-if Added `SourceLocation` field to Sema that saves the location of `constexpr` in

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

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

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

cjdb added inline comments. 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

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

hazohelet updated this revision to Diff 549203. hazohelet added a comment. Resolved the constexpr-if init-statement expression evaluation context problem. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 Files: clang/docs/ReleaseNotes.rst

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

hazohelet updated this revision to Diff 548197. hazohelet added a comment. - Tentatively disable the warning on macros so as not to make a fuss in libc++ tests. - Fixed incorrect output in arguments. - Fixed incorrect output in declaration of init-statement of constexpr-if condition. - Added

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

hazohelet added a comment. I've noticed several shortcoming/limitations of this patch. 1. Function arguments: When we parse the arguments to a function call, the callee isn't still resolved, so we don't know whether it's consteval. If the callee is consteval, its argument is known to be

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

hazohelet added a comment. @ldionne @philnik @Mordante Is this way of fixing libc++ tests OK from libc++ side? If there's better way to fix them I am happy to follow it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064

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

cor3ntin added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +//

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

hazohelet added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +//

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

hazohelet updated this revision to Diff 543510. hazohelet marked 4 inline comments as done. hazohelet added a comment. Address comments from Corentin - When emitting "lambda expression may not appear inside of a constant expression" error from `Sema::PopExpressionEvaluationContext`, mark the

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

cor3ntin added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +//

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

hazohelet added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +//

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

cor3ntin added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +//

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

hazohelet updated this revision to Diff 543189. hazohelet marked 3 inline comments as done. hazohelet edited the summary of this revision. hazohelet added a comment. Address comments from Corentin - NFC stylistic changes - Bring back narrowing warning on constexpr variables to avoid regression -

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

cor3ntin added a comment. In D155064#4523024 , @hazohelet wrote: > In D155064#4514893 , @cor3ntin > wrote: > >> This looks good to me modulo nitpicks. >> When you land that, please make an issue on github for

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

hazohelet added a comment. In D155064#4514893 , @cor3ntin wrote: > This looks good to me modulo nitpicks. > When you land that, please make an issue on github for the missing narrowing > warning, it seems important. > > I'll wait before approving in

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

cor3ntin added a comment. This looks good to me modulo nitpicks. When you land that, please make an issue on github for the missing narrowing warning, it seems important. I'll wait before approving in case @aaron.ballman spot things i missed Comment at:

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

ldionne accepted this revision as: libc++. ldionne added a comment. LGTM for libc++. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

hazohelet added a comment. Regarding the clang CI failure, libcxx's `__libcpp_is_constant_evaluated` always returns false under C++03 or earlier, where constexpr isn't available. (Code:

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

hazohelet updated this revision to Diff 541528. hazohelet marked 8 inline comments as done. hazohelet edited the summary of this revision. hazohelet added a comment. Address review comments - Rename tablegen name of the diagnostics to follow other tautological warnings - Remove redundant

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

ldionne added inline comments. Comment at: libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.verify.cpp:27 static_assert(!std::is_constant_evaluated(), ""); - // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always evaluate to 'true' in a

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

hazohelet marked an inline comment as done. hazohelet added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:2440 +S.ExprEvalContexts.back().Context; +if ((D.getDeclSpec().getTypeQualifiers() == DeclSpec::TQ_const || +

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

philnik added inline comments. Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.segmented.pass.cpp:96 int main(int, char**) { - if (!std::is_constant_evaluated()) { -test_containers, std::deque>(); cor3ntin wrote: >

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

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1546 def warn_consteval_if_always_true : Warning< + "consteval if is always %select{true|false}0 in this context">, To be consistent with other tautological warnings

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

Mordante added inline comments. Comment at: libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.verify.cpp:27 static_assert(!std::is_constant_evaluated(), ""); - // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always evaluate to 'true' in a

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

hazohelet marked 9 inline comments as done. hazohelet added a comment. Thank you for the review! Comment at: clang/include/clang/Sema/Sema.h:1277 +/// as PotentiallyEvaluated. +RuntimeEvaluated }; cor3ntin wrote: > I think I'd prefer a boolean for

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

hazohelet updated this revision to Diff 540029. hazohelet edited the summary of this revision. hazohelet added a comment. Address comments from Corentin, Mark and Nikolas - Removed `InConstantEvaluated` - Removed `RuntimeEvaluated` expression evaluation context kind and instead added a boolean

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

philnik added inline comments. Comment at: libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.verify.cpp:27 static_assert(!std::is_constant_evaluated(), ""); - // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always evaluate to 'true' in a

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

Mordante added a comment. I only looked at the libc++ part. Comment at: libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.verify.cpp:27 static_assert(!std::is_constant_evaluated(), ""); - // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will

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

cor3ntin added a comment. Thanks for working on this! I left some comments, i hope it helps! Comment at: clang/include/clang/Sema/Sema.h:1277 +/// as PotentiallyEvaluated. +RuntimeEvaluated }; I think I'd prefer a boolean for that,

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

hazohelet created this revision. hazohelet added reviewers: aaron.ballman, rsmith, cor3ntin, erichkeane, tbaeder. Herald added a project: All. hazohelet requested review of this revision. Herald added projects: clang, libc++. Herald added a subscriber: libcxx-commits. Herald added a reviewer: