Skip to content

Commit

Permalink
Revert "[Clang][Sema] Fix comparison of constraint expressions"
Browse files Browse the repository at this point in the history
This reverts commit e3b1083.

This was reverted because it breaks a number of libstdc++ examples, AND
required a workaround that causes hiding of legitimate bugs.
  • Loading branch information
Erich Keane committed May 2, 2023
1 parent ad71114 commit 3e850a6
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 419 deletions.
6 changes: 0 additions & 6 deletions clang/include/clang/AST/DeclTemplate.h
Original file line number Diff line number Diff line change
Expand Up @@ -2309,15 +2309,9 @@ class ClassTemplateDecl : public RedeclarableTemplateDecl {
return static_cast<Common *>(RedeclarableTemplateDecl::getCommonPtr());
}

void setCommonPtr(Common *C) {
RedeclarableTemplateDecl::Common = C;
}

public:

friend class ASTDeclReader;
friend class ASTDeclWriter;
friend class TemplateDeclInstantiator;

/// Load any lazily-loaded specializations from the external source.
void LoadLazySpecializations() const;
Expand Down
18 changes: 3 additions & 15 deletions clang/include/clang/Sema/Template.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,21 +232,9 @@ enum class TemplateSubstitutionKind : char {
/// Replaces the current 'innermost' level with the provided argument list.
/// This is useful for type deduction cases where we need to get the entire
/// list from the AST, but then add the deduced innermost list.
void replaceInnermostTemplateArguments(Decl *AssociatedDecl, ArgList Args) {
assert((!TemplateArgumentLists.empty() || NumRetainedOuterLevels) &&
"Replacing in an empty list?");

if (!TemplateArgumentLists.empty()) {
assert((TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ||
TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ==
AssociatedDecl) &&
"Trying to change incorrect declaration?");
TemplateArgumentLists[0].Args = Args;
} else {
--NumRetainedOuterLevels;
TemplateArgumentLists.push_back(
{{AssociatedDecl, /*Final=*/false}, Args});
}
void replaceInnermostTemplateArguments(ArgList Args) {
assert(TemplateArgumentLists.size() > 0 && "Replacing in an empty list?");
TemplateArgumentLists[0].Args = Args;
}

/// Add an outermost level that we are not substituting. We have no
Expand Down
51 changes: 17 additions & 34 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ CalculateTemplateDepthForConstraints(Sema &S, const NamedDecl *ND,
ND, /*Final=*/false, /*Innermost=*/nullptr, /*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true, SkipForSpecialization);
return MLTAL.getNumLevels();
return MLTAL.getNumSubstitutedLevels();
}

namespace {
Expand Down Expand Up @@ -752,44 +752,27 @@ namespace {
};
} // namespace

static const Expr *SubstituteConstraintExpression(Sema &S, const NamedDecl *ND,
const Expr *ConstrExpr) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
ND, /*Final=*/false, /*Innermost=*/nullptr,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
/*SkipForSpecialization*/ false);
if (MLTAL.getNumSubstitutedLevels() == 0)
return ConstrExpr;

Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
std::optional<Sema::CXXThisScopeRAII> ThisScope;
if (auto *RD = dyn_cast<CXXRecordDecl>(ND->getDeclContext()))
ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
ExprResult SubstConstr =
S.SubstConstraintExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
return nullptr;
return SubstConstr.get();
}

bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
const Expr *OldConstr,
const NamedDecl *New,
const Expr *NewConstr) {
if (OldConstr == NewConstr)
return true;
if (Old && New && Old != New) {
if (const Expr *SubstConstr =
SubstituteConstraintExpression(*this, Old, OldConstr))
OldConstr = SubstConstr;
else
return false;
if (const Expr *SubstConstr =
SubstituteConstraintExpression(*this, New, NewConstr))
NewConstr = SubstConstr;
else
return false;
unsigned Depth1 = CalculateTemplateDepthForConstraints(
*this, Old);
unsigned Depth2 = CalculateTemplateDepthForConstraints(
*this, New);

// Adjust the 'shallowest' verison of this to increase the depth to match
// the 'other'.
if (Depth2 > Depth1) {
OldConstr = AdjustConstraintDepth(*this, Depth2 - Depth1)
.TransformExpr(const_cast<Expr *>(OldConstr))
.get();
} else if (Depth1 > Depth2) {
NewConstr = AdjustConstraintDepth(*this, Depth1 - Depth2)
.TransformExpr(const_cast<Expr *>(NewConstr))
.get();
}
}

llvm::FoldingSetNodeID ID1, ID2;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,7 @@ bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
// We check the return type and template parameter lists for function
// templates first; the remaining checks follow.
bool SameTemplateParameterList = TemplateParameterListsAreEqual(
NewTemplate, NewTemplate->getTemplateParameters(), OldTemplate,
NewTemplate->getTemplateParameters(),
OldTemplate->getTemplateParameters(), false, TPL_TemplateMatch);
bool SameReturnType = Context.hasSameType(Old->getDeclaredReturnType(),
New->getDeclaredReturnType());
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaTemplateDeduction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2881,7 +2881,7 @@ CheckDeducedArgumentConstraints(Sema &S, TemplateDeclT *Template,
// not class-scope explicit specialization, so replace with Deduced Args
// instead of adding to inner-most.
if (NeedsReplacement)
MLTAL.replaceInnermostTemplateArguments(Template, CanonicalDeducedArgs);
MLTAL.replaceInnermostTemplateArguments(CanonicalDeducedArgs);

if (S.CheckConstraintSatisfaction(Template, AssociatedConstraints, MLTAL,
Info.getLocation(),
Expand Down
38 changes: 7 additions & 31 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,6 @@ HandleDefaultTempArgIntoTempTempParam(const TemplateTemplateParmDecl *TTP,
return Response::Done();
}

Response HandlePartialClassTemplateSpec(
const ClassTemplatePartialSpecializationDecl *PartialClassTemplSpec,
MultiLevelTemplateArgumentList &Result, bool SkipForSpecialization) {
if (!SkipForSpecialization)
Result.addOuterRetainedLevels(PartialClassTemplSpec->getTemplateDepth());
return Response::Done();
}

// Add template arguments from a class template instantiation.
Response
HandleClassTemplateSpec(const ClassTemplateSpecializationDecl *ClassTemplSpec,
Expand Down Expand Up @@ -216,21 +208,6 @@ Response HandleFunction(const FunctionDecl *Function,
return Response::UseNextDecl(Function);
}

Response HandleFunctionTemplateDecl(const FunctionTemplateDecl *FTD,
MultiLevelTemplateArgumentList &Result) {
if (!isa<ClassTemplateSpecializationDecl>(FTD->getDeclContext())) {
NestedNameSpecifier *NNS = FTD->getTemplatedDecl()->getQualifier();
const Type *Ty;
const TemplateSpecializationType *TSTy;
if (NNS && (Ty = NNS->getAsType()) &&
(TSTy = Ty->getAs<TemplateSpecializationType>()))
Result.addOuterTemplateArguments(const_cast<FunctionTemplateDecl *>(FTD),
TSTy->template_arguments(),
/*Final=*/false);
}
return Response::ChangeDecl(FTD->getLexicalDeclContext());
}

Response HandleRecordDecl(const CXXRecordDecl *Rec,
MultiLevelTemplateArgumentList &Result,
ASTContext &Context,
Expand All @@ -241,10 +218,15 @@ Response HandleRecordDecl(const CXXRecordDecl *Rec,
"Outer template not instantiated?");
if (ClassTemplate->isMemberSpecialization())
return Response::Done();
if (ForConstraintInstantiation)
if (ForConstraintInstantiation) {
QualType RecordType = Context.getTypeDeclType(Rec);
QualType Injected = cast<InjectedClassNameType>(RecordType)
->getInjectedSpecializationType();
const auto *InjectedType = cast<TemplateSpecializationType>(Injected);
Result.addOuterTemplateArguments(const_cast<CXXRecordDecl *>(Rec),
ClassTemplate->getInjectedTemplateArgs(),
InjectedType->template_arguments(),
/*Final=*/false);
}
}

bool IsFriend = Rec->getFriendObjectKind() ||
Expand Down Expand Up @@ -324,10 +306,6 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
if (const auto *VarTemplSpec =
dyn_cast<VarTemplateSpecializationDecl>(CurDecl)) {
R = HandleVarTemplateSpec(VarTemplSpec, Result, SkipForSpecialization);
} else if (const auto *PartialClassTemplSpec =
dyn_cast<ClassTemplatePartialSpecializationDecl>(CurDecl)) {
R = HandlePartialClassTemplateSpec(PartialClassTemplSpec, Result,
SkipForSpecialization);
} else if (const auto *ClassTemplSpec =
dyn_cast<ClassTemplateSpecializationDecl>(CurDecl)) {
R = HandleClassTemplateSpec(ClassTemplSpec, Result,
Expand All @@ -340,8 +318,6 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
} else if (const auto *CSD =
dyn_cast<ImplicitConceptSpecializationDecl>(CurDecl)) {
R = HandleImplicitConceptSpecializationDecl(CSD, Result);
} else if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(CurDecl)) {
R = HandleFunctionTemplateDecl(FTD, Result);
} else if (!isa<DeclContext>(CurDecl)) {
R = Response::DontClearRelativeToPrimaryNextDecl(CurDecl);
if (CurDecl->getDeclContext()->isTranslationUnit()) {
Expand Down
61 changes: 30 additions & 31 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1653,12 +1653,33 @@ Decl *TemplateDeclInstantiator::VisitClassTemplateDecl(ClassTemplateDecl *D) {
<< QualifierLoc.getSourceRange();
return nullptr;
}

if (PrevClassTemplate) {
const ClassTemplateDecl *MostRecentPrevCT =
PrevClassTemplate->getMostRecentDecl();
TemplateParameterList *PrevParams =
MostRecentPrevCT->getTemplateParameters();

// Make sure the parameter lists match.
if (!SemaRef.TemplateParameterListsAreEqual(
D->getTemplatedDecl(), InstParams,
MostRecentPrevCT->getTemplatedDecl(), PrevParams, true,
Sema::TPL_TemplateMatch))
return nullptr;

// Do some additional validation, then merge default arguments
// from the existing declarations.
if (SemaRef.CheckTemplateParameterList(InstParams, PrevParams,
Sema::TPC_ClassTemplate))
return nullptr;
}
}

CXXRecordDecl *RecordInst = CXXRecordDecl::Create(
SemaRef.Context, Pattern->getTagKind(), DC, Pattern->getBeginLoc(),
Pattern->getLocation(), Pattern->getIdentifier(), PrevDecl,
/*DelayTypeCreation=*/true);

if (QualifierLoc)
RecordInst->setQualifierInfo(QualifierLoc);

Expand All @@ -1668,38 +1689,16 @@ Decl *TemplateDeclInstantiator::VisitClassTemplateDecl(ClassTemplateDecl *D) {
ClassTemplateDecl *Inst
= ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(),
D->getIdentifier(), InstParams, RecordInst);
assert(!(isFriend && Owner->isDependentContext()));
Inst->setPreviousDecl(PrevClassTemplate);

RecordInst->setDescribedClassTemplate(Inst);

if (isFriend) {
assert(!Owner->isDependentContext());
Inst->setLexicalDeclContext(Owner);
RecordInst->setLexicalDeclContext(Owner);

if (PrevClassTemplate) {
Inst->setCommonPtr(PrevClassTemplate->getCommonPtr());
RecordInst->setTypeForDecl(
PrevClassTemplate->getTemplatedDecl()->getTypeForDecl());
const ClassTemplateDecl *MostRecentPrevCT =
PrevClassTemplate->getMostRecentDecl();
TemplateParameterList *PrevParams =
MostRecentPrevCT->getTemplateParameters();

// Make sure the parameter lists match.
if (!SemaRef.TemplateParameterListsAreEqual(
RecordInst, InstParams, MostRecentPrevCT->getTemplatedDecl(),
PrevParams, true, Sema::TPL_TemplateMatch))
return nullptr;

// Do some additional validation, then merge default arguments
// from the existing declarations.
if (SemaRef.CheckTemplateParameterList(InstParams, PrevParams,
Sema::TPC_ClassTemplate))
return nullptr;

if (PrevClassTemplate)
Inst->setAccess(PrevClassTemplate->getAccess());
} else {
else
Inst->setAccess(D->getAccess());
}

Inst->setObjectOfFriendDecl();
// TODO: do we want to track the instantiation progeny of this
Expand All @@ -1710,15 +1709,15 @@ Decl *TemplateDeclInstantiator::VisitClassTemplateDecl(ClassTemplateDecl *D) {
Inst->setInstantiatedFromMemberTemplate(D);
}

Inst->setPreviousDecl(PrevClassTemplate);

// Trigger creation of the type for the instantiation.
SemaRef.Context.getInjectedClassNameType(
RecordInst, Inst->getInjectedClassNameSpecialization());
SemaRef.Context.getInjectedClassNameType(RecordInst,
Inst->getInjectedClassNameSpecialization());

// Finish handling of friends.
if (isFriend) {
DC->makeDeclVisibleInContext(Inst);
Inst->setLexicalDeclContext(Owner);
RecordInst->setLexicalDeclContext(Owner);
return Inst;
}

Expand Down
24 changes: 0 additions & 24 deletions clang/test/SemaTemplate/concepts-friends.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,27 +441,3 @@ namespace NTTP {
templ_func<1>(u2);
}
}


namespace FrineOfFriend {

template <typename>
concept Concept = true;

template <Concept> class FriendOfBar;

template <Concept> class Bar {
template <Concept> friend class FriendOfBar;
};

Bar<void> BarInstance;

namespace internal {
void FriendOfFoo(FriendOfBar<void>);
}

template <Concept> class Foo {
friend void internal::FriendOfFoo(FriendOfBar<void>);
};

} // namespace FrineOfFriend

0 comments on commit 3e850a6

Please sign in to comment.