Differential D119544 Diff 417948 clang/lib/Sema/SemaConcept.cpp

Changeset View

Standalone View

View Options

clang/lib/Sema/SemaConcept.cpp

Show All 24 Lines

#include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/PointerUnion.h"

#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringExtras.h"

using namespace clang; using namespace clang;

using namespace sema; using namespace sema;

namespace { namespace {

class LogicalBinOp { class LogicalBinOp {

SourceLocation Loc;

OverloadedOperatorKind Op = OO_None; OverloadedOperatorKind Op = OO_None;

const Expr *LHS = nullptr; const Expr *LHS = nullptr;

const Expr *RHS = nullptr; const Expr *RHS = nullptr;

public: public:

LogicalBinOp(const Expr *E) { LogicalBinOp(const Expr *E) {

if (auto *BO = dyn_cast<BinaryOperator>(E)) { if (auto *BO = dyn_cast<BinaryOperator>(E)) {

Op = BinaryOperator::getOverloadedOperator(BO->getOpcode()); Op = BinaryOperator::getOverloadedOperator(BO->getOpcode());

LHS = BO->getLHS(); LHS = BO->getLHS();

RHS = BO->getRHS(); RHS = BO->getRHS();

Loc = BO->getExprLoc();

} else if (auto *OO = dyn_cast<CXXOperatorCallExpr>(E)) { } else if (auto *OO = dyn_cast<CXXOperatorCallExpr>(E)) {

// If OO is not || or && it might not have exactly 2 arguments. // If OO is not || or && it might not have exactly 2 arguments.

if (OO->getNumArgs() == 2) { if (OO->getNumArgs() == 2) {

Op = OO->getOperator(); Op = OO->getOperator();

LHS = OO->getArg(0); LHS = OO->getArg(0);

RHS = OO->getArg(1); RHS = OO->getArg(1);

Loc = OO->getOperatorLoc();

} }

bool isAnd() const { return Op == OO_AmpAmp; } bool isAnd() const { return Op == OO_AmpAmp; }

bool isOr() const { return Op == OO_PipePipe; } bool isOr() const { return Op == OO_PipePipe; }

explicit operator bool() const { return isAnd() || isOr(); } explicit operator bool() const { return isAnd() || isOr(); }

const Expr *getLHS() const { return LHS; } const Expr *getLHS() const { return LHS; }

const Expr *getRHS() const { return RHS; } const Expr *getRHS() const { return RHS; }

ExprResult recreateBinOp(Sema &SemaRef, ExprResult LHS) {

return recreateBinOp(SemaRef, LHS, const_cast<Expr *>(getRHS()));

}

ExprResult recreateBinOp(Sema &SemaRef, ExprResult LHS, ExprResult RHS) {

assert((isAnd() || isOr()) && "Not the right kind of op?");

assert((!LHS.isInvalid() && !RHS.isInvalid()) && "not good expressions?");

assert(LHS.isUsable() && RHS.isUsable() && "Side not usable?");

ChuanqiXuUnsubmitted

Done

assert((!LHS.isInvalid() && !RHS.isInvalid()) && "not good expressions?");

- assert(LHS.isUsable() && RHS.isUsable() && "Side not usable?");

+ if (LHS.isUsable() || RHS.isUsable())

+ return ExprEmpty();

// We should just be able to 'normalize' these to the builtin Binary

I feel like the usage of the API could be further simplified.

ChuanqiXu: I feel like the usage of the API could be further simplified.

erichkeaneAuthorUnsubmitted

Done

Heh, the suggestion is inverted slightly (those should be !isUsable) which caught me for a while :) Either way, I think it is a good suggestion.

erichkeane: Heh, the suggestion is inverted slightly (those should be `!isUsable`) which caught me for a…

ChuanqiXuUnsubmitted

Not Done

Oh, my bad. It is a typo.

ChuanqiXu: Oh, my bad. It is a typo.

// We should just be able to 'normalize' these to the builtin Binary

// Operator, since that is how they are evaluated in constriant checks.

return BinaryOperator::Create(SemaRef.Context, LHS.get(), RHS.get(),

BinaryOperator::getOverloadedOpcode(Op),

SemaRef.Context.BoolTy, VK_PRValue,

OK_Ordinary, Loc, FPOptionsOverride{});

}

}; };

} }

bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression, bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,

Token NextToken, bool *PossibleNonPrimary, Token NextToken, bool *PossibleNonPrimary,

bool IsTrailingRequiresClause) { bool IsTrailingRequiresClause) {

// C++2a [temp.constr.atomic]p1 // C++2a [temp.constr.atomic]p1

// ..E shall be a constant expression of type bool. // ..E shall be a constant expression of type bool.

▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,

} }

if (PossibleNonPrimary) if (PossibleNonPrimary)

*PossibleNonPrimary = false; *PossibleNonPrimary = false;

return true; return true;

} }

template <typename AtomicEvaluator> template <typename AtomicEvaluator>

static bool static ExprResult

calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr, calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,

ConstraintSatisfaction &Satisfaction, ConstraintSatisfaction &Satisfaction,

AtomicEvaluator &&Evaluator) { AtomicEvaluator &&Evaluator) {

ConstraintExpr = ConstraintExpr->IgnoreParenImpCasts(); ConstraintExpr = ConstraintExpr->IgnoreParenImpCasts();

if (LogicalBinOp BO = ConstraintExpr) { if (LogicalBinOp BO = ConstraintExpr) {

if (calculateConstraintSatisfaction(S, BO.getLHS(), Satisfaction, ExprResult LHSRes = calculateConstraintSatisfaction(

Evaluator)) S, BO.getLHS(), Satisfaction, Evaluator);

return true;

if (LHSRes.isInvalid())

return ExprError();

bool IsLHSSatisfied = Satisfaction.IsSatisfied; bool IsLHSSatisfied = Satisfaction.IsSatisfied;

if (BO.isOr() && IsLHSSatisfied) if (BO.isOr() && IsLHSSatisfied)

// [temp.constr.op] p3 // [temp.constr.op] p3

// A disjunction is a constraint taking two operands. To determine if // A disjunction is a constraint taking two operands. To determine if

// a disjunction is satisfied, the satisfaction of the first operand // a disjunction is satisfied, the satisfaction of the first operand

// is checked. If that is satisfied, the disjunction is satisfied. // is checked. If that is satisfied, the disjunction is satisfied.

// Otherwise, the disjunction is satisfied if and only if the second // Otherwise, the disjunction is satisfied if and only if the second

// operand is satisfied. // operand is satisfied.

return false; return LHSRes.isUsable() ? BO.recreateBinOp(S, LHSRes) : ExprEmpty();

//return BO.recreateBinOp(S, LHSRes);

Lint: Pre-merge checks

clang-format: please reformat the code

-      //return BO.recreateBinOp(S, LHSRes);
+    // return BO.recreateBinOp(S, LHSRes);

Lint: Pre-merge checks: clang-format: please reformat the code ``` - //return BO.recreateBinOp(S, LHSRes); + //…

ChuanqiXuUnsubmitted

Done

We should delete this one.

ChuanqiXu: We should delete this one.

ChuanqiXuUnsubmitted

Not Done

// operand is satisfied.

- return LHSRes.isUsable() ? BO.recreateBinOp(S, LHSRes) : ExprEmpty();

+ return BO.recreateBinOp(S, LHSRes);

if (BO.isAnd() && !IsLHSSatisfied)

ChuanqiXu:

if (BO.isAnd() && !IsLHSSatisfied) if (BO.isAnd() && !IsLHSSatisfied)

// [temp.constr.op] p2 // [temp.constr.op] p2

// A conjunction is a constraint taking two operands. To determine if // A conjunction is a constraint taking two operands. To determine if

// a conjunction is satisfied, the satisfaction of the first operand // a conjunction is satisfied, the satisfaction of the first operand

// is checked. If that is not satisfied, the conjunction is not // is checked. If that is not satisfied, the conjunction is not

// satisfied. Otherwise, the conjunction is satisfied if and only if // satisfied. Otherwise, the conjunction is satisfied if and only if

// the second operand is satisfied. // the second operand is satisfied.

return false; return LHSRes.isUsable() ? BO.recreateBinOp(S, LHSRes) : ExprEmpty();

// return BO.recreateBinOp(S, LHSRes);

Lint: Pre-merge checks

clang-format: please reformat the code

-      // return BO.recreateBinOp(S, LHSRes);
+    // return BO.recreateBinOp(S, LHSRes);

Lint: Pre-merge checks: clang-format: please reformat the code ``` - // return BO.recreateBinOp(S, LHSRes); +…

ChuanqiXuUnsubmitted

Done

ditto

ChuanqiXu: ditto

ChuanqiXuUnsubmitted

Not Done

// the second operand is satisfied.

return LHSRes.isUsable() ? BO.recreateBinOp(S, LHSRes) : ExprEmpty();

+ return BO.recreateBinOp(S, LHSRes);

ExprResult RHSRes = calculateConstraintSatisfaction(

ChuanqiXu:

return calculateConstraintSatisfaction( ExprResult RHSRes = calculateConstraintSatisfaction(

S, BO.getRHS(), Satisfaction, std::forward<AtomicEvaluator>(Evaluator)); S, BO.getRHS(), Satisfaction, std::forward<AtomicEvaluator>(Evaluator));

if (RHSRes.isInvalid())

return ExprError();

if (!LHSRes.isUsable() || !RHSRes.isUsable())

return ExprEmpty();

ChuanqiXuUnsubmitted

Not Done

return ExprError();

- if (!LHSRes.isUsable() || !RHSRes.isUsable())

- return ExprEmpty();

return BO.recreateBinOp(S, LHSRes, RHSRes);

ChuanqiXu:

return BO.recreateBinOp(S, LHSRes, RHSRes);

} else if (auto *C = dyn_cast<ExprWithCleanups>(ConstraintExpr)) { } else if (auto *C = dyn_cast<ExprWithCleanups>(ConstraintExpr)) {

return calculateConstraintSatisfaction(S, C->getSubExpr(), Satisfaction, // These aren't evaluated, so we don't care about cleanups, so we can just

// evaluate these as if the cleanups didn't exist.

return calculateConstraintSatisfaction(

S, C->getSubExpr(), Satisfaction,

std::forward<AtomicEvaluator>(Evaluator)); std::forward<AtomicEvaluator>(Evaluator));

} }

// An atomic constraint expression // An atomic constraint expression

ExprResult SubstitutedAtomicExpr = Evaluator(ConstraintExpr); ExprResult SubstitutedAtomicExpr = Evaluator(ConstraintExpr);

if (SubstitutedAtomicExpr.isInvalid()) if (SubstitutedAtomicExpr.isInvalid())

return true; return ExprError();

if (!SubstitutedAtomicExpr.isUsable()) if (!SubstitutedAtomicExpr.isUsable())

// Evaluator has decided satisfaction without yielding an expression. // Evaluator has decided satisfaction without yielding an expression.

return false; return ExprEmpty();

EnterExpressionEvaluationContext ConstantEvaluated( EnterExpressionEvaluationContext ConstantEvaluated(

S, Sema::ExpressionEvaluationContext::ConstantEvaluated); S, Sema::ExpressionEvaluationContext::ConstantEvaluated);

SmallVector<PartialDiagnosticAt, 2> EvaluationDiags; SmallVector<PartialDiagnosticAt, 2> EvaluationDiags;

Expr::EvalResult EvalResult; Expr::EvalResult EvalResult;

EvalResult.Diag = &EvaluationDiags; EvalResult.Diag = &EvaluationDiags;

if (!SubstitutedAtomicExpr.get()->EvaluateAsConstantExpr(EvalResult, if (!SubstitutedAtomicExpr.get()->EvaluateAsConstantExpr(EvalResult,

S.Context) || S.Context) ||

!EvaluationDiags.empty()) { !EvaluationDiags.empty()) {

// C++2a [temp.constr.atomic]p1 // C++2a [temp.constr.atomic]p1

// ...E shall be a constant expression of type bool. // ...E shall be a constant expression of type bool.

S.Diag(SubstitutedAtomicExpr.get()->getBeginLoc(), S.Diag(SubstitutedAtomicExpr.get()->getBeginLoc(),

diag::err_non_constant_constraint_expression) diag::err_non_constant_constraint_expression)

<< SubstitutedAtomicExpr.get()->getSourceRange(); << SubstitutedAtomicExpr.get()->getSourceRange();

for (const PartialDiagnosticAt &PDiag : EvaluationDiags) for (const PartialDiagnosticAt &PDiag : EvaluationDiags)

S.Diag(PDiag.first, PDiag.second); S.Diag(PDiag.first, PDiag.second);

return true; return ExprError();

} }

assert(EvalResult.Val.isInt() && assert(EvalResult.Val.isInt() &&

"evaluating bool expression didn't produce int"); "evaluating bool expression didn't produce int");

Satisfaction.IsSatisfied = EvalResult.Val.getInt().getBoolValue(); Satisfaction.IsSatisfied = EvalResult.Val.getInt().getBoolValue();

if (!Satisfaction.IsSatisfied) if (!Satisfaction.IsSatisfied)

Satisfaction.Details.emplace_back(ConstraintExpr, Satisfaction.Details.emplace_back(ConstraintExpr,

SubstitutedAtomicExpr.get()); SubstitutedAtomicExpr.get());

return false; return SubstitutedAtomicExpr;

} }

static bool calculateConstraintSatisfaction( static ExprResult calculateConstraintSatisfaction(

Sema &S, const NamedDecl *Template, ArrayRef<TemplateArgument> TemplateArgs, Sema &S, const NamedDecl *Template, SourceLocation TemplateNameLoc,

SourceLocation TemplateNameLoc, MultiLevelTemplateArgumentList &MLTAL, const MultiLevelTemplateArgumentList &MLTAL, const Expr *ConstraintExpr,

const Expr *ConstraintExpr, ConstraintSatisfaction &Satisfaction) { ConstraintSatisfaction &Satisfaction) {

return calculateConstraintSatisfaction( return calculateConstraintSatisfaction(

S, ConstraintExpr, Satisfaction, [&](const Expr *AtomicExpr) { S, ConstraintExpr, Satisfaction, [&](const Expr *AtomicExpr) {

EnterExpressionEvaluationContext ConstantEvaluated( EnterExpressionEvaluationContext ConstantEvaluated(

S, Sema::ExpressionEvaluationContext::ConstantEvaluated); S, Sema::ExpressionEvaluationContext::ConstantEvaluated);

// Atomic constraint - substitute arguments and check satisfaction. // Atomic constraint - substitute arguments and check satisfaction.

ExprResult SubstitutedExpression; ExprResult SubstitutedExpression;

{ {

TemplateDeductionInfo Info(TemplateNameLoc); TemplateDeductionInfo Info(TemplateNameLoc);

Sema::InstantiatingTemplate Inst(S, AtomicExpr->getBeginLoc(), Sema::InstantiatingTemplate Inst(S, AtomicExpr->getBeginLoc(),

Sema::InstantiatingTemplate::ConstraintSubstitution{}, Sema::InstantiatingTemplate::ConstraintSubstitution{},

const_cast<NamedDecl *>(Template), Info, const_cast<NamedDecl *>(Template), Info,

AtomicExpr->getSourceRange()); AtomicExpr->getSourceRange());

if (Inst.isInvalid()) if (Inst.isInvalid())

return ExprError(); return ExprError();

// We do not want error diagnostics escaping here. // We do not want error diagnostics escaping here.

Sema::SFINAETrap Trap(S); Sema::SFINAETrap Trap(S);

SubstitutedExpression = S.SubstExpr(const_cast<Expr *>(AtomicExpr), SubstitutedExpression =

MLTAL); S.SubstConstraintExpr(const_cast<Expr *>(AtomicExpr), MLTAL);

// Substitution might have stripped off a contextual conversion to // Substitution might have stripped off a contextual conversion to

// bool if this is the operand of an '&&' or '||'. For example, we // 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 // might lose an lvalue-to-rvalue conversion here. If so, put it back

// before we try to evaluate. // before we try to evaluate.

if (!SubstitutedExpression.isInvalid()) if (!SubstitutedExpression.isInvalid())

SubstitutedExpression = SubstitutedExpression =

S.PerformContextuallyConvertToBool(SubstitutedExpression.get()); S.PerformContextuallyConvertToBool(SubstitutedExpression.get());

if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) { if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {

Show All 31 Lines return calculateConstraintSatisfaction(

if (!S.CheckConstraintExpression(SubstitutedExpression.get())) if (!S.CheckConstraintExpression(SubstitutedExpression.get()))

return ExprError(); return ExprError();

return SubstitutedExpression; return SubstitutedExpression;

}); });

} }

static bool CheckConstraintSatisfaction(Sema &S, const NamedDecl *Template, static bool CheckConstraintSatisfaction(

ArrayRef<const Expr *> ConstraintExprs, Sema &S, const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,

ArrayRef<TemplateArgument> TemplateArgs, llvm::SmallVectorImpl<Expr*> &Converted,

Lint: Pre-merge checks

clang-format: please reformat the code

-    llvm::SmallVectorImpl<Expr*> &Converted,
+    llvm::SmallVectorImpl<Expr *> &Converted,

Lint: Pre-merge checks: clang-format: please reformat the code ``` - llvm::SmallVectorImpl<Expr*> &Converted, +…

SourceRange TemplateIDRange, const MultiLevelTemplateArgumentList &TemplateArgsList,

ConstraintSatisfaction &Satisfaction) { SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction) {

if (ConstraintExprs.empty()) { if (ConstraintExprs.empty()) {

Satisfaction.IsSatisfied = true; Satisfaction.IsSatisfied = true;

return false; return false;

} }

for (auto& Arg : TemplateArgs) if (TemplateArgsList.isAnyArgInstantiationDependent()) {

if (Arg.isInstantiationDependent()) {

// No need to check satisfaction for dependent constraint expressions. // No need to check satisfaction for dependent constraint expressions.

Satisfaction.IsSatisfied = true; Satisfaction.IsSatisfied = true;

return false; return false;

} }

Sema::InstantiatingTemplate Inst(S, TemplateIDRange.getBegin(), ArrayRef<TemplateArgument> TemplateArgs =

TemplateArgsList.getNumSubstitutedLevels() > 0

? TemplateArgsList.getOutermost()

: ArrayRef<TemplateArgument>{};

Sema::InstantiatingTemplate Inst(

S, TemplateIDRange.getBegin(),

Sema::InstantiatingTemplate::ConstraintsCheck{}, Sema::InstantiatingTemplate::ConstraintsCheck{},

const_cast<NamedDecl *>(Template), TemplateArgs, TemplateIDRange); const_cast<NamedDecl *>(Template), TemplateArgs,

Lint: Pre-merge checks

clang-format: please reformat the code

-      const_cast<NamedDecl *>(Template), TemplateArgs,
-      TemplateIDRange);
+      const_cast<NamedDecl *>(Template), TemplateArgs, TemplateIDRange);

Lint: Pre-merge checks: clang-format: please reformat the code ``` - const_cast<NamedDecl *>(Template)…

TemplateIDRange);

if (Inst.isInvalid()) if (Inst.isInvalid())

return true; return true;

MultiLevelTemplateArgumentList MLTAL;

MLTAL.addOuterTemplateArguments(TemplateArgs);

for (const Expr *ConstraintExpr : ConstraintExprs) { for (const Expr *ConstraintExpr : ConstraintExprs) {

if (calculateConstraintSatisfaction(S, Template, TemplateArgs, ExprResult Res = calculateConstraintSatisfaction(

TemplateIDRange.getBegin(), MLTAL, S, Template, TemplateIDRange.getBegin(), TemplateArgsList,

ConstraintExpr, Satisfaction)) ConstraintExpr, Satisfaction);

if (Res.isInvalid())

return true; return true;

if (!Satisfaction.IsSatisfied)

// [temp.constr.op] p2 Converted.push_back(Res.get());

if (!Satisfaction.IsSatisfied) {

// Backfill the 'converted' list with nulls so we can keep the Converted

Converted.append(ConstraintExprs.size() - Converted.size(), nullptr);

// and unconverted lists in sync. [temp.constr.op] p2

ChuanqiXuUnsubmitted

Done

if (!Satisfaction.IsSatisfied) {

// Backfill the 'converted' list with nulls so we can keep the Converted

+ // and unconverted lists in sync.

Converted.append(ConstraintExprs.size() - Converted.size(), nullptr);

- // and unconverted lists in sync. [temp.constr.op] p2

+ // [temp.constr.op] p2

// [...] To determine if a conjunction is satisfied, the satisfaction

ChuanqiXu:

// [...] To determine if a conjunction is satisfied, the satisfaction // [...] To determine if a conjunction is satisfied, the satisfaction

// of the first operand is checked. If that is not satisfied, the // of the first operand is checked. If that is not satisfied, the

// conjunction is not satisfied. [...] // conjunction is not satisfied. [...]

return false; return false;

} }

}

return false; return false;

} }

bool Sema::CheckConstraintSatisfaction( bool Sema::CheckConstraintSatisfaction(

const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs, const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,

ArrayRef<TemplateArgument> TemplateArgs, SourceRange TemplateIDRange, llvm::SmallVectorImpl<Expr *> &ConvertedConstraints,

ConstraintSatisfaction &OutSatisfaction) { const MultiLevelTemplateArgumentList &TemplateArgsList,

SourceRange TemplateIDRange, ConstraintSatisfaction &OutSatisfaction) {

if (ConstraintExprs.empty()) { if (ConstraintExprs.empty()) {

OutSatisfaction.IsSatisfied = true; OutSatisfaction.IsSatisfied = true;

return false; return false;

} }

ArrayRef<TemplateArgument> TemplateArgs =

TemplateArgsList.getNumSubstitutedLevels() > 0

? TemplateArgsList.getOutermost()

: ArrayRef<TemplateArgument>{};

llvm::FoldingSetNodeID ID; llvm::FoldingSetNodeID ID;

void *InsertPos; void *InsertPos;

ConstraintSatisfaction *Satisfaction = nullptr; ConstraintSatisfaction *Satisfaction = nullptr;

bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template; bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;

if (ShouldCache) { if (ShouldCache) {

ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs); ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);

ChuanqiXuUnsubmitted

Done

I think it is a chance to add range style interfaces for MultiLevelTemplateArgumentList. So that we could use range-based loop here.

ChuanqiXu: I think it is a chance to add range style interfaces for MultiLevelTemplateArgumentList. So…

Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos); Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);

if (Satisfaction) { if (Satisfaction) {

OutSatisfaction = *Satisfaction; OutSatisfaction = *Satisfaction;

return false; return false;

} }

Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs); Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);

} else { } else {

Satisfaction = &OutSatisfaction; Satisfaction = &OutSatisfaction;

} }

if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs, if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,

TemplateArgs, TemplateIDRange, ConvertedConstraints, TemplateArgsList,

*Satisfaction)) { TemplateIDRange, *Satisfaction)) {

if (ShouldCache) if (ShouldCache)

delete Satisfaction; delete Satisfaction;

return true; return true;

} }

if (ShouldCache) { if (ShouldCache) {

// We cannot use InsertNode here because CheckConstraintSatisfaction might // We cannot use InsertNode here because CheckConstraintSatisfaction might

// have invalidated it. // have invalidated it.

SatisfactionCache.InsertNode(Satisfaction); SatisfactionCache.InsertNode(Satisfaction);

OutSatisfaction = *Satisfaction; OutSatisfaction = *Satisfaction;

} }

return false; return false;

} }

bool Sema::CheckConstraintSatisfaction(const Expr *ConstraintExpr, bool Sema::CheckConstraintSatisfaction(const Expr *ConstraintExpr,

ConstraintSatisfaction &Satisfaction) { ConstraintSatisfaction &Satisfaction) {

return calculateConstraintSatisfaction( return calculateConstraintSatisfaction(

*this, ConstraintExpr, Satisfaction, *this, ConstraintExpr, Satisfaction,

Lint: Pre-merge checks

clang-format: please reformat the code

-      *this, ConstraintExpr, Satisfaction,
-      [](const Expr *AtomicExpr) -> ExprResult {
-        return ExprResult(const_cast<Expr *>(AtomicExpr));
-      }).isInvalid();
+             *this, ConstraintExpr, Satisfaction,
+             [](const Expr *AtomicExpr) -> ExprResult {
+               return ExprResult(const_cast<Expr *>(AtomicExpr));
+             })
+      .isInvalid();

Lint: Pre-merge checks: clang-format: please reformat the code ``` - *this, ConstraintExpr, Satisfaction…

[](const Expr *AtomicExpr) -> ExprResult { [](const Expr *AtomicExpr) -> ExprResult {

return ExprResult(const_cast<Expr *>(AtomicExpr)); return ExprResult(const_cast<Expr *>(AtomicExpr));

}); }).isInvalid();

}

llvm::Optional<MultiLevelTemplateArgumentList>

Sema::SetupConstraintCheckingTemplateArgumentsAndScope(

FunctionDecl *FD, llvm::Optional<ArrayRef<TemplateArgument>> TemplateArgs,

LocalInstantiationScope &Scope) {

MultiLevelTemplateArgumentList MLTAL;

MLTAL = getTemplateInstantiationArgs(FD, nullptr, /*RelativeToPrimary*/ true,

/*Pattern*/ nullptr,

/*LookBeyondLambda*/ true);

if (FD->isTemplateInstantiation() && FD->getPrimaryTemplate()) {

FunctionTemplateDecl *PrimaryTemplate = FD->getPrimaryTemplate();

InstantiatingTemplate Inst(

*this, FD->getPointOfInstantiation(),

InstantiatingTemplate::ConstraintsCheck{}, PrimaryTemplate,

TemplateArgs ? *TemplateArgs : ArrayRef<TemplateArgument>{},

SourceRange());

if (Inst.isInvalid())

return {};

if (TemplateArgs) {

MultiLevelTemplateArgumentList JustTemplArgs(

*FD->getTemplateSpecializationArgs());

if (addInstantiatedParametersToScope(

FD, PrimaryTemplate->getTemplatedDecl(), Scope, JustTemplArgs))

return {};

}

// If this is a member function, make sure we get the parameters that

// reference the original primary template.

if (const auto *FromMemTempl =

ChuanqiXuUnsubmitted

Done

Might you elaborate more on this. I am not sure about the intention.

ChuanqiXu: Might you elaborate more on this. I am not sure about the intention.

PrimaryTemplate->getInstantiatedFromMemberTemplate()) {

if (addInstantiatedParametersToScope(

Lint: Pre-merge checks

clang-format: please reformat the code

-      if (addInstantiatedParametersToScope(
-              FD, FromMemTempl ->getTemplatedDecl(), Scope, MLTAL))
+      if (addInstantiatedParametersToScope(FD, FromMemTempl->getTemplatedDecl(),
+                                           Scope, MLTAL))

Lint: Pre-merge checks: clang-format: please reformat the code ``` - if (addInstantiatedParametersToScope…

FD, FromMemTempl ->getTemplatedDecl(), Scope, MLTAL))

return {};

}

return MLTAL;

} }

bool Sema::CheckFunctionConstraints(const FunctionDecl *FD, bool Sema::CheckFunctionConstraints(const FunctionDecl *FD,

ConstraintSatisfaction &Satisfaction, ConstraintSatisfaction &Satisfaction,

SourceLocation UsageLoc) { SourceLocation UsageLoc) {

const Expr *RC = FD->getTrailingRequiresClause(); // Don't check constraints if the function is dependent. Also don't check if

if (RC->isInstantiationDependent()) { // this is a function template specialization, as the call to

// CheckinstantiatedFunctionTemplateConstraints after this will check it

// better.

if (FD->isDependentContext() ||

FD->getTemplatedKind() ==

FunctionDecl::TK_FunctionTemplateSpecialization) {

Satisfaction.IsSatisfied = true; Satisfaction.IsSatisfied = true;

return false; return false;

} }

LocalInstantiationScope Scope(*this);

erichkeaneAuthorUnsubmitted

Done

This line was the one that fixed the variable lookup in non-templates.

erichkeane: This line was the one that fixed the variable lookup in non-templates.

erichkeaneAuthorUnsubmitted

Done

By changing FD to FD->GetNonClosureContext (with the appropriate casts), I was at least able to fix the crash. The constraint not being checked for failure however, is still a problem.

erichkeane: By changing `FD` to `FD->GetNonClosureContext` (with the appropriate casts), I was at least…

llvm::Optional<MultiLevelTemplateArgumentList> MLTAL =

SetupConstraintCheckingTemplateArgumentsAndScope(

const_cast<FunctionDecl *>(FD), {}, Scope);

Qualifiers ThisQuals; Qualifiers ThisQuals;

CXXRecordDecl *Record = nullptr; CXXRecordDecl *Record = nullptr;

if (auto *Method = dyn_cast<CXXMethodDecl>(FD)) { if (auto *Method = dyn_cast<CXXMethodDecl>(FD)) {

ThisQuals = Method->getMethodQualifiers(); ThisQuals = Method->getMethodQualifiers();

Record = const_cast<CXXRecordDecl *>(Method->getParent()); Record = const_cast<CXXRecordDecl *>(Method->getParent());

} }

CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr); CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr);

ChuanqiXuUnsubmitted

Done

// be picked out of the map.

- if (TemplateArgs) {

- MultiLevelTemplateArgumentList JustTemplArgs(

- *FD->getTemplateSpecializationArgs());

+ if (auto *SpecArgs = FD->getTemplateSpecializationArgs()) {

+ MultiLevelTemplateArgumentList JustTemplArgs(*SpecArgs);

if (addInstantiatedParametersToScope(

FD, PrimaryTemplate->getTemplatedDecl(), Scope, JustTemplArgs))

return true;

}

// If this is a member function, make sure we get the parameters that

// reference the original primary template.

if (const auto *FromMemTempl =

PrimaryTemplate->getInstantiatedFromMemberTemplate()) {

if (addInstantiatedParametersToScope(FD, FromMemTempl->getTemplatedDecl(),

Scope, MLTAL))

return true;

}

- } else if (FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization) {

+ return false;

+ }

+ if (FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization ||

+ FD->getTemplatedKind() == FunctionDecl::TK_DependentNonTemplate) {

+ FunctionDecl *InstantiatedFrom = FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization ?

+ FD->getInstantiatedFromMemberFunction() :

+ FD->getInstantiatedFromDecl();

InstantiatingTemplate Inst(*this, FD->getPointOfInstantiation(),

Sema::InstantiatingTemplate::ConstraintsCheck{},

- FD->getInstantiatedFromMemberFunction(),

+ InstantiatedFrom,

TemplateArgs ? *TemplateArgs

: ArrayRef<TemplateArgument>{},

SourceRange());

if (Inst.isInvalid())

return true;

// Case where this was not a template, but instantiated as a child-function.

- if (addInstantiatedParametersToScope(

- FD, FD->getInstantiatedFromMemberFunction(), Scope, MLTAL))

- return true;

- } else if (FD->getTemplatedKind() == FunctionDecl::TK_DependentNonTemplate) {

- InstantiatingTemplate Inst(*this, FD->getPointOfInstantiation(),

- Sema::InstantiatingTemplate::ConstraintsCheck{},

- FD->getInstantiatedFromDecl(),

- TemplateArgs ? *TemplateArgs

- : ArrayRef<TemplateArgument>{},

- SourceRange());

- if (Inst.isInvalid())

+ if (addInstantiatedParametersToScope(FD, InstantiatedFrom, Scope, MLTAL))

return true;

- // case where this is a function declared inside of another.

- if (addInstantiatedParametersToScope(FD, FD->getInstantiatedFromDecl(),

- Scope, MLTAL))

- return true;

- }

- return false;

+ } return false;

The suggested change works. I feel it is not identical with the original. Is it correct or do we miss something in the test?

ChuanqiXu: The suggested change works. I feel it is not identical with the original. Is it correct or do…

erichkeaneAuthorUnsubmitted

Done

The very top condition is not exactly the same I think, but in a way that I don't believe matters now that this function isn't recursive.

Otherwise I think this is a vast improvement, so I'll keep it!

erichkeane: The very top condition is not exactly the same I think, but in a way that I don't believe…

// We substitute with empty arguments in order to rebuild the atomic // We substitute with empty arguments in order to rebuild the atomic

// constraint in a constant-evaluated context. // constraint in a constant-evaluated context.

// FIXME: Should this be a dedicated TreeTransform? // FIXME: Should this be a dedicated TreeTransform?

return CheckConstraintSatisfaction( const Expr *RC = FD->getTrailingRequiresClause();

FD, {RC}, /*TemplateArgs=*/{}, llvm::SmallVector<Expr *, 1> Converted;

if (CheckConstraintSatisfaction(

FD, {RC}, Converted, *MLTAL,

Lint: Pre-merge checks

clang-format: please reformat the code

-      FD, {RC}, Converted, *MLTAL,
-      SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
-      Satisfaction))
+          FD, {RC}, Converted, *MLTAL,
+          SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
+          Satisfaction))

Lint: Pre-merge checks: clang-format: please reformat the code ``` - FD, {RC}, Converted, *MLTAL…

SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()), SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),

Satisfaction); Satisfaction))

return true;

// Attn Reviewers: we need to do this for the function constraints for

// comparison of constraints to work, but do we also need to do it for

// CheckInstantiatedFunctionConstraints? That one is more difficult, but we

// seem to always just pick up the constraints from the primary template.

assert(Converted.size() <= 1 && "Got more expressions converted?");

if (!Converted.empty() && Converted[0] != nullptr)

const_cast<FunctionDecl *>(FD)->setTrailingRequiresClause(Converted[0]);

ChuanqiXuUnsubmitted

Done

// Make sure we recursively add the ParmVarDecls from the containing function.

- if (DeclContext *ParentFunc = FD->getParentFunctionOrMethod()) {

+ if (DeclContext *ParentFunc = FD->getParentFunctionOrMethod())

return SetupConstraintScope(cast<FunctionDecl>(ParentFunc), TemplateArgs,

MLTAL, Scope);

- } else if (DeclContext *ParentFunc = FD->getLexicalParentFunctionOrMethod()) {

+ if (DeclContext *ParentFunc = FD->getLexicalParentFunctionOrMethod())

// In the case of functions-declared-in-functions, the DeclContext is the

// TU, so make sure we get the LEXICAL decl context in this case.

return SetupConstraintScope(cast<FunctionDecl>(ParentFunc), TemplateArgs,

MLTAL, Scope);

- }

return false;

Don't use else-after-return: https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return.

And I am wondering if we could hit these 2 checks only if the FD is TK_DependentNonTemplate. If yes, I think we could move these two checks in the above block. In this way, the code could be simplified further.

ChuanqiXu: Don't use else-after-return: https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a…

erichkeaneAuthorUnsubmitted

Done

Changes made!

And I am wondering if we could hit these 2 checks only...

We cannot, this applies generally, including in cases where the current function is a local lambda or function object.

erichkeane: Changes made! >And I am wondering if we could hit these 2 checks only... We cannot, this…

return false;

ChuanqiXuUnsubmitted

Done

Would you mind to elaborate for the issue "function constraints for comparison of constraints to work" more? Maybe it is said in previous messages, but the history is hard to follow...

ChuanqiXu: Would you mind to elaborate for the issue "function constraints for comparison of constraints…

ChuanqiXuUnsubmitted

Done

We need to edit the Attn Reviewers to TODO before landing. The content need to be rewording too. Your English is much better than me. So no concrete suggestion here : )

ChuanqiXu: We need to edit the `Attn Reviewers` to `TODO` before landing. The content need to be rewording…

erichkeaneAuthorUnsubmitted

Done

Yep, this one is difficult :/ Basically, when we instantiate the constraints at 'checking' time, if the function is NOT a template, we call CheckFunctionConstraints. When we go to check the 'subsumption' type things with a fully instantiated version, they fail if they are dependent (as it is expected that way). See the setTrailingRequiresClause here.

HOWEVER, it seems we ALWAYS pick constraints off the primary template, rather than the 'stored' version of the constraint on the current declaration. I tried to do something similar here for those cases, but 1: it had no effect seemingly, and 2: it ended up getting complicated in many cases (as figuring out the relationship between the constraints in the two nodes was difficult/near impossible).

I opted to not do it, and I don't THINK it needs to happen over there, but I wanted to point out that I was skipping it in case reviewers had a better idea.

erichkeane: Yep, this one is difficult :/ Basically, when we instantiate the constraints at 'checking'…

ChuanqiXuUnsubmitted

Done

Let me ask some questions to get in consensus for the problem:

Basically, when we instantiate the constraints at 'checking' time, if the function is NOT a template, we call CheckFunctionConstraints.

I think the function who contains a trailing require clause should be template one. Do you want to say independent ? Or do you refer the following case?

C++
template<typename T> struct A {
  static void f(int) requires xxxx;
};

And the related question would be what's the cases that CheckFunctionConstraints would be called and what's the cases that CheckinstantiatedFunctionTemplateConstraints would be called?

When we go to check the 'subsumption' type things with a fully instantiated version, they fail if they are dependent (as it is expected that way).

If it is fully instantiated, how could it be dependent?

HOWEVER, it seems we ALWAYS pick constraints off the primary template, rather than the 'stored' version of the constraint on the current declaration.

What is we? I mean what's the function would pick up the constraints from the primary template.
Does the stored version of the constraint means the trailing require clause of FD? Would it be the original one or Converted[0].

ChuanqiXu: Let me ask some questions to get in consensus for the problem: > Basically, when we…

erichkeaneAuthorUnsubmitted

Done

Or do you refer the following case?

Yeah, thats exactly the case I am talking about. A case where the function itself isn't a template, but is dependent.

And the related question would be what's the cases that CheckFunctionConstraints would be called and what's the cases that CheckinstantiatedFunctionTemplateConstraints would be called?

The former when either the function is not dependent at all, OR it is dependent-but-fully-instantiated (like in the example you gave). All other cases (where the template is NOT fully instantiated) calls the other function.

If it is fully instantiated, how could it be dependent?

By "Fully Instantiated" I mean "everything except the constraints (and technically, some noexcept)", since we are deferring constraint checking until that point. A bunch of the changes in this commit STOP us from instantiating the constraint except when checking, since that is the point of the patch! SO, the constriant is still 'dependent' (like in your example above).

What is we? I mean what's the function would pick up the constraints from the primary template.

Royal 'we', or clang. The function is getAssociatedConstraints.

Does the stored version of the constraint means the trailing require clause of FD? Would it be the original one or Converted[0].

The 'stored' version (that is, not on the primary template) is the one that we partially instantiated when going through earlier instantiations. In the case that it was a template, we seem to always ignore these instantiated versions. IN the case where it is NOT a template, we use that for checking (which is why Converted[0] needs replacing here).

erichkeane: >>Or do you refer the following case? Yeah, thats exactly the case I am talking about. A case…

ChuanqiXuUnsubmitted

Done

I see roughly. The implementation looks a little bit odd to me.. But I don't find better solutions..

ChuanqiXu: I see roughly. The implementation looks a little bit odd to me.. But I don't find better…

} }

bool Sema::EnsureTemplateArgumentListConstraints( bool Sema::EnsureTemplateArgumentListConstraints(

TemplateDecl *TD, ArrayRef<TemplateArgument> TemplateArgs, TemplateDecl *TD, MultiLevelTemplateArgumentList TemplateArgs,

SourceRange TemplateIDRange) { SourceRange TemplateIDRange) {

ConstraintSatisfaction Satisfaction; ConstraintSatisfaction Satisfaction;

llvm::SmallVector<const Expr *, 3> AssociatedConstraints; llvm::SmallVector<const Expr *, 3> AssociatedConstraints;

erichkeaneAuthorUnsubmitted

Done

Ugh... it looks like all of this might just be 'wrong', and I have no idea how to fix it. @rsmith ANY advise you could give here would be unbelievably appreciated.

Basically, anything involving variables besides ParmVarDecls seems to be broken in some way:

template<typename T>
void foo(T x) {

// This asserts because 'y' is not in the Scope's "FindInstantiationOf"
[y = x]() requires(constraint<decltype(y)>){}


// This asserts because 'Local' is not in the Scope's "FindInstantiationOf"
T Local;
[]() requires(constraint<decltype(Local)>){}

// This gives a "stack nearly exhausted" error, followed by a bunch of "while checking constraint satisfaction for function 'foo' required here'", THEN crashes checking Constraints due to MLTAL being empty at one point (or perhaps just corrupt?). BUT the stack is only 40 deep at the crash.
struct S {
int local;
void foo() requires(constraint<decltype(local)>){}
}

FURTHER, this is also broken by this patch:

template<typename T>
struct S {
T t;
void foo() requires(constraint<decltype(t)>){}
};
void use() {
S<int> s;
s.foo();

With "constraints not satisfied" "because substituted constraint expression is ill-formed: 'S::t' is not a member of 'class S<int>'"

Curiously, it works if 'foo' is itself a template.

I can't help but think that this attempt to re-generate the instantiation is just BROKEN, and I have no idea how to fix it, or what the right approach is. BUT I cannot help but think that I'm doing the 'wrong thing'.

erichkeane: Ugh... it looks like all of this might just be 'wrong', and I have no idea how to fix it.

erichkeaneAuthorUnsubmitted

Done

Note that each of those lambdas ALSO needs to be called there, I forgot the last ().

erichkeane: Note that each of those lambdas ALSO needs to be called there, I forgot the last ().

TD->getAssociatedConstraints(AssociatedConstraints); TD->getAssociatedConstraints(AssociatedConstraints);

if (CheckConstraintSatisfaction(TD, AssociatedConstraints, TemplateArgs, if (CheckConstraintSatisfaction(TD, AssociatedConstraints, TemplateArgs,

TemplateIDRange, Satisfaction)) TemplateIDRange, Satisfaction))

return true; return true;

if (!Satisfaction.IsSatisfied) { if (!Satisfaction.IsSatisfied) {

SmallString<128> TemplateArgString; SmallString<128> TemplateArgString;

TemplateArgString = " "; TemplateArgString = " ";

TemplateArgString += getTemplateArgumentBindingsText( TemplateArgString += getTemplateArgumentBindingsText(

TD->getTemplateParameters(), TemplateArgs.data(), TemplateArgs.size()); TD->getTemplateParameters(), TemplateArgs.getInnermost().data(),

TemplateArgs.getInnermost().size());

Diag(TemplateIDRange.getBegin(), Diag(TemplateIDRange.getBegin(),

diag::err_template_arg_list_constraints_not_satisfied) diag::err_template_arg_list_constraints_not_satisfied)

<< (int)getTemplateNameKindForDiagnostics(TemplateName(TD)) << TD << (int)getTemplateNameKindForDiagnostics(TemplateName(TD)) << TD

<< TemplateArgString << TemplateIDRange; << TemplateArgString << TemplateIDRange;

DiagnoseUnsatisfiedConstraint(Satisfaction); DiagnoseUnsatisfiedConstraint(Satisfaction);

return true; return true;

} }

Show All 15 Lines if (TemplateAC.empty()) {

return false; return false;

} }

// Enter the scope of this instantiation. We don't use // Enter the scope of this instantiation. We don't use

// PushDeclContext because we don't have a scope. // PushDeclContext because we don't have a scope.

Sema::ContextRAII savedContext(*this, Decl); Sema::ContextRAII savedContext(*this, Decl);

LocalInstantiationScope Scope(*this); LocalInstantiationScope Scope(*this);

// If this is not an explicit specialization - we need to get the instantiated Optional<MultiLevelTemplateArgumentList> MLTAL =

// version of the template arguments and add them to scope for the SetupConstraintCheckingTemplateArgumentsAndScope(Decl, TemplateArgs,

// substitution. Scope);

if (Decl->isTemplateInstantiation()) {

InstantiatingTemplate Inst(*this, Decl->getPointOfInstantiation(), if (!MLTAL)

InstantiatingTemplate::ConstraintsCheck{}, Decl->getPrimaryTemplate(),

TemplateArgs, SourceRange());

if (Inst.isInvalid())

return true;

MultiLevelTemplateArgumentList MLTAL(

*Decl->getTemplateSpecializationArgs());

if (addInstantiatedParametersToScope(

Decl, Decl->getPrimaryTemplate()->getTemplatedDecl(), Scope, MLTAL))

return true; return true;

}

Qualifiers ThisQuals; Qualifiers ThisQuals;

CXXRecordDecl *Record = nullptr; CXXRecordDecl *Record = nullptr;

if (auto *Method = dyn_cast<CXXMethodDecl>(Decl)) { if (auto *Method = dyn_cast<CXXMethodDecl>(Decl)) {

ThisQuals = Method->getMethodQualifiers(); ThisQuals = Method->getMethodQualifiers();

Record = Method->getParent(); Record = Method->getParent();

} }

CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr); CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr);

return CheckConstraintSatisfaction(Template, TemplateAC, TemplateArgs, llvm::SmallVector<Expr *, 1> Converted;

return CheckConstraintSatisfaction(Template, TemplateAC, Converted, *MLTAL,

PointOfInstantiation, Satisfaction); PointOfInstantiation, Satisfaction);

} }

static void diagnoseUnsatisfiedRequirement(Sema &S, static void diagnoseUnsatisfiedRequirement(Sema &S,

concepts::ExprRequirement *Req, concepts::ExprRequirement *Req,

bool First) { bool First) {

assert(!Req->isSatisfied() assert(!Req->isSatisfied()

&& "Diagnose() can only be used on an unsatisfied requirement"); && "Diagnose() can only be used on an unsatisfied requirement");

▲ Show 20 Lines • Show All 668 Lines • Show Last 20 Lines