usaxena95 created this revision.
Herald added a project: All.
usaxena95 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixes: https://github.com/llvm/llvm-project/issues/45563
template<class T> concept True = true;
template <class T>
concept C1 = requires (T) {
requires True<typename T::value> || True<T>;
};
template <class T>
constexpr bool foo()
requires True<typename T::value> || True<T> {
return true;
}
static_assert(C1<double>); // Previously failed due to SFINAE error
static_assert(foo<int>()); // but this works fine.
The issue here is the discrepancy between how a nested requirement is evaluated
<https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplateInstantiate.cpp#L2331>
Vs how a non-nested requirement is evaluated
<https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaConcept.cpp#L167-L200>.
This patch makes constraint checking consistent for nested requirement
and trailing requires expressions by reusing the same evaluator.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138914
Files:
clang/include/clang/AST/ExprConcepts.h
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
clang/test/SemaTemplate/instantiate-requires-expr.cpp
Index: clang/test/SemaTemplate/instantiate-requires-expr.cpp
===================================================================
--- clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -24,8 +24,8 @@
template<typename T> requires
false_v<requires (T t) { requires is_same_v<decltype(t), int>; }>
-// expected-note@-1 {{because 'false_v<requires (int t) { requires is_same_v<decltype(t), int>; }>' evaluated to false}}
-// expected-note@-2 {{because 'false_v<requires (char t) { requires is_same_v<decltype(t), int>; }>' evaluated to false}}
+// expected-note@-1 {{because 'false_v<requires (int t) { requires <null expr>; }>' evaluated to false}}
+// expected-note@-2 {{because 'false_v<requires (char t) { requires <null expr>; }>' evaluated to false}}
struct r1 {};
using r1i1 = r1<int>; // expected-error {{constraints not satisfied for class template 'r1' [with T = int]}}
@@ -35,8 +35,8 @@
template<typename... Ts> requires
false_v<requires (Ts... ts) {requires ((sizeof(ts) == 2) && ...);}>
-// expected-note@-1 {{because 'false_v<requires (short ts, unsigned short ts) { requires (sizeof (ts) == 2) && (sizeof (ts) == 2); }>'}}
-// expected-note@-2 {{because 'false_v<requires (short ts) { requires (sizeof (ts) == 2); }>' evaluated to false}}
+// expected-note@-1 {{because 'false_v<requires (short ts, unsigned short ts) { requires <null expr>; }>' evaluated to false}}
+// expected-note@-2 {{because 'false_v<requires (short ts) { requires <null expr>; }>' evaluated to false}}
struct r2 {};
using r2i1 = r2<short, unsigned short>; // expected-error {{constraints not satisfied for class template 'r2' [with Ts = <short, unsigned short>]}}
@@ -44,8 +44,8 @@
template<typename... Ts> requires
false_v<(requires (Ts ts) {requires sizeof(ts) != 0;} && ...)>
-// expected-note@-1 {{because 'false_v<requires (short ts) { requires sizeof (ts) != 0; } && requires (unsigned short ts) { requires sizeof (ts) != 0; }>' evaluated to false}}
-// expected-note@-2 {{because 'false_v<requires (short ts) { requires sizeof (ts) != 0; }>' evaluated to false}}
+// expected-note@-1 {{because 'false_v<requires (short ts) { requires <null expr>; } && requires (unsigned short ts) { requires <null expr>; }>' evaluated to false}}
+// expected-note@-2 {{because 'false_v<requires (short ts) { requires <null expr>; }>' evaluated to false}}
struct r3 {};
using r3i1 = r3<short, unsigned short>; // expected-error {{constraints not satisfied for class template 'r3' [with Ts = <short, unsigned short>]}}
@@ -181,7 +181,7 @@
namespace nested_requirement {
// check that constraint expression is instantiated correctly
- template<typename T> requires false_v<requires { requires sizeof(T) == 2; }> // expected-note{{because 'false_v<requires { requires sizeof(int) == 2; }>' evaluated to false}}
+ template<typename T> requires false_v<requires { requires sizeof(T) == 2; }> // expected-note{{because 'false_v<requires { requires <null expr>; }>' evaluated to false}}
struct r1 {};
using r1i = r1<int>; // expected-error{{constraints not satisfied for class template 'r1' [with T = int]}}
@@ -190,7 +190,7 @@
template<typename T>
struct a {
template<typename U> requires
- (requires { requires sizeof(T::a) == 0; }, false) // expected-note{{because 'requires { requires <<error-expression>>; } , false' evaluated to false}}
+ (requires { requires sizeof(T::a) == 0; }, false) // expected-note{{because 'requires { requires <null expr>; } , false' evaluated to false}}
struct r {};
};
@@ -199,7 +199,7 @@
// Parameter pack inside expr
template<typename... Ts> requires
false_v<(requires { requires sizeof(Ts) == 0; } && ...)>
- // expected-note@-1 {{because 'false_v<requires { requires sizeof(int) == 0; } && requires { requires sizeof(short) == 0; }>' evaluated to false}}
+ // expected-note@-1 {{because 'false_v<requires { requires <null expr>; } && requires { requires <null expr>; }>' evaluated to false}}
struct r2 {};
using r2i = r2<int, short>; // expected-error{{constraints not satisfied for class template 'r2' [with Ts = <int, short>]}}
@@ -208,14 +208,14 @@
// Parameter pack inside multiple requirements
template<typename... Ts> requires
false_v<(requires { requires sizeof(Ts) == 0; sizeof(Ts); } && ...)>
-// expected-note@-1 {{because 'false_v<requires { requires sizeof(int) == 0; sizeof(Ts); } && requires { requires sizeof(short) == 0; sizeof(Ts); }>' evaluated to false}}
+// expected-note@-1 {{because 'false_v<requires { requires <null expr>; sizeof(Ts); } && requires { requires <null expr>; sizeof(Ts); }>' evaluated to false}}
struct r4 {};
using r4i = r4<int, short>; // expected-error{{constraints not satisfied for class template 'r4' [with Ts = <int, short>]}}
template<typename... Ts> requires
false_v<(requires(Ts t) { requires sizeof(t) == 0; t++; } && ...)>
-// expected-note@-1 {{because 'false_v<requires (int t) { requires sizeof (t) == 0; t++; } && requires (short t) { requires sizeof (t) == 0; t++; }>' evaluated to false}}
+// expected-note@-1 {{because 'false_v<requires (int t) { requires <null expr>; t++; } && requires (short t) { requires <null expr>; t++; }>' evaluated to false}}
struct r5 {};
using r5i = r5<int, short>; // expected-error{{constraints not satisfied for class template 'r5' [with Ts = <int, short>]}}
Index: clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
===================================================================
--- clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
@@ -19,7 +19,8 @@
template<typename T>
struct X {
- template<typename U> requires requires (U u) { requires sizeof(u) == sizeof(T); } // expected-note{{because 'sizeof (u) == sizeof(T)' would be invalid: invalid application of 'sizeof' to an incomplete type 'void'}}
+ template<typename U> requires requires (U u) { requires sizeof(u) == sizeof(T); }
+ // expected-note@-1 {{because substituted constraint expression is ill-formed: invalid application of 'sizeof' to an incomplete type 'void'}}
struct r4 {};
};
@@ -41,7 +42,7 @@
template<typename T>
concept C2 = requires (T a) {
requires sizeof(a) == 4; // OK
- requires a == 0; // expected-note{{because 'a == 0' would be invalid: constraint variable 'a' cannot be used in an evaluated context}}
+ requires a == 0; // expected-note{{because substituted constraint expression is ill-formed: constraint variable 'a' cannot be used in an evaluated context}}
};
static_assert(C2<int>); // expected-note{{because 'int' does not satisfy 'C2'}} expected-error{{static assertion failed}}
}
@@ -51,3 +52,90 @@
X.next();
};
+namespace SubstitutionFailureNestedRequires {
+template<class T> concept True = true;
+
+struct S { double value; };
+
+template <class T>
+concept Pipes = requires (T x) {
+ requires True<decltype(x.value)> || True<T>;
+ requires True<T> || True<decltype(x.value)>;
+};
+
+template <class T>
+concept Amps1 = requires (T x) {
+ requires True<decltype(x.value)> && True<T>;
+ // expected-note@-1{{because substituted constraint expression is ill-formed: member reference base type 'int' is not a structure or union}}
+};
+template <class T>
+concept Amps2 = requires (T x) {
+ requires True<T> && True<decltype(x.value)>;
+};
+
+static_assert(Pipes<S>);
+static_assert(Pipes<double>);
+
+static_assert(Amps1<S>);
+static_assert(!Amps1<double>);
+
+static_assert(Amps2<S>);
+static_assert(!Amps2<double>);
+
+template<class T>
+void foo1() requires requires (T x) { // expected-note {{candidate template ignored: constraints not satisfied [with T = int]}}
+ requires
+ True<decltype(x.value)> // expected-note {{because substituted constraint expression is ill-formed: member reference base type 'int' is not a structure or union}}
+ && True<T>;
+} {}
+template<class T> void fooPipes() requires Pipes<T> {}
+template<class T> void fooAmps1() requires Amps1<T> {}
+// expected-note@-1 {{candidate template ignored: constraints not satisfied [with T = int]}} \
+// expected-note@-1 {{because 'int' does not satisfy 'Amps1'}}
+
+void foo() {
+ foo1<S>();
+ foo1<int>(); // expected-error {{no matching function for call to 'foo1'}}
+ fooPipes<S>();
+ fooPipes<int>();
+ fooAmps1<S>();
+ fooAmps1<int>(); // expected-error {{no matching function for call to 'fooAmps1'}}
+}
+
+template<class T>
+concept HasNoValue = requires (T x) {
+ requires !True<decltype(x.value)> && True<T>;
+};
+// FIXME: 'int' does not satisfy 'HasNoValue' currently since `!True<decltype(x.value)>` is an invalid expression.
+// But, in principle, it should be constant-evaluated to true.
+static_assert(!HasNoValue<int>);
+static_assert(!HasNoValue<S>);
+
+template<class T> constexpr bool NotAConceptTrue = true;
+template <class T>
+concept SFinNestedRequires = requires (T x) {
+ // SF in a non-concept specialisation should also be evaluated to false.
+ requires NotAConceptTrue<decltype(x.value)> || NotAConceptTrue<T>;
+};
+static_assert(SFinNestedRequires<int>);
+static_assert(SFinNestedRequires<S>);
+template <class T>
+void foo() requires SFinNestedRequires<T> {}
+void bar() {
+ foo<int>();
+ foo<S>();
+}
+namespace SFINAEWithoutConcept {
+ //Add more TODO(usx).
+template<typename T> struct X { static constexpr bool value = T::value; };
+struct True { static constexpr bool value = true; };
+template<typename T> concept C = true;
+
+template<typename T> requires requires(T) { requires C<T> || X<T>::value; } void foo();
+
+void func() {
+ foo<True>();
+ foo<int>();
+}
+}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2328,8 +2328,12 @@
Req->getConstraintExpr()->getSourceRange());
if (ConstrInst.isInvalid())
return nullptr;
- TransConstraint = TransformExpr(Req->getConstraintExpr());
- if (!TransConstraint.isInvalid()) {
+ llvm::SmallVector<Expr *> Result;
+ if (SemaRef.CheckConstraintSatisfaction(
+ nullptr, {Req->getConstraintExpr()}, Result, TemplateArgs,
+ Req->getConstraintExpr()->getSourceRange(), Satisfaction))
+ TransConstraint = Result[0];
+ if (TransConstraint.isUsable() && !TransConstraint.isInvalid()) {
bool CheckSucceeded =
SemaRef.CheckConstraintExpression(TransConstraint.get());
(void)CheckSucceeded;
@@ -2338,7 +2342,7 @@
"did not produce a SFINAE error");
}
// Use version of CheckConstraintSatisfaction that does no substitutions.
- if (!TransConstraint.isInvalid() &&
+ if (TransConstraint.isUsable() && !TransConstraint.isInvalid() &&
!TransConstraint.get()->isInstantiationDependent() &&
!Trap.hasErrorOccurred()) {
bool CheckFailed = SemaRef.CheckConstraintSatisfaction(
@@ -2355,7 +2359,8 @@
SemaRef.getPrintingPolicy());
}));
}
- if (TransConstraint.get()->isInstantiationDependent())
+ if (TransConstraint.isUsable() &&
+ TransConstraint.get()->isInstantiationDependent())
return new (SemaRef.Context)
concepts::NestedRequirement(TransConstraint.get());
return new (SemaRef.Context) concepts::NestedRequirement(
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -161,7 +161,7 @@
return ExprError();
bool IsLHSSatisfied = Satisfaction.IsSatisfied;
-
+
if (BO.isOr() && IsLHSSatisfied)
// [temp.constr.op] p3
// A disjunction is a constraint taking two operands. To determine if
@@ -286,7 +286,8 @@
// 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())
+ if (SubstitutedExpression.isUsable() &&
+ !SubstitutedExpression.isInvalid())
SubstitutedExpression =
S.PerformContextuallyConvertToBool(SubstitutedExpression.get());
if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {
@@ -316,7 +317,7 @@
Satisfaction.Details.emplace_back(
AtomicExpr,
new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{
- SubstDiag.first, StringRef(Mem, MessageSize)});
+ SubstDiag.first, StringRef(Mem, MessageSize)});
Satisfaction.IsSatisfied = false;
return ExprEmpty();
}
Index: clang/include/clang/AST/ExprConcepts.h
===================================================================
--- clang/include/clang/AST/ExprConcepts.h
+++ clang/include/clang/AST/ExprConcepts.h
@@ -423,12 +423,13 @@
}
NestedRequirement(ASTContext &C, Expr *Constraint,
- const ConstraintSatisfaction &Satisfaction) :
- Requirement(RK_Nested, Constraint->isInstantiationDependent(),
- Constraint->containsUnexpandedParameterPack(),
- Satisfaction.IsSatisfied),
- Value(Constraint),
- Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
+ const ConstraintSatisfaction &Satisfaction)
+ : Requirement(RK_Nested,
+ Constraint && Constraint->isInstantiationDependent(),
+ Constraint && Constraint->containsUnexpandedParameterPack(),
+ Satisfaction.IsSatisfied),
+ Value(Constraint),
+ Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
bool isSubstitutionFailure() const {
return Value.is<SubstitutionDiagnostic *>();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
