[clang] 6c29073 - PR45589: Properly decompose overloaded `&&` and `||` operators in
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue May 12 13:45:51 PDT 2020
Author: Richard Smith
Date: 2020-05-12T13:45:45-07:00
New Revision: 6c29073efb0c22045868bc3622f8fc27f43fca41
URL: https://github.com/llvm/llvm-project/commit/6c29073efb0c22045868bc3622f8fc27f43fca41
DIFF: https://github.com/llvm/llvm-project/commit/6c29073efb0c22045868bc3622f8fc27f43fca41.diff
LOG: PR45589: Properly decompose overloaded `&&` and `||` operators in
constraint expressions.
We create overloaded `&&` and `||` operators to hold the possible
unqualified lookup results (if any) when the operands are dependent. We
could avoid building these in some cases (we will never use the stored
lookup results, and it would be better to not store them or perform the
lookups), but in the general case we will probably still need to handle
overloaded operators even with that optimization.
Added:
clang/test/SemaTemplate/constraints.cpp
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaExpr.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 202f0f2c9a14..5adb66dc9ec6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6394,15 +6394,10 @@ class Sema final {
/// A diagnostic is emitted if it is not, false is returned, and
/// PossibleNonPrimary will be set to true if the failure might be due to a
/// non-primary expression being used as an atomic constraint.
- bool CheckConstraintExpression(Expr *CE, Token NextToken = Token(),
+ bool CheckConstraintExpression(const Expr *CE, Token NextToken = Token(),
bool *PossibleNonPrimary = nullptr,
bool IsTrailingRequiresClause = false);
- /// Check whether the given type-dependent expression will be the name of a
- /// function or another callable function-like entity (e.g. a function
- // template or overload set) for any substitution.
- bool IsDependentFunctionNameExpr(Expr *E);
-
private:
/// Caches pairs of template-like decls whose associated constraints were
/// checked for subsumption and whether or not the first's constraints did in
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 290e4cbff4fd..ddd95faebe99 100755
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -28,21 +28,47 @@
using namespace clang;
using namespace sema;
-bool
-Sema::CheckConstraintExpression(Expr *ConstraintExpression, Token NextToken,
- bool *PossibleNonPrimary,
- bool IsTrailingRequiresClause) {
+namespace {
+class LogicalBinOp {
+ OverloadedOperatorKind Op = OO_None;
+ const Expr *LHS = nullptr;
+ const Expr *RHS = nullptr;
+
+public:
+ LogicalBinOp(const Expr *E) {
+ if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+ Op = BinaryOperator::getOverloadedOperator(BO->getOpcode());
+ LHS = BO->getLHS();
+ RHS = BO->getRHS();
+ } else if (auto *OO = dyn_cast<CXXOperatorCallExpr>(E)) {
+ Op = OO->getOperator();
+ LHS = OO->getArg(0);
+ RHS = OO->getArg(1);
+ }
+ }
+
+ bool isAnd() const { return Op == OO_AmpAmp; }
+ bool isOr() const { return Op == OO_PipePipe; }
+ explicit operator bool() const { return isAnd() || isOr(); }
+
+ const Expr *getLHS() const { return LHS; }
+ const Expr *getRHS() const { return RHS; }
+};
+}
+
+bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,
+ Token NextToken, bool *PossibleNonPrimary,
+ bool IsTrailingRequiresClause) {
// C++2a [temp.constr.atomic]p1
// ..E shall be a constant expression of type bool.
ConstraintExpression = ConstraintExpression->IgnoreParenImpCasts();
- if (auto *BinOp = dyn_cast<BinaryOperator>(ConstraintExpression)) {
- if (BinOp->getOpcode() == BO_LAnd || BinOp->getOpcode() == BO_LOr)
- return CheckConstraintExpression(BinOp->getLHS(), NextToken,
- PossibleNonPrimary) &&
- CheckConstraintExpression(BinOp->getRHS(), NextToken,
- PossibleNonPrimary);
+ if (LogicalBinOp BO = ConstraintExpression) {
+ return CheckConstraintExpression(BO.getLHS(), NextToken,
+ PossibleNonPrimary) &&
+ CheckConstraintExpression(BO.getRHS(), NextToken,
+ PossibleNonPrimary);
} else if (auto *C = dyn_cast<ExprWithCleanups>(ConstraintExpression))
return CheckConstraintExpression(C->getSubExpr(), NextToken,
PossibleNonPrimary);
@@ -60,7 +86,7 @@ Sema::CheckConstraintExpression(Expr *ConstraintExpression, Token NextToken,
(NextToken.is(tok::l_paren) &&
(IsTrailingRequiresClause ||
(Type->isDependentType() &&
- IsDependentFunctionNameExpr(ConstraintExpression)) ||
+ isa<UnresolvedLookupExpr>(ConstraintExpression)) ||
Type->isFunctionType() ||
Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
// We have the following case:
@@ -99,39 +125,37 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
AtomicEvaluator &&Evaluator) {
ConstraintExpr = ConstraintExpr->IgnoreParenImpCasts();
- if (auto *BO = dyn_cast<BinaryOperator>(ConstraintExpr)) {
- if (BO->getOpcode() == BO_LAnd || BO->getOpcode() == BO_LOr) {
- if (calculateConstraintSatisfaction(S, BO->getLHS(), Satisfaction,
- Evaluator))
- return true;
+ if (LogicalBinOp BO = ConstraintExpr) {
+ if (calculateConstraintSatisfaction(S, BO.getLHS(), Satisfaction,
+ Evaluator))
+ return true;
- bool IsLHSSatisfied = Satisfaction.IsSatisfied;
+ bool IsLHSSatisfied = Satisfaction.IsSatisfied;
- if (BO->getOpcode() == BO_LOr && IsLHSSatisfied)
- // [temp.constr.op] p3
- // A disjunction is a constraint taking two operands. To determine if
- // a disjunction is satisfied, the satisfaction of the first operand
- // is checked. If that is satisfied, the disjunction is satisfied.
- // Otherwise, the disjunction is satisfied if and only if the second
- // operand is satisfied.
- return false;
+ if (BO.isOr() && IsLHSSatisfied)
+ // [temp.constr.op] p3
+ // A disjunction is a constraint taking two operands. To determine if
+ // a disjunction is satisfied, the satisfaction of the first operand
+ // is checked. If that is satisfied, the disjunction is satisfied.
+ // Otherwise, the disjunction is satisfied if and only if the second
+ // operand is satisfied.
+ return false;
- if (BO->getOpcode() == BO_LAnd && !IsLHSSatisfied)
- // [temp.constr.op] p2
- // A conjunction is a constraint taking two operands. To determine if
- // a conjunction is satisfied, the satisfaction of the first operand
- // is checked. If that is not satisfied, the conjunction is not
- // satisfied. Otherwise, the conjunction is satisfied if and only if
- // the second operand is satisfied.
- return false;
+ if (BO.isAnd() && !IsLHSSatisfied)
+ // [temp.constr.op] p2
+ // A conjunction is a constraint taking two operands. To determine if
+ // a conjunction is satisfied, the satisfaction of the first operand
+ // is checked. If that is not satisfied, the conjunction is not
+ // satisfied. Otherwise, the conjunction is satisfied if and only if
+ // the second operand is satisfied.
+ return false;
- return calculateConstraintSatisfaction(S, BO->getRHS(), Satisfaction,
- std::forward<AtomicEvaluator>(Evaluator));
- }
- }
- else if (auto *C = dyn_cast<ExprWithCleanups>(ConstraintExpr))
+ return calculateConstraintSatisfaction(
+ S, BO.getRHS(), Satisfaction, std::forward<AtomicEvaluator>(Evaluator));
+ } else if (auto *C = dyn_cast<ExprWithCleanups>(ConstraintExpr)) {
return calculateConstraintSatisfaction(S, C->getSubExpr(), Satisfaction,
std::forward<AtomicEvaluator>(Evaluator));
+ }
// An atomic constraint expression
ExprResult SubstitutedAtomicExpr = Evaluator(ConstraintExpr);
@@ -725,19 +749,16 @@ NormalizedConstraint::fromConstraintExpr(Sema &S, NamedDecl *D, const Expr *E) {
// - The normal form of an expression (E) is the normal form of E.
// [...]
E = E->IgnoreParenImpCasts();
- if (auto *BO = dyn_cast<const BinaryOperator>(E)) {
- if (BO->getOpcode() == BO_LAnd || BO->getOpcode() == BO_LOr) {
- auto LHS = fromConstraintExpr(S, D, BO->getLHS());
- if (!LHS)
- return None;
- auto RHS = fromConstraintExpr(S, D, BO->getRHS());
- if (!RHS)
- return None;
+ if (LogicalBinOp BO = E) {
+ auto LHS = fromConstraintExpr(S, D, BO.getLHS());
+ if (!LHS)
+ return None;
+ auto RHS = fromConstraintExpr(S, D, BO.getRHS());
+ if (!RHS)
+ return None;
- return NormalizedConstraint(
- S.Context, std::move(*LHS), std::move(*RHS),
- BO->getOpcode() == BO_LAnd ? CCK_Conjunction : CCK_Disjunction);
- }
+ return NormalizedConstraint(S.Context, std::move(*LHS), std::move(*RHS),
+ BO.isAnd() ? CCK_Conjunction : CCK_Disjunction);
} else if (auto *CSE = dyn_cast<const ConceptSpecializationExpr>(E)) {
const NormalizedConstraint *SubNF;
{
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6f25f71c77dd..e8be40c3a92b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18945,11 +18945,6 @@ ExprResult Sema::ActOnObjCAvailabilityCheckExpr(
ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
}
-bool Sema::IsDependentFunctionNameExpr(Expr *E) {
- assert(E->isTypeDependent());
- return isa<UnresolvedLookupExpr>(E);
-}
-
ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
ArrayRef<Expr *> SubExprs, QualType T) {
// FIXME: enable it for C++, RecoveryExpr is type-dependent to suppress
diff --git a/clang/test/SemaTemplate/constraints.cpp b/clang/test/SemaTemplate/constraints.cpp
new file mode 100644
index 000000000000..0bc4727245f6
--- /dev/null
+++ b/clang/test/SemaTemplate/constraints.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DDEPENDENT_OR
+
+#ifdef DEPENDENT_OR
+// This causes the || below to be a CXXOperatorCallExpr not a BinaryOperator.
+struct A {}; bool operator||(A, A);
+#endif
+
+namespace PR45589 {
+ template<typename T> struct X { static constexpr bool value = T::value; }; // expected-error {{cannot be used prior to '::'}}
+ struct False { static constexpr bool value = false; };
+ struct True { static constexpr bool value = true; };
+
+ template<typename T> concept C = true;
+
+ template<bool B, typename T> constexpr int test = 0;
+ template<bool B, typename T> requires C<T> constexpr int test<B, T> = 1;
+ template<bool B, typename T> requires (B && C<T>) || (X<T>::value && C<T>) constexpr int test<B, T> = 2; // expected-error {{non-constant expression}} expected-note {{subexpression}} expected-note {{instantiation of}} expected-note {{while substituting}}
+ static_assert(test<true, False> == 2);
+ static_assert(test<true, True> == 2);
+ static_assert(test<true, char> == 2); // satisfaction of second term of || not considered
+ static_assert(test<false, False> == 1);
+ static_assert(test<false, True> == 2); // constraints are partially ordered
+ // FIXME: These diagnostics are excessive.
+ static_assert(test<false, char> == 1); // expected-note 2{{while}} expected-note 2{{during}}
+}
More information about the cfe-commits
mailing list