Author: Richard Smith Date: 2021-05-19T16:02:53-07:00 New Revision: 2f8ac0758bbfad72e52ef8602fbbd796e1347784
URL: https://github.com/llvm/llvm-project/commit/2f8ac0758bbfad72e52ef8602fbbd796e1347784 DIFF: https://github.com/llvm/llvm-project/commit/2f8ac0758bbfad72e52ef8602fbbd796e1347784.diff LOG: PR50402: Use proper constant evaluation rules for checking constraint satisfaction. Previously we used the rules for constant folding in a non-constant context, meaning that we'd incorrectly accept foldable non-constant expressions and that std::is_constant_evaluated() would evaluate to false. Added: Modified: clang/lib/Sema/SemaConcept.cpp clang/test/SemaTemplate/concepts.cpp clang/test/SemaTemplate/cxx2a-constraint-caching.cpp clang/test/SemaTemplate/instantiate-requires-clause.cpp Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 592bf5633ea5..b67776b5348d 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -172,9 +172,11 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr, SmallVector<PartialDiagnosticAt, 2> EvaluationDiags; Expr::EvalResult EvalResult; EvalResult.Diag = &EvaluationDiags; - if (!SubstitutedAtomicExpr.get()->EvaluateAsRValue(EvalResult, S.Context)) { - // C++2a [temp.constr.atomic]p1 - // ...E shall be a constant expression of type bool. + if (!SubstitutedAtomicExpr.get()->EvaluateAsConstantExpr(EvalResult, + S.Context) || + !EvaluationDiags.empty()) { + // C++2a [temp.constr.atomic]p1 + // ...E shall be a constant expression of type bool. S.Diag(SubstitutedAtomicExpr.get()->getBeginLoc(), diag::err_non_constant_constraint_expression) << SubstitutedAtomicExpr.get()->getSourceRange(); @@ -183,6 +185,8 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr, return true; } + assert(EvalResult.Val.isInt() && + "evaluating bool expression didn't produce int"); Satisfaction.IsSatisfied = EvalResult.Val.getInt().getBoolValue(); if (!Satisfaction.IsSatisfied) Satisfaction.Details.emplace_back(ConstraintExpr, @@ -214,6 +218,13 @@ static bool calculateConstraintSatisfaction( Sema::SFINAETrap Trap(S); SubstitutedExpression = S.SubstExpr(const_cast<Expr *>(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.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 @@ -523,9 +534,9 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S, diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getRHS(), /*First=*/false); return; - case BO_LAnd: - bool LHSSatisfied; - BO->getLHS()->EvaluateAsBooleanCondition(LHSSatisfied, S.Context); + case BO_LAnd: { + bool LHSSatisfied = + BO->getLHS()->EvaluateKnownConstInt(S.Context).getBoolValue(); if (LHSSatisfied) { // LHS is true, so RHS must be false. diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getRHS(), First); @@ -535,12 +546,13 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S, diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getLHS(), First); // RHS might also be false - bool RHSSatisfied; - BO->getRHS()->EvaluateAsBooleanCondition(RHSSatisfied, S.Context); + bool RHSSatisfied = + BO->getRHS()->EvaluateKnownConstInt(S.Context).getBoolValue(); if (!RHSSatisfied) diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getRHS(), /*First=*/false); return; + } case BO_GE: case BO_LE: case BO_GT: @@ -551,8 +563,12 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S, BO->getRHS()->getType()->isIntegerType()) { Expr::EvalResult SimplifiedLHS; Expr::EvalResult SimplifiedRHS; - BO->getLHS()->EvaluateAsInt(SimplifiedLHS, S.Context); - BO->getRHS()->EvaluateAsInt(SimplifiedRHS, S.Context); + BO->getLHS()->EvaluateAsInt(SimplifiedLHS, S.Context, + Expr::SE_NoSideEffects, + /*InConstantContext=*/true); + BO->getRHS()->EvaluateAsInt(SimplifiedRHS, S.Context, + Expr::SE_NoSideEffects, + /*InConstantContext=*/true); if (!SimplifiedLHS.Diag && ! SimplifiedRHS.Diag) { S.Diag(SubstExpr->getBeginLoc(), diag::note_atomic_constraint_evaluated_to_false_elaborated) diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index 01a2618d0ab1..60c8bc59d33c 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -126,3 +126,31 @@ namespace PackInTypeConstraint { ...); // expected-error {{does not contain any unexpanded}} } } + +namespace BuiltinIsConstantEvaluated { + // Check that we do all satisfaction and diagnostic checks in a constant context. + template<typename T> concept C = __builtin_is_constant_evaluated(); // expected-warning {{always}} + static_assert(C<int>); + + template<typename T> concept D = __builtin_is_constant_evaluated() == true; // expected-warning {{always}} + static_assert(D<int>); + + template<typename T> concept E = __builtin_is_constant_evaluated() == true && // expected-warning {{always}} + false; // expected-note {{'false' evaluated to false}} + static_assert(E<int>); // expected-error {{failed}} expected-note {{because 'int' does not satisfy 'E'}} + + template<typename T> concept F = __builtin_is_constant_evaluated() == false; // expected-warning {{always}} + // expected-note@-1 {{'__builtin_is_constant_evaluated() == false' (1 == 0)}} + static_assert(F<int>); // expected-error {{failed}} expected-note {{because 'int' does not satisfy 'F'}} + + template<typename T> concept G = __builtin_is_constant_evaluated() && // expected-warning {{always}} + false; // expected-note {{'false' evaluated to false}} + static_assert(G<int>); // expected-error {{failed}} expected-note {{because 'int' does not satisfy 'G'}} +} + +namespace NoConstantFolding { + // Ensure we use strict constant evaluation rules when checking satisfaction. + int n; + template <class T> concept C = &n + 3 - 3 == &n; // expected-error {{non-constant expression}} expected-note {{cannot refer to element 3 of non-array object}} + static_assert(C<void>); // expected-note {{while checking}} +} diff --git a/clang/test/SemaTemplate/cxx2a-constraint-caching.cpp b/clang/test/SemaTemplate/cxx2a-constraint-caching.cpp index aa0ad94bef72..62aa0446673e 100644 --- a/clang/test/SemaTemplate/cxx2a-constraint-caching.cpp +++ b/clang/test/SemaTemplate/cxx2a-constraint-caching.cpp @@ -14,7 +14,7 @@ constexpr bool foo() requires (f(T()), true) { return true; } namespace a { struct A {}; - void f(A a); + constexpr void f(A a) {} } static_assert(C<a::A>); @@ -23,7 +23,7 @@ static_assert(foo<a::A>()); namespace a { // This makes calls to f ambiguous, but the second check will still succeed // because the constraint satisfaction results are cached. - void f(A a, int = 2); + constexpr void f(A a, int = 2) {} } #ifdef NO_CACHE static_assert(!C<a::A>); @@ -31,4 +31,4 @@ static_assert(!foo<a::A>()); #else static_assert(C<a::A>); static_assert(foo<a::A>()); -#endif \ No newline at end of file +#endif diff --git a/clang/test/SemaTemplate/instantiate-requires-clause.cpp b/clang/test/SemaTemplate/instantiate-requires-clause.cpp index a4d1b6597201..73dd20d27d22 100644 --- a/clang/test/SemaTemplate/instantiate-requires-clause.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-clause.cpp @@ -62,8 +62,8 @@ static_assert((S3<int>::f(), true)); template<typename T> struct S4 { template<typename> - constexpr void foo() requires (*this, true) { } - constexpr void goo() requires (*this, true) { } + constexpr void foo() requires (decltype(this)(), true) { } + constexpr void goo() requires (decltype(this)(), true) { } }; static_assert((S4<int>{}.foo<int>(), S4<int>{}.goo(), true)); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
