From b5d9f00b2096290653fcb6e8de38d5c352af63a0 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Tue, 17 Jan 2023 11:29:04 -0800 Subject: [PATCH] Forbid implicit conversion of constraint expression to bool As reported in https://github.com/llvm/llvm-project/issues/54524, and later in https://github.com/llvm/llvm-project/issues/60038, we were not properly implmenting temp.constr.atomic P3. This patch stops implicitly converting constraints to bool, and ensures the Rvalue conversion takes place as needed. Differential Revision: https://reviews.llvm.org/D141954 --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/Sema/SemaConcept.cpp | 25 +++++--- .../constrant-satisfaction-conversions.cpp | 67 ++++++++++++++++++++++ 3 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8de179c..97ce886 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -774,6 +774,9 @@ C++20 Feature Support and `P1975R0: `_, which allows parenthesized aggregate-initialization. +- Fixed an issue with concept requirement evaluation, where we incorrectly allowed implicit + conversions to bool for a requirement. This fixes `GH54524 `_. + C++2b Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index f20e475..4d4b248 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -329,14 +329,7 @@ static ExprResult calculateConstraintSatisfaction( Sema::SFINAETrap Trap(S); SubstitutedExpression = S.SubstConstraintExpr(const_cast(AtomicExpr), MLTAL); - // Substitution might have stripped off a contextual conversion to - // bool if this is the operand of an '&&' or '||'. For example, we - // might lose an lvalue-to-rvalue conversion here. If so, put it back - // before we try to evaluate. - if (SubstitutedExpression.isUsable() && - !SubstitutedExpression.isInvalid()) - SubstitutedExpression = - S.PerformContextuallyConvertToBool(SubstitutedExpression.get()); + if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) { // C++2a [temp.constr.atomic]p1 // ...If substitution results in an invalid type or expression, the @@ -373,6 +366,22 @@ static ExprResult calculateConstraintSatisfaction( if (!S.CheckConstraintExpression(SubstitutedExpression.get())) return ExprError(); + // [temp.constr.atomic]p3: To determine if an atomic constraint is + // satisfied, the parameter mapping and template arguments are first + // substituted into its expression. If substitution results in an + // invalid type or expression, the constraint is not satisfied. + // Otherwise, the lvalue-to-rvalue conversion is performed if necessary, + // and E shall be a constant expression of type bool. + // + // Perform the L to R Value conversion if necessary. We do so for all + // non-PRValue categories, else we fail to extend the lifetime of + // temporaries, and that fails the constant expression check. + if (!SubstitutedExpression.get()->isPRValue()) + SubstitutedExpression = ImplicitCastExpr::Create( + S.Context, SubstitutedExpression.get()->getType(), + CK_LValueToRValue, SubstitutedExpression.get(), + /*BasePath=*/nullptr, VK_PRValue, FPOptionsOverride()); + return SubstitutedExpression; }); } diff --git a/clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp b/clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp new file mode 100644 index 0000000..ba8e2dc --- /dev/null +++ b/clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp @@ -0,0 +1,67 @@ +// RUN: %clang_cc1 -std=c++20 -x c++ -Wno-constant-logical-operand -verify %s + +template concept C = +sizeof(T) == 4 && !true; // requires atomic constraints sizeof(T) == 4 and !true + +template concept C2 = sizeof(T); // expected-error{{atomic constraint must be of type 'bool' (found }} + +template struct S { + constexpr operator bool() const { return true; } +}; + +// expected-error@+3{{atomic constraint must be of type 'bool' (found 'S')}} +// expected-note@#FINST{{while checking constraint satisfaction}} +// expected-note@#FINST{{in instantiation of function template specialization}} +template requires (S{}) +void f(T); +void f(int); + +// Ensure this applies to operator && as well. +// expected-error@+3{{atomic constraint must be of type 'bool' (found 'S')}} +// expected-note@#F2INST{{while checking constraint satisfaction}} +// expected-note@#F2INST{{in instantiation of function template specialization}} +template requires (S{} && true) +void f2(T); +void f2(int); + +template requires requires { + requires S{}; + // expected-error@-1{{atomic constraint must be of type 'bool' (found 'S')}} + // expected-note@-2{{while checking the satisfaction}} + // expected-note@-3{{in instantiation of requirement}} + // expected-note@-4{{while checking the satisfaction}} + // expected-note@-6{{while substituting template arguments}} + // expected-note@#F3INST{{while checking constraint satisfaction}} + // expected-note@#F3INST{{in instantiation of function template specialization}} + // +} +void f3(T); +void f3(int); + +// Doesn't diagnose, since this is no longer a compound requirement. +template requires (bool(1 && 2)) +void f4(T); +void f4(int); + +void g() { + f(0); // #FINST + f2(0); // #F2INST + f3(0); // #F3INST + f4(0); +} + +template +auto Nullptr = nullptr; + +template concept NullTy = Nullptr; +// expected-error@-1{{atomic constraint must be of type 'bool' (found }} +// expected-note@+1{{while checking the satisfaction}} +static_assert(NullTy); + +template +auto Struct = S{}; + +template concept StructTy = Struct; +// expected-error@-1{{atomic constraint must be of type 'bool' (found 'S')}} +// expected-note@+1{{while checking the satisfaction}} +static_assert(StructTy); -- 2.7.4