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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits