Skip to content

Commit

Permalink
[Concepts] Transform constraints of non-template functions to Constan…
Browse files Browse the repository at this point in the history
…tEvaluated

We would previously try to evaluate atomic constraints of non-template functions as-is,
and since they are now unevaluated at first, this would cause incorrect evaluation (bugs #44657, #44656).

Substitute into atomic constraints of non-template functions as we would atomic constraints
of template functions, in order to rebuild the expressions in a constant-evaluated context.

(cherry picked from commit 713562f)
  • Loading branch information
saarraz committed Jan 25, 2020
1 parent 0df1362 commit c21e178
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 62 deletions.
6 changes: 3 additions & 3 deletions clang/include/clang/AST/ASTConcept.h
Expand Up @@ -29,14 +29,14 @@ class ConceptSpecializationExpr;
class ConstraintSatisfaction : public llvm::FoldingSetNode {
// The template-like entity that 'owns' the constraint checked here (can be a
// constrained entity or a concept).
NamedDecl *ConstraintOwner = nullptr;
const NamedDecl *ConstraintOwner = nullptr;
llvm::SmallVector<TemplateArgument, 4> TemplateArgs;

public:

ConstraintSatisfaction() = default;

ConstraintSatisfaction(NamedDecl *ConstraintOwner,
ConstraintSatisfaction(const NamedDecl *ConstraintOwner,
ArrayRef<TemplateArgument> TemplateArgs) :
ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs.begin(),
TemplateArgs.end()) { }
Expand All @@ -57,7 +57,7 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
}

static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C,
NamedDecl *ConstraintOwner,
const NamedDecl *ConstraintOwner,
ArrayRef<TemplateArgument> TemplateArgs);
};

Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -4683,6 +4683,8 @@ def note_checking_constraints_for_var_spec_id_here : Note<
def note_checking_constraints_for_class_spec_id_here : Note<
"while checking constraint satisfaction for class template partial "
"specialization '%0' required here">;
def note_checking_constraints_for_function_here : Note<
"while checking constraint satisfaction for function '%0' required here">;
def note_constraint_substitution_here : Note<
"while substituting template arguments into constraint expression here">;
def note_constraint_normalization_here : Note<
Expand Down
13 changes: 12 additions & 1 deletion clang/include/clang/Sema/Sema.h
Expand Up @@ -6275,7 +6275,7 @@ class Sema final {
/// \returns true if an error occurred and satisfaction could not be checked,
/// false otherwise.
bool CheckConstraintSatisfaction(
NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
ArrayRef<TemplateArgument> TemplateArgs,
SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction);

Expand All @@ -6288,6 +6288,17 @@ class Sema final {
bool CheckConstraintSatisfaction(const Expr *ConstraintExpr,
ConstraintSatisfaction &Satisfaction);

/// Check whether the given function decl's trailing requires clause is
/// satisfied, if any. Returns false and updates Satisfaction with the
/// satisfaction verdict if successful, emits a diagnostic and returns true if
/// an error occured and satisfaction could not be determined.
///
/// \returns true if an error occurred, false otherwise.
bool CheckFunctionConstraints(const FunctionDecl *FD,
ConstraintSatisfaction &Satisfaction,
SourceLocation UsageLoc = SourceLocation());


/// \brief Ensure that the given template arguments satisfy the constraints
/// associated with the given template, emitting a diagnostic if they do not.
///
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/AST/ASTConcept.cpp
Expand Up @@ -59,8 +59,8 @@ ASTConstraintSatisfaction::Create(const ASTContext &C,
}

void ConstraintSatisfaction::Profile(
llvm::FoldingSetNodeID &ID, const ASTContext &C, NamedDecl *ConstraintOwner,
ArrayRef<TemplateArgument> TemplateArgs) {
llvm::FoldingSetNodeID &ID, const ASTContext &C,
const NamedDecl *ConstraintOwner, ArrayRef<TemplateArgument> TemplateArgs) {
ID.AddPointer(ConstraintOwner);
ID.AddInteger(TemplateArgs.size());
for (auto &Arg : TemplateArgs)
Expand Down
58 changes: 31 additions & 27 deletions clang/lib/Sema/SemaConcept.cpp
Expand Up @@ -167,9 +167,8 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
return false;
}

template <typename TemplateDeclT>
static bool calculateConstraintSatisfaction(
Sema &S, TemplateDeclT *Template, ArrayRef<TemplateArgument> TemplateArgs,
Sema &S, const NamedDecl *Template, ArrayRef<TemplateArgument> TemplateArgs,
SourceLocation TemplateNameLoc, MultiLevelTemplateArgumentList &MLTAL,
const Expr *ConstraintExpr, ConstraintSatisfaction &Satisfaction) {
return calculateConstraintSatisfaction(
Expand All @@ -182,8 +181,9 @@ static bool calculateConstraintSatisfaction(
{
TemplateDeductionInfo Info(TemplateNameLoc);
Sema::InstantiatingTemplate Inst(S, AtomicExpr->getBeginLoc(),
Sema::InstantiatingTemplate::ConstraintSubstitution{}, Template,
Info, AtomicExpr->getSourceRange());
Sema::InstantiatingTemplate::ConstraintSubstitution{},
const_cast<NamedDecl *>(Template), Info,
AtomicExpr->getSourceRange());
if (Inst.isInvalid())
return ExprError();
// We do not want error diagnostics escaping here.
Expand Down Expand Up @@ -230,8 +230,7 @@ static bool calculateConstraintSatisfaction(
});
}

template<typename TemplateDeclT>
static bool CheckConstraintSatisfaction(Sema &S, TemplateDeclT *Template,
static bool CheckConstraintSatisfaction(Sema &S, const NamedDecl *Template,
ArrayRef<const Expr *> ConstraintExprs,
ArrayRef<TemplateArgument> TemplateArgs,
SourceRange TemplateIDRange,
Expand All @@ -249,8 +248,8 @@ static bool CheckConstraintSatisfaction(Sema &S, TemplateDeclT *Template,
}

Sema::InstantiatingTemplate Inst(S, TemplateIDRange.getBegin(),
Sema::InstantiatingTemplate::ConstraintsCheck{}, Template, TemplateArgs,
TemplateIDRange);
Sema::InstantiatingTemplate::ConstraintsCheck{},
const_cast<NamedDecl *>(Template), TemplateArgs, TemplateIDRange);
if (Inst.isInvalid())
return true;

Expand All @@ -273,7 +272,7 @@ static bool CheckConstraintSatisfaction(Sema &S, TemplateDeclT *Template,
}

bool Sema::CheckConstraintSatisfaction(
NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
ArrayRef<TemplateArgument> TemplateArgs, SourceRange TemplateIDRange,
ConstraintSatisfaction &OutSatisfaction) {
if (ConstraintExprs.empty()) {
Expand All @@ -284,7 +283,8 @@ bool Sema::CheckConstraintSatisfaction(
llvm::FoldingSetNodeID ID;
void *InsertPos;
ConstraintSatisfaction *Satisfaction = nullptr;
if (LangOpts.ConceptSatisfactionCaching) {
bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
if (ShouldCache) {
ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
if (Satisfaction) {
Expand All @@ -295,27 +295,15 @@ bool Sema::CheckConstraintSatisfaction(
} else {
Satisfaction = &OutSatisfaction;
}
bool Failed;
if (auto *T = dyn_cast<TemplateDecl>(Template))
Failed = ::CheckConstraintSatisfaction(*this, T, ConstraintExprs,
TemplateArgs, TemplateIDRange,
*Satisfaction);
else if (auto *P =
dyn_cast<ClassTemplatePartialSpecializationDecl>(Template))
Failed = ::CheckConstraintSatisfaction(*this, P, ConstraintExprs,
TemplateArgs, TemplateIDRange,
*Satisfaction);
else
Failed = ::CheckConstraintSatisfaction(
*this, cast<VarTemplatePartialSpecializationDecl>(Template),
ConstraintExprs, TemplateArgs, TemplateIDRange, *Satisfaction);
if (Failed) {
if (LangOpts.ConceptSatisfactionCaching)
if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
TemplateArgs, TemplateIDRange,
*Satisfaction)) {
if (ShouldCache)
delete Satisfaction;
return true;
}

if (LangOpts.ConceptSatisfactionCaching) {
if (ShouldCache) {
// We cannot use InsertNode here because CheckConstraintSatisfaction might
// have invalidated it.
SatisfactionCache.InsertNode(Satisfaction);
Expand All @@ -333,6 +321,22 @@ bool Sema::CheckConstraintSatisfaction(const Expr *ConstraintExpr,
});
}

bool Sema::CheckFunctionConstraints(const FunctionDecl *FD,
ConstraintSatisfaction &Satisfaction,
SourceLocation UsageLoc) {
const Expr *RC = FD->getTrailingRequiresClause();
assert(!RC->isInstantiationDependent() &&
"CheckFunctionConstraints can only be used with functions with "
"non-dependent constraints");
// We substitute with empty arguments in order to rebuild the atomic
// constraint in a constant-evaluated context.
// FIXME: Should this be a dedicated TreeTransform?
return CheckConstraintSatisfaction(
FD, {RC}, /*TemplateArgs=*/{},
SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
Satisfaction);
}

bool Sema::EnsureTemplateArgumentListConstraints(
TemplateDecl *TD, ArrayRef<TemplateArgument> TemplateArgs,
SourceRange TemplateIDRange) {
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaExpr.cpp
Expand Up @@ -335,7 +335,8 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
if (Expr *RC = FD->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
bool Failed = CheckConstraintSatisfaction(RC, Satisfaction);
bool Failed = CheckConstraintSatisfaction(FD, {RC}, /*TemplateArgs=*/{},
SourceRange(Loc), Satisfaction);
if (Failed)
// A diagnostic will have already been generated (non-constant
// constraint expression, for example)
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaExprCXX.cpp
Expand Up @@ -8487,7 +8487,8 @@ concepts::NestedRequirement *
Sema::BuildNestedRequirement(Expr *Constraint) {
ConstraintSatisfaction Satisfaction;
if (!Constraint->isInstantiationDependent() &&
CheckConstraintSatisfaction(Constraint, Satisfaction))
CheckConstraintSatisfaction(nullptr, {Constraint}, /*TemplateArgs=*/{},
Constraint->getSourceRange(), Satisfaction))
return nullptr;
return new (Context) concepts::NestedRequirement(Context, Constraint,
Satisfaction);
Expand Down
20 changes: 9 additions & 11 deletions clang/lib/Sema/SemaOverload.cpp
Expand Up @@ -6291,9 +6291,9 @@ void Sema::AddOverloadCandidate(
return;
}

if (Expr *RequiresClause = Function->getTrailingRequiresClause()) {
if (Function->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
if (CheckConstraintSatisfaction(RequiresClause, Satisfaction) ||
if (CheckFunctionConstraints(Function, Satisfaction) ||
!Satisfaction.IsSatisfied) {
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_constraints_not_satisfied;
Expand Down Expand Up @@ -6808,9 +6808,9 @@ Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
return;
}

if (Expr *RequiresClause = Method->getTrailingRequiresClause()) {
if (Method->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
if (CheckConstraintSatisfaction(RequiresClause, Satisfaction) ||
if (CheckFunctionConstraints(Method, Satisfaction) ||
!Satisfaction.IsSatisfied) {
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_constraints_not_satisfied;
Expand Down Expand Up @@ -7204,10 +7204,9 @@ void Sema::AddConversionCandidate(
return;
}

Expr *RequiresClause = Conversion->getTrailingRequiresClause();
if (RequiresClause) {
if (Conversion->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
if (CheckConstraintSatisfaction(RequiresClause, Satisfaction) ||
if (CheckFunctionConstraints(Conversion, Satisfaction) ||
!Satisfaction.IsSatisfied) {
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_constraints_not_satisfied;
Expand Down Expand Up @@ -9947,9 +9946,9 @@ static bool checkAddressOfFunctionIsAvailable(Sema &S, const FunctionDecl *FD,
return false;
}

if (const Expr *RC = FD->getTrailingRequiresClause()) {
if (FD->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
if (S.CheckConstraintSatisfaction(RC, Satisfaction))
if (S.CheckFunctionConstraints(FD, Satisfaction, Loc))
return false;
if (!Satisfaction.IsSatisfied) {
if (Complain) {
Expand Down Expand Up @@ -10974,8 +10973,7 @@ static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand,
<< (unsigned)FnKindPair.first << (unsigned)ocs_non_template
<< FnDesc /* Ignored */;
ConstraintSatisfaction Satisfaction;
if (S.CheckConstraintSatisfaction(Fn->getTrailingRequiresClause(),
Satisfaction))
if (S.CheckFunctionConstraints(Fn, Satisfaction))
break;
S.DiagnoseUnsatisfiedConstraint(Satisfaction);
}
Expand Down
17 changes: 13 additions & 4 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Expand Up @@ -763,21 +763,30 @@ void Sema::PrintInstantiationStack() {

case CodeSynthesisContext::ConstraintsCheck: {
unsigned DiagID = 0;
if (!Active->Entity) {
Diags.Report(Active->PointOfInstantiation,
diag::note_nested_requirement_here)
<< Active->InstantiationRange;
break;
}
if (isa<ConceptDecl>(Active->Entity))
DiagID = diag::note_concept_specialization_here;
else if (isa<TemplateDecl>(Active->Entity))
DiagID = diag::note_checking_constraints_for_template_id_here;
else if (isa<VarTemplatePartialSpecializationDecl>(Active->Entity))
DiagID = diag::note_checking_constraints_for_var_spec_id_here;
else {
assert(isa<ClassTemplatePartialSpecializationDecl>(Active->Entity));
else if (isa<ClassTemplatePartialSpecializationDecl>(Active->Entity))
DiagID = diag::note_checking_constraints_for_class_spec_id_here;
else {
assert(isa<FunctionDecl>(Active->Entity));
DiagID = diag::note_checking_constraints_for_function_here;
}
SmallVector<char, 128> TemplateArgsStr;
llvm::raw_svector_ostream OS(TemplateArgsStr);
cast<NamedDecl>(Active->Entity)->printName(OS);
printTemplateArgumentList(OS, Active->template_arguments(),
getPrintingPolicy());
if (!isa<FunctionDecl>(Active->Entity))
printTemplateArgumentList(OS, Active->template_arguments(),
getPrintingPolicy());
Diags.Report(Active->PointOfInstantiation, DiagID) << OS.str()
<< Active->InstantiationRange;
break;
Expand Down
60 changes: 48 additions & 12 deletions clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp
Expand Up @@ -3,15 +3,51 @@
// Make sure constraint expressions are unevaluated before being substituted
// into during satisfaction checking.

template<typename T> constexpr int f() { return T::value; }
template<typename T> concept Foo = false && (f<int>(), true);
bool k = Foo<int>;
template<typename T> requires false && (f<int>(), true) struct S {};
// expected-note@-1{{because}}
using s = S<int>; // expected-error {{constraints not satisfied}}
template<typename T> void foo() requires false && (f<int>(), true) { };
// expected-note@-1{{because}} expected-note@-1{{candidate template ignored}}
int a = (foo<int>(), 0); // expected-error{{no matching function}}
template<typename T> void bar() requires requires { requires false && (f<int>(), true); } { };
// expected-note@-1{{because}} expected-note@-1{{candidate template ignored}}
int b = (bar<int>(), 0); // expected-error{{no matching function}}
template<typename T> constexpr bool f = T::value;
// expected-error@-1 4{{type}}

namespace unevaluated {
template<typename T> concept Foo = false && f<int>;
bool k = Foo<int>;
template<typename T> requires false && f<int> struct S {};
// expected-note@-1{{because}}
using s = S<int>; // expected-error {{constraints not satisfied}}
template<typename T> void foo() requires false && f<int> { };
// expected-note@-1{{because}} expected-note@-1{{candidate template ignored}}
int a = (foo<int>(), 0); // expected-error{{no matching function}}
template<typename T> void bar() requires requires { requires false && f<int>; } { };
// expected-note@-1{{because}} expected-note@-1{{candidate template ignored}}
int b = (bar<int>(), 0); // expected-error{{no matching function}}
template<typename T> struct M { static void foo() requires false && f<int> { }; };
// expected-note@-1{{because}}
int c = (M<int>::foo(), 0);
// expected-error@-1{{invalid reference to function 'foo': constraints not satisfied}}
}

namespace constant_evaluated {
template<typename T> requires f<int[0]> struct S {};
// expected-note@-1{{in instantiation of}} expected-note@-1{{while substituting}} \
expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}} \
expected-note@-1{{subexpression not valid}}
using s = S<int>;
// expected-note@-1 2{{while checking}}
template<typename T> void foo() requires f<int[1]> { };
// expected-note@-1{{in instantiation}} expected-note@-1{{while substituting}} \
expected-note@-1{{candidate template ignored}} expected-note@-1{{subexpression not valid}} \
expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
int a = (foo<int>(), 0);
// expected-note@-1 2{{while checking}} expected-error@-1{{no matching function}} \
expected-note@-1 2{{in instantiation}}
template<typename T> void bar() requires requires { requires f<int[2]>; } { };
// expected-note@-1{{in instantiation}} expected-note@-1{{subexpression not valid}} \
expected-note@-1{{while substituting}} \
expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}} \
expected-note@-1 2{{while checking the satisfaction of nested requirement}}
int b = (bar<int>(), 0);
template<typename T> struct M { static void foo() requires f<int[3]> { }; };
// expected-note@-1{{in instantiation}} expected-note@-1{{subexpression not valid}} \
expected-note@-1{{while substituting}} \
expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
int c = (M<int>::foo(), 0);
// expected-note@-1 2{{while checking}}
}

0 comments on commit c21e178

Please sign in to comment.