[clang] 2f8ac07 - PR50402: Use proper constant evaluation rules for checking constraint
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed May 19 16:03:37 PDT 2021
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 at -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));
More information about the cfe-commits
mailing list