Skip to content

[Clang][Sema] Refactor collection of multi-level template argument lists #106585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 35 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
072be6e
[WIP] set isMemberSpecialization early
sdkrystian Aug 1, 2024
6cb3f61
[WIP] set isMemberSpecialization before comparing template parameter …
sdkrystian Aug 8, 2024
420b553
[FOLD]
sdkrystian Aug 8, 2024
bd8156b
[FOLD] pass old and new declarations to TemplateParameterListsAreEqua…
sdkrystian Aug 12, 2024
a37d796
[FOLD] implement TemplateInstantiationArgumentCollecter
sdkrystian Aug 12, 2024
07d100a
[FOLD] update test
sdkrystian Aug 12, 2024
37f1461
[FOLD] further simplify getTemplateInstantiationArgs
sdkrystian Aug 12, 2024
f1e1272
[FOLD] add tests
sdkrystian Aug 12, 2024
623e4ac
[FOLD]
sdkrystian Aug 12, 2024
3c78bbf
[FOLD] remove SkipForSpecialization and Pattern parameters from getTe…
sdkrystian Aug 13, 2024
e0bd523
[FOLD] using the right next decl after adding innermost template argu…
sdkrystian Aug 14, 2024
658c6c9
[FOLD] instantiate lambda default args correctly
sdkrystian Aug 20, 2024
4832287
[FOLD] handle template template params correctly and switch to descri…
sdkrystian Aug 23, 2024
3c8ce5c
[FOLD] remove code for handling described templates
sdkrystian Aug 23, 2024
fd186a9
[FOLD] handle partial specializations that are member specializations
sdkrystian Aug 23, 2024
66b0d89
[FOLD] cleanups
sdkrystian Aug 23, 2024
b467dfe
[FOLD] cleanup dead code
sdkrystian Aug 27, 2024
c0af059
[FOLD] revert whitespace only changes
sdkrystian Aug 27, 2024
a82ccbb
[FOLD] remove some redundant checks
sdkrystian Aug 27, 2024
e235dce
[FOLD] fix common pointer being overwritten when deserializing
sdkrystian Aug 28, 2024
af67f88
[FOLD] more cleanup
sdkrystian Aug 29, 2024
2b98556
[FOLD] more cleanups
sdkrystian Aug 29, 2024
86e9e55
[FOLD] format
sdkrystian Aug 29, 2024
fec5107
[FOLD] address some review feedback
sdkrystian Aug 30, 2024
6b39e14
[FOLD] use pointer for InstantiatedFromMember
sdkrystian Aug 30, 2024
9e18656
[FOLD] add comment
sdkrystian Aug 30, 2024
6073ed4
[FOLD] diagnose redefinitions after checking template parameter equiv…
sdkrystian Sep 3, 2024
f94fb03
[FOLD] more tests
sdkrystian Sep 3, 2024
0a64b37
[FOLD] add getCommonPtrInternal
sdkrystian Sep 20, 2024
a9b14fa
[FOLD] add asserts in setInstantiationOf
sdkrystian Sep 20, 2024
ad6e50d
[FOLD] split up template argument collection for class/variable templ…
sdkrystian Sep 20, 2024
3add30a
[FOLD] add RedeclarableTemplateDecl::setCommonPtr
sdkrystian Sep 20, 2024
29745c7
[FOLD] update docs
sdkrystian Sep 20, 2024
22e4d64
[FOLD] remove DontClearRelativeToPrimaryNextDecl
sdkrystian Sep 20, 2024
bd6a615
[FOLD] add release note
sdkrystian Sep 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ Bug Fixes to C++ Support
- Fixed an assertion failure in debug mode, and potential crashes in release mode, when
diagnosing a failed cast caused indirectly by a failed implicit conversion to the type of the constructor parameter.
- Fixed an assertion failure by adjusting integral to boolean vector conversions (#GH108326)
- Clang now uses the correct set of template argument lists when comparing the constraints of
out-of-line definitions and member templates explicitly specialized for a given implicit instantiation of
a class template. (#GH102320)

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
66 changes: 27 additions & 39 deletions clang/include/clang/AST/DeclTemplate.h
Original file line number Diff line number Diff line change
Expand Up @@ -781,15 +781,11 @@ class RedeclarableTemplateDecl : public TemplateDecl,
EntryType *Entry, void *InsertPos);

struct CommonBase {
CommonBase() : InstantiatedFromMember(nullptr, false) {}
CommonBase() {}

/// The template from which this was most
/// directly instantiated (or null).
///
/// The boolean value indicates whether this template
/// was explicitly specialized.
llvm::PointerIntPair<RedeclarableTemplateDecl*, 1, bool>
InstantiatedFromMember;
RedeclarableTemplateDecl *InstantiatedFromMember = nullptr;

/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
Expand All @@ -809,14 +805,19 @@ class RedeclarableTemplateDecl : public TemplateDecl,
};

/// Pointer to the common data shared by all declarations of this
/// template.
mutable CommonBase *Common = nullptr;
/// template, and a flag indicating if the template is a member
/// specialization.
mutable llvm::PointerIntPair<CommonBase *, 1, bool> Common;

CommonBase *getCommonPtrInternal() const { return Common.getPointer(); }

/// Retrieves the "common" pointer shared by all (re-)declarations of
/// the same template. Calling this routine may implicitly allocate memory
/// for the common pointer.
CommonBase *getCommonPtr() const;

void setCommonPtr(CommonBase *C) const { Common.setPointer(C); }

virtual CommonBase *newCommon(ASTContext &C) const = 0;

// Construct a template decl with name, parameters, and templated element.
Expand Down Expand Up @@ -857,15 +858,12 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// template<> template<typename T>
/// struct X<int>::Inner { /* ... */ };
/// \endcode
bool isMemberSpecialization() const {
return getCommonPtr()->InstantiatedFromMember.getInt();
}
bool isMemberSpecialization() const { return Common.getInt(); }

/// Note that this member template is a specialization.
void setMemberSpecialization() {
assert(getCommonPtr()->InstantiatedFromMember.getPointer() &&
"Only member templates can be member template specializations");
getCommonPtr()->InstantiatedFromMember.setInt(true);
assert(!isMemberSpecialization() && "already a member specialization");
Common.setInt(true);
}

/// Retrieve the member template from which this template was
Expand Down Expand Up @@ -905,12 +903,12 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// void X<T>::f(T, U);
/// \endcode
RedeclarableTemplateDecl *getInstantiatedFromMemberTemplate() const {
return getCommonPtr()->InstantiatedFromMember.getPointer();
return getCommonPtr()->InstantiatedFromMember;
}

void setInstantiatedFromMemberTemplate(RedeclarableTemplateDecl *TD) {
assert(!getCommonPtr()->InstantiatedFromMember.getPointer());
getCommonPtr()->InstantiatedFromMember.setPointer(TD);
assert(!getCommonPtr()->InstantiatedFromMember);
getCommonPtr()->InstantiatedFromMember = TD;
}

/// Retrieve the "injected" template arguments that correspond to the
Expand Down Expand Up @@ -1989,6 +1987,8 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
/// template arguments have been deduced.
void setInstantiationOf(ClassTemplatePartialSpecializationDecl *PartialSpec,
const TemplateArgumentList *TemplateArgs) {
assert(!isa<ClassTemplatePartialSpecializationDecl>(this) &&
"A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization*>() &&
"Already set to a class template partial specialization!");
auto *PS = new (getASTContext()) SpecializedPartialSpecialization();
Expand All @@ -2000,6 +2000,8 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
/// Note that this class template specialization is an instantiation
/// of the given class template.
void setInstantiationOf(ClassTemplateDecl *TemplDecl) {
assert(!isa<ClassTemplatePartialSpecializationDecl>(this) &&
"A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization*>() &&
"Previously set to a class template partial specialization!");
SpecializedTemplate = TemplDecl;
Expand Down Expand Up @@ -2187,18 +2189,11 @@ class ClassTemplatePartialSpecializationDecl
/// struct X<int>::Inner<T*> { /* ... */ };
/// \endcode
bool isMemberSpecialization() const {
const auto *First =
cast<ClassTemplatePartialSpecializationDecl>(getFirstDecl());
return First->InstantiatedFromMember.getInt();
return InstantiatedFromMember.getInt();
}

/// Note that this member template is a specialization.
void setMemberSpecialization() {
auto *First = cast<ClassTemplatePartialSpecializationDecl>(getFirstDecl());
assert(First->InstantiatedFromMember.getPointer() &&
"Only member templates can be member template specializations");
return First->InstantiatedFromMember.setInt(true);
}
void setMemberSpecialization() { return InstantiatedFromMember.setInt(true); }

/// Retrieves the injected specialization type for this partial
/// specialization. This is not the same as the type-decl-type for
Expand Down Expand Up @@ -2268,10 +2263,6 @@ class ClassTemplateDecl : public RedeclarableTemplateDecl {
return static_cast<Common *>(RedeclarableTemplateDecl::getCommonPtr());
}

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

public:

friend class ASTDeclReader;
Expand Down Expand Up @@ -2754,6 +2745,8 @@ class VarTemplateSpecializationDecl : public VarDecl,
/// template arguments have been deduced.
void setInstantiationOf(VarTemplatePartialSpecializationDecl *PartialSpec,
const TemplateArgumentList *TemplateArgs) {
assert(!isa<VarTemplatePartialSpecializationDecl>(this) &&
"A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization *>() &&
"Already set to a variable template partial specialization!");
auto *PS = new (getASTContext()) SpecializedPartialSpecialization();
Expand All @@ -2765,6 +2758,8 @@ class VarTemplateSpecializationDecl : public VarDecl,
/// Note that this variable template specialization is an instantiation
/// of the given variable template.
void setInstantiationOf(VarTemplateDecl *TemplDecl) {
assert(!isa<VarTemplatePartialSpecializationDecl>(this) &&
"A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization *>() &&
"Previously set to a variable template partial specialization!");
SpecializedTemplate = TemplDecl;
Expand Down Expand Up @@ -2949,18 +2944,11 @@ class VarTemplatePartialSpecializationDecl
/// U* X<int>::Inner<T*> = (T*)(0) + 1;
/// \endcode
bool isMemberSpecialization() const {
const auto *First =
cast<VarTemplatePartialSpecializationDecl>(getFirstDecl());
return First->InstantiatedFromMember.getInt();
return InstantiatedFromMember.getInt();
}

/// Note that this member template is a specialization.
void setMemberSpecialization() {
auto *First = cast<VarTemplatePartialSpecializationDecl>(getFirstDecl());
assert(First->InstantiatedFromMember.getPointer() &&
"Only member templates can be member template specializations");
return First->InstantiatedFromMember.setInt(true);
}
void setMemberSpecialization() { return InstantiatedFromMember.setInt(true); }

SourceRange getSourceRange() const override LLVM_READONLY;

Expand Down
25 changes: 6 additions & 19 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -11389,9 +11389,9 @@ class Sema final : public SemaBase {
CXXScopeSpec &SS, IdentifierInfo *Name, SourceLocation NameLoc,
const ParsedAttributesView &Attr, TemplateParameterList *TemplateParams,
AccessSpecifier AS, SourceLocation ModulePrivateLoc,
SourceLocation FriendLoc, unsigned NumOuterTemplateParamLists,
TemplateParameterList **OuterTemplateParamLists,
SkipBodyInfo *SkipBody = nullptr);
SourceLocation FriendLoc,
ArrayRef<TemplateParameterList *> OuterTemplateParamLists,
bool IsMemberSpecialization, SkipBodyInfo *SkipBody = nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a preference for using an enum instead of bool params (so that callers do: SpecializationKind::None, or SpecializationKind::Member or something like that?). Even if there are two options, it improves readability of callers a ton.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still have a preference for something like this, but otherwise


/// Translates template arguments as provided by the parser
/// into template arguments used by semantic analysis.
Expand Down Expand Up @@ -11430,7 +11430,8 @@ class Sema final : public SemaBase {
DeclResult ActOnVarTemplateSpecialization(
Scope *S, Declarator &D, TypeSourceInfo *DI, LookupResult &Previous,
SourceLocation TemplateKWLoc, TemplateParameterList *TemplateParams,
StorageClass SC, bool IsPartialSpecialization);
StorageClass SC, bool IsPartialSpecialization,
bool IsMemberSpecialization);

/// Get the specialization of the given variable template corresponding to
/// the specified argument list, or a null-but-valid result if the arguments
Expand Down Expand Up @@ -13071,28 +13072,14 @@ class Sema final : public SemaBase {
/// dealing with a specialization. This is only relevant for function
/// template specializations.
///
/// \param Pattern If non-NULL, indicates the pattern from which we will be
/// instantiating the definition of the given declaration, \p ND. This is
/// used to determine the proper set of template instantiation arguments for
/// friend function template specializations.
///
/// \param ForConstraintInstantiation when collecting arguments,
/// ForConstraintInstantiation indicates we should continue looking when
/// encountering a lambda generic call operator, and continue looking for
/// arguments on an enclosing class template.
///
/// \param SkipForSpecialization when specified, any template specializations
/// in a traversal would be ignored.
/// \param ForDefaultArgumentSubstitution indicates we should continue looking
/// when encountering a specialized member function template, rather than
/// returning immediately.
MultiLevelTemplateArgumentList getTemplateInstantiationArgs(
const NamedDecl *D, const DeclContext *DC = nullptr, bool Final = false,
std::optional<ArrayRef<TemplateArgument>> Innermost = std::nullopt,
bool RelativeToPrimary = false, const FunctionDecl *Pattern = nullptr,
bool ForConstraintInstantiation = false,
bool SkipForSpecialization = false,
bool ForDefaultArgumentSubstitution = false);
bool RelativeToPrimary = false, bool ForConstraintInstantiation = false);

/// RAII object to handle the state changes required to synthesize
/// a function body.
Expand Down
28 changes: 14 additions & 14 deletions clang/lib/AST/DeclTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,35 +309,35 @@ bool TemplateDecl::isTypeAlias() const {
void RedeclarableTemplateDecl::anchor() {}

RedeclarableTemplateDecl::CommonBase *RedeclarableTemplateDecl::getCommonPtr() const {
if (Common)
return Common;
if (CommonBase *C = getCommonPtrInternal())
return C;

// Walk the previous-declaration chain until we either find a declaration
// with a common pointer or we run out of previous declarations.
SmallVector<const RedeclarableTemplateDecl *, 2> PrevDecls;
for (const RedeclarableTemplateDecl *Prev = getPreviousDecl(); Prev;
Prev = Prev->getPreviousDecl()) {
if (Prev->Common) {
Common = Prev->Common;
if (CommonBase *C = Prev->getCommonPtrInternal()) {
setCommonPtr(C);
break;
}

PrevDecls.push_back(Prev);
}

// If we never found a common pointer, allocate one now.
if (!Common) {
if (!getCommonPtrInternal()) {
// FIXME: If any of the declarations is from an AST file, we probably
// need an update record to add the common data.

Common = newCommon(getASTContext());
setCommonPtr(newCommon(getASTContext()));
}

// Update any previous declarations we saw with the common pointer.
for (const RedeclarableTemplateDecl *Prev : PrevDecls)
Prev->Common = Common;
Prev->setCommonPtr(getCommonPtrInternal());

return Common;
return getCommonPtrInternal();
}

void RedeclarableTemplateDecl::loadLazySpecializationsImpl() const {
Expand Down Expand Up @@ -467,15 +467,15 @@ void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {

// If we haven't created a common pointer yet, then it can just be created
// with the usual method.
if (!Base::Common)
if (!getCommonPtrInternal())
return;

Common *ThisCommon = static_cast<Common *>(Base::Common);
Common *ThisCommon = static_cast<Common *>(getCommonPtrInternal());
Common *PrevCommon = nullptr;
SmallVector<FunctionTemplateDecl *, 8> PreviousDecls;
for (; Prev; Prev = Prev->getPreviousDecl()) {
if (Prev->Base::Common) {
PrevCommon = static_cast<Common *>(Prev->Base::Common);
if (CommonBase *C = Prev->getCommonPtrInternal()) {
PrevCommon = static_cast<Common *>(C);
break;
}
PreviousDecls.push_back(Prev);
Expand All @@ -485,15 +485,15 @@ void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {
// use this common pointer.
if (!PrevCommon) {
for (auto *D : PreviousDecls)
D->Base::Common = ThisCommon;
D->setCommonPtr(ThisCommon);
return;
}

// Ensure we don't leak any important state.
assert(ThisCommon->Specializations.size() == 0 &&
"Can't merge incompatible declarations!");

Base::Common = PrevCommon;
setCommonPtr(PrevCommon);
}

//===----------------------------------------------------------------------===//
Expand Down
29 changes: 12 additions & 17 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,8 +585,8 @@ static bool CheckConstraintSatisfaction(

ArrayRef<TemplateArgument> TemplateArgs =
TemplateArgsLists.getNumSubstitutedLevels() > 0
? TemplateArgsLists.getOutermost()
: ArrayRef<TemplateArgument> {};
? TemplateArgsLists.getInnermost()
: ArrayRef<TemplateArgument>{};
Sema::InstantiatingTemplate Inst(S, TemplateIDRange.getBegin(),
Sema::InstantiatingTemplate::ConstraintsCheck{},
const_cast<NamedDecl *>(Template), TemplateArgs, TemplateIDRange);
Expand Down Expand Up @@ -833,7 +833,6 @@ Sema::SetupConstraintCheckingTemplateArgumentsAndScope(
getTemplateInstantiationArgs(FD, FD->getLexicalDeclContext(),
/*Final=*/false, /*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true);
if (SetupConstraintScope(FD, TemplateArgs, MLTAL, Scope))
return std::nullopt;
Expand Down Expand Up @@ -909,15 +908,13 @@ bool Sema::CheckFunctionConstraints(const FunctionDecl *FD,
// Figure out the to-translation-unit depth for this function declaration for
// the purpose of seeing if they differ by constraints. This isn't the same as
// getTemplateDepth, because it includes already instantiated parents.
static unsigned
CalculateTemplateDepthForConstraints(Sema &S, const NamedDecl *ND,
bool SkipForSpecialization = false) {
static unsigned CalculateTemplateDepthForConstraints(Sema &S,
const NamedDecl *ND) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
ND, ND->getLexicalDeclContext(), /*Final=*/false,
/*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true, SkipForSpecialization);
/*ForConstraintInstantiation=*/true);
return MLTAL.getNumLevels();
}

Expand Down Expand Up @@ -956,8 +953,7 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
/*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
/*SkipForSpecialization*/ false);
/*ForConstraintInstantiation=*/true);

if (MLTAL.getNumSubstitutedLevels() == 0)
return ConstrExpr;
Expand Down Expand Up @@ -1063,16 +1059,16 @@ bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
bool Sema::FriendConstraintsDependOnEnclosingTemplate(const FunctionDecl *FD) {
assert(FD->getFriendObjectKind() && "Must be a friend!");

FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate();
// The logic for non-templates is handled in ASTContext::isSameEntity, so we
// don't have to bother checking 'DependsOnEnclosingTemplate' for a
// non-function-template.
assert(FD->getDescribedFunctionTemplate() &&
"Non-function templates don't need to be checked");
assert(FTD && "Non-function templates don't need to be checked");

SmallVector<const Expr *, 3> ACs;
FD->getDescribedFunctionTemplate()->getAssociatedConstraints(ACs);
FTD->getAssociatedConstraints(ACs);

unsigned OldTemplateDepth = CalculateTemplateDepthForConstraints(*this, FD);
unsigned OldTemplateDepth = FTD->getTemplateParameters()->getDepth();
for (const Expr *Constraint : ACs)
if (ConstraintExpressionDependsOnEnclosingTemplate(FD, OldTemplateDepth,
Constraint))
Expand Down Expand Up @@ -1519,7 +1515,6 @@ static bool substituteParameterMappings(Sema &S, NormalizedConstraint &N,
CSE->getNamedConcept(), CSE->getNamedConcept()->getLexicalDeclContext(),
/*Final=*/false, CSE->getTemplateArguments(),
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true);

return substituteParameterMappings(S, N, CSE->getNamedConcept(), MLTAL,
Expand Down Expand Up @@ -1800,8 +1795,8 @@ bool Sema::IsAtLeastAsConstrained(NamedDecl *D1,
return false;
}

unsigned Depth1 = CalculateTemplateDepthForConstraints(*this, D1, true);
unsigned Depth2 = CalculateTemplateDepthForConstraints(*this, D2, true);
unsigned Depth1 = CalculateTemplateDepthForConstraints(*this, D1);
unsigned Depth2 = CalculateTemplateDepthForConstraints(*this, D2);

for (size_t I = 0; I != AC1.size() && I != AC2.size(); ++I) {
if (Depth2 > Depth1) {
Expand Down
Loading