Skip to content

Commit

Permalink
[Sema] When checking for constraint equivalence, do not calculate sat…
Browse files Browse the repository at this point in the history
…isfaction (#74490)

... and only look at equivalence of substituted expressions, not results
of constraint satisfaction.
This is required by the standard when matching redeclarations.

Fixes #74314.

There is already some existing machinery for that in
`TemplateInstantiator` and `Sema` exposed separate functions for
substituting expressions with intention to do that:
- `Sema::SubstExpr` should not evaluate constraints.
- `Sema::SubstConstraintExpr` should.

However, both functions used to be equivalent. Introduce a new function
that does not evaluate constraint and use it when matching declarations.

Also change implementation of `SubstConstraintExpr` to call `SubstExpr`
directly so it's obvious they behave in the same way and add a FIXME to
call out that we might need to revamp this approach in the future.
  • Loading branch information
ilya-biryukov committed Jan 4, 2024
1 parent f45b759 commit f5efa74
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 12 deletions.
6 changes: 4 additions & 2 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -10263,11 +10263,13 @@ class Sema final {
~ConstraintEvalRAII() { TI.setEvaluateConstraints(OldValue); }
};

// Unlike the above, this evaluates constraints, which should only happen at
// 'constraint checking' time.
// Must be used instead of SubstExpr at 'constraint checking' time.
ExprResult
SubstConstraintExpr(Expr *E,
const MultiLevelTemplateArgumentList &TemplateArgs);
// Unlike the above, this does not evaluates constraints.
ExprResult SubstConstraintExprWithoutSatisfaction(
Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs);

/// Substitute the given template arguments into a list of
/// expressions, expanding pack expansions if required.
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Sema/Template.h
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ enum class TemplateSubstitutionKind : char {
const MultiLevelTemplateArgumentList &TemplateArgs;
Sema::LateInstantiatedAttrVec* LateAttrs = nullptr;
LocalInstantiationScope *StartingScope = nullptr;
// Whether to evaluate the C++20 constraints or simply substitute into them.
bool EvaluateConstraints = true;

/// A list of out-of-line class template partial
Expand Down
17 changes: 9 additions & 8 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -771,10 +771,9 @@ namespace {
};
} // namespace

static const Expr *
SubstituteConstraintExpression(Sema &S,
const Sema::TemplateCompareNewDeclInfo &DeclInfo,
const Expr *ConstrExpr) {
static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo,
const Expr *ConstrExpr) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
/*Innermost=*/nullptr,
Expand All @@ -797,8 +796,8 @@ SubstituteConstraintExpression(Sema &S,
std::optional<Sema::CXXThisScopeRAII> ThisScope;
if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
ExprResult SubstConstr =
S.SubstConstraintExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
ExprResult SubstConstr = S.SubstConstraintExprWithoutSatisfaction(
const_cast<clang::Expr *>(ConstrExpr), MLTAL);
if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
return nullptr;
return SubstConstr.get();
Expand All @@ -814,12 +813,14 @@ bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
if (Old && !New.isInvalid() && !New.ContainsDecl(Old) &&
Old->getLexicalDeclContext() != New.getLexicalDeclContext()) {
if (const Expr *SubstConstr =
SubstituteConstraintExpression(*this, Old, OldConstr))
SubstituteConstraintExpressionWithoutSatisfaction(*this, Old,
OldConstr))
OldConstr = SubstConstr;
else
return false;
if (const Expr *SubstConstr =
SubstituteConstraintExpression(*this, New, NewConstr))
SubstituteConstraintExpressionWithoutSatisfaction(*this, New,
NewConstr))
NewConstr = SubstConstr;
else
return false;
Expand Down
22 changes: 20 additions & 2 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,7 @@ namespace {
const MultiLevelTemplateArgumentList &TemplateArgs;
SourceLocation Loc;
DeclarationName Entity;
// Whether to evaluate the C++20 constraints or simply substitute into them.
bool EvaluateConstraints = true;

public:
Expand Down Expand Up @@ -2499,6 +2500,17 @@ TemplateInstantiator::TransformNestedRequirement(
Req->getConstraintExpr()->getBeginLoc(), Req,
Sema::InstantiatingTemplate::ConstraintsCheck{},
Req->getConstraintExpr()->getSourceRange());
if (!getEvaluateConstraints()) {
ExprResult TransConstraint = TransformExpr(Req->getConstraintExpr());
if (TransConstraint.isInvalid() || !TransConstraint.get())
return nullptr;
if (TransConstraint.get()->isInstantiationDependent())
return new (SemaRef.Context)
concepts::NestedRequirement(TransConstraint.get());
ConstraintSatisfaction Satisfaction;
return new (SemaRef.Context) concepts::NestedRequirement(
SemaRef.Context, TransConstraint.get(), Satisfaction);
}

ExprResult TransConstraint;
ConstraintSatisfaction Satisfaction;
Expand Down Expand Up @@ -4093,13 +4105,19 @@ Sema::SubstExpr(Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
ExprResult
Sema::SubstConstraintExpr(Expr *E,
const MultiLevelTemplateArgumentList &TemplateArgs) {
// FIXME: should call SubstExpr directly if this function is equivalent or
// should it be different?
return SubstExpr(E, TemplateArgs);
}

ExprResult Sema::SubstConstraintExprWithoutSatisfaction(
Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
if (!E)
return E;

// This is where we need to make sure we 'know' constraint checking needs to
// happen.
TemplateInstantiator Instantiator(*this, TemplateArgs, SourceLocation(),
DeclarationName());
Instantiator.setEvaluateConstraints(false);
return Instantiator.TransformExpr(E);
}

Expand Down
32 changes: 32 additions & 0 deletions clang/test/SemaTemplate/concepts-out-of-line-def.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,3 +504,35 @@ struct bar {
bar<int> x;
} // namespace GH61763


namespace GH74314 {
template <class T, class U> constexpr bool is_same_v = __is_same(T, U);
template <class T, class U> constexpr bool is_not_same_v = !__is_same(T, U);

template <class Result>
concept something_interesting = requires {
true;
requires is_same_v<int, Result>;
};

template <class T>
struct X {
void foo() requires requires { requires is_not_same_v<T, int>; };
void bar(decltype(requires { requires is_not_same_v<T, int>; }));
};

template <class T>
void X<T>::foo() requires requires { requires something_interesting<T>; } {}
// expected-error@-1{{definition of 'foo' does not match any declaration}}
// expected-note@*{{}}

template <class T>
void X<T>::foo() requires requires { requires is_not_same_v<T, int>; } {} // ok

template <class T>
void X<T>::bar(decltype(requires { requires something_interesting<T>; })) {}
// expected-error@-1{{definition of 'bar' does not match any declaration}}

template <class T>
void X<T>::bar(decltype(requires { requires is_not_same_v<T, int>; })) {}
} // namespace GH74314

0 comments on commit f5efa74

Please sign in to comment.