Skip to content

[Clang][NFC] Remove TemplateArgumentList::OnStack #79760

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.

Sign up for GitHub

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 1 commit into from
Feb 1, 2024
+81 −112
Merged

[Clang][NFC] Remove TemplateArgumentList::OnStack #79760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ AST Dumping Potentially Breaking Changes

Clang Frontend Potentially Breaking Changes
-------------------------------------------
- Removed support for constructing on-stack ``TemplateArgumentList``s; interfaces should instead
use ``ArrayRef<TemplateArgument>`` to pass template arguments. Transitioning internal uses to
``ArrayRef<TemplateArgument>`` reduces AST memory usage by 0.4% when compiling clang, and is
expected to show similar improvements on other workloads.

Target OS macros extension
^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
26 changes: 3 additions & 23 deletions clang/include/clang/AST/DeclTemplate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,6 @@ class FixedSizeTemplateParameterListStorage
/// A template argument list.
class TemplateArgumentList final
: private llvm::TrailingObjects<TemplateArgumentList, TemplateArgument> {
/// The template argument list.
const TemplateArgument *Arguments;

/// The number of template arguments in this template
/// argument list.
unsigned NumArguments;
Expand All @@ -258,30 +255,11 @@ class TemplateArgumentList final
TemplateArgumentList(const TemplateArgumentList &) = delete;
TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;

/// Type used to indicate that the template argument list itself is a
/// stack object. It does not own its template arguments.
enum OnStackType { OnStack };

/// Create a new template argument list that copies the given set of
/// template arguments.
static TemplateArgumentList *CreateCopy(ASTContext &Context,
ArrayRef<TemplateArgument> Args);

/// Construct a new, temporary template argument list on the stack.
///
/// The template argument list does not own the template arguments
/// provided.
explicit TemplateArgumentList(OnStackType, ArrayRef<TemplateArgument> Args)
: Arguments(Args.data()), NumArguments(Args.size()) {}

/// Produces a shallow copy of the given template argument list.
///
/// This operation assumes that the input argument list outlives it.
/// This takes the list as a pointer to avoid looking like a copy
/// constructor, since this really isn't safe to use that way.
explicit TemplateArgumentList(const TemplateArgumentList *Other)
: Arguments(Other->data()), NumArguments(Other->size()) {}

/// Retrieve the template argument at a given index.
const TemplateArgument &get(unsigned Idx) const {
assert(Idx < NumArguments && "Invalid template argument index");
Expand All @@ -301,7 +279,9 @@ class TemplateArgumentList final
unsigned size() const { return NumArguments; }

/// Retrieve a pointer to the template argument list.
const TemplateArgument *data() const { return Arguments; }
const TemplateArgument *data() const {
return getTrailingObjects<TemplateArgument>();
}
Comment on lines 279 to +284
Copy link
Contributor

@cor3ntin cor3ntin Jan 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if we could get rid of data and size entirely in the public interface

Copy link
Member Author

@sdkrystian sdkrystian Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We definitely could, but perhaps that is outside the scope of this PR? That being said, It would be good to unify the interfaces of TemplateArgumentList, TemplateArgumentListInfo, and ASTTemplateArgumentListInfo by perhaps giving them all the following members:

  • begin() and end()
  • size() and empty()
  • operator[]
  • operator ArrayRef<TemplateArgument>()/operator ArrayRef<TemplateArgumentInfo>()

};

void *allocateDefaultArgStorageChain(const ASTContext &C);
Expand Down
8 changes: 4 additions & 4 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9329,12 +9329,12 @@ class Sema final {

TemplateDeductionResult
DeduceTemplateArguments(ClassTemplatePartialSpecializationDecl *Partial,
const TemplateArgumentList &TemplateArgs,
ArrayRef<TemplateArgument> TemplateArgs,
sema::TemplateDeductionInfo &Info);

TemplateDeductionResult
DeduceTemplateArguments(VarTemplatePartialSpecializationDecl *Partial,
const TemplateArgumentList &TemplateArgs,
ArrayRef<TemplateArgument> TemplateArgs,
sema::TemplateDeductionInfo &Info);

TemplateDeductionResult SubstituteExplicitTemplateArguments(
Expand Down Expand Up @@ -9507,7 +9507,7 @@ class Sema final {

MultiLevelTemplateArgumentList getTemplateInstantiationArgs(
const NamedDecl *D, const DeclContext *DC = nullptr, bool Final = false,
const TemplateArgumentList *Innermost = nullptr,
std::optional<ArrayRef<TemplateArgument>> Innermost = std::nullopt,
bool RelativeToPrimary = false, const FunctionDecl *Pattern = nullptr,
bool ForConstraintInstantiation = false,
bool SkipForSpecialization = false);
Expand Down Expand Up @@ -10537,7 +10537,7 @@ class Sema final {
bool AtEndOfTU = false);
VarTemplateSpecializationDecl *BuildVarTemplateInstantiation(
VarTemplateDecl *VarTemplate, VarDecl *FromVar,
const TemplateArgumentList &TemplateArgList,
const TemplateArgumentList *PartialSpecArgs,
const TemplateArgumentListInfo &TemplateArgsInfo,
SmallVectorImpl<TemplateArgument> &Converted,
SourceLocation PointOfInstantiation,
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/AST/DeclTemplate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,8 +871,7 @@ void TemplateTemplateParmDecl::setDefaultArgument(
// TemplateArgumentList Implementation
//===----------------------------------------------------------------------===//
TemplateArgumentList::TemplateArgumentList(ArrayRef<TemplateArgument> Args)
: Arguments(getTrailingObjects<TemplateArgument>()),
NumArguments(Args.size()) {
: NumArguments(Args.size()) {
std::uninitialized_copy(Args.begin(), Args.end(),
getTrailingObjects<TemplateArgument>());
}
Expand Down
20 changes: 10 additions & 10 deletions clang/lib/Sema/SemaConcept.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,11 +661,12 @@ Sema::SetupConstraintCheckingTemplateArgumentsAndScope(
// Collect the list of template arguments relative to the 'primary' template.
// We need the entire list, since the constraint is completely uninstantiated
// at this point.
MLTAL = getTemplateInstantiationArgs(FD, FD->getLexicalDeclContext(),
/*Final=*/false, /*Innermost=*/nullptr,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true);
MLTAL =
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 @@ -740,7 +741,8 @@ static unsigned
CalculateTemplateDepthForConstraints(Sema &S, const NamedDecl *ND,
bool SkipForSpecialization = false) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
ND, ND->getLexicalDeclContext(), /*Final=*/false, /*Innermost=*/nullptr,
ND, ND->getLexicalDeclContext(), /*Final=*/false,
/*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true, SkipForSpecialization);
Expand Down Expand Up @@ -780,7 +782,7 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
const Expr *ConstrExpr) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
/*Innermost=*/nullptr,
/*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
/*SkipForSpecialization*/ false);
Expand Down Expand Up @@ -1279,11 +1281,9 @@ substituteParameterMappings(Sema &S, NormalizedConstraint &N,

static bool substituteParameterMappings(Sema &S, NormalizedConstraint &N,
const ConceptSpecializationExpr *CSE) {
TemplateArgumentList TAL{TemplateArgumentList::OnStack,
CSE->getTemplateArguments()};
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
CSE->getNamedConcept(), CSE->getNamedConcept()->getLexicalDeclContext(),
/*Final=*/false, &TAL,
/*Final=*/false, CSE->getTemplateArguments(),
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true);
Expand Down
4 changes: 1 addition & 3 deletions clang/lib/Sema/SemaExprCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9113,9 +9113,7 @@ Sema::BuildExprRequirement(

auto *Param = cast<TemplateTypeParmDecl>(TPL->getParam(0));

TemplateArgumentList TAL(TemplateArgumentList::OnStack, Args);
MultiLevelTemplateArgumentList MLTAL(Param, TAL.asArray(),
/*Final=*/false);
MultiLevelTemplateArgumentList MLTAL(Param, Args, /*Final=*/false);
MLTAL.addOuterRetainedLevels(TPL->getDepth());
const TypeConstraint *TC = Param->getTypeConstraint();
assert(TC && "Type Constraint cannot be null here");
Expand Down
20 changes: 8 additions & 12 deletions clang/lib/Sema/SemaTemplate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4843,9 +4843,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
// the set of specializations, based on the closest partial specialization
// that it represents. That is,
VarDecl *InstantiationPattern = Template->getTemplatedDecl();
TemplateArgumentList TemplateArgList(TemplateArgumentList::OnStack,
CanonicalConverted);
TemplateArgumentList *InstantiationArgs = &TemplateArgList;
const TemplateArgumentList *PartialSpecArgs = nullptr;
bool AmbiguousPartialSpec = false;
typedef PartialSpecMatchResult MatchResult;
SmallVector<MatchResult, 4> Matched;
Expand All @@ -4866,7 +4864,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
TemplateDeductionInfo Info(FailedCandidates.getLocation());

if (TemplateDeductionResult Result =
DeduceTemplateArguments(Partial, TemplateArgList, Info)) {
DeduceTemplateArguments(Partial, CanonicalConverted, Info)) {
// Store the failed-deduction information for use in diagnostics, later.
// TODO: Actually use the failed-deduction info?
FailedCandidates.addCandidate().set(
Expand Down Expand Up @@ -4919,7 +4917,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,

// Instantiate using the best variable template partial specialization.
InstantiationPattern = Best->Partial;
InstantiationArgs = Best->Args;
PartialSpecArgs = Best->Args;
} else {
// -- If no match is found, the instantiation is generated
// from the primary template.
Expand All @@ -4931,7 +4929,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
// in DoMarkVarDeclReferenced().
// FIXME: LateAttrs et al.?
VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation(
Template, InstantiationPattern, *InstantiationArgs, TemplateArgs,
Template, InstantiationPattern, PartialSpecArgs, TemplateArgs,
CanonicalConverted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
if (!Decl)
return true;
Expand All @@ -4952,7 +4950,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,

if (VarTemplatePartialSpecializationDecl *D =
dyn_cast<VarTemplatePartialSpecializationDecl>(InstantiationPattern))
Decl->setInstantiationOf(D, InstantiationArgs);
Decl->setInstantiationOf(D, PartialSpecArgs);

checkSpecializationReachability(TemplateNameLoc, Decl);

Expand Down Expand Up @@ -6257,8 +6255,6 @@ bool Sema::CheckTemplateArgumentList(
TemplateArgs = std::move(NewArgs);

if (!PartialTemplateArgs) {
TemplateArgumentList StackTemplateArgs(TemplateArgumentList::OnStack,
CanonicalConverted);
// Setup the context/ThisScope for the case where we are needing to
// re-instantiate constraints outside of normal instantiation.
DeclContext *NewContext = Template->getDeclContext();
Expand All @@ -6278,7 +6274,7 @@ bool Sema::CheckTemplateArgumentList(
CXXThisScopeRAII(*this, RD, ThisQuals, RD != nullptr);

MultiLevelTemplateArgumentList MLTAL = getTemplateInstantiationArgs(
Template, NewContext, /*Final=*/false, &StackTemplateArgs,
Template, NewContext, /*Final=*/false, CanonicalConverted,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConceptInstantiation=*/true);
Expand Down Expand Up @@ -9782,8 +9778,8 @@ bool Sema::CheckFunctionTemplateSpecialization(
// specialization, with the template arguments from the previous
// specialization.
// Take copies of (semantic and syntactic) template argument lists.
const TemplateArgumentList* TemplArgs = new (Context)
TemplateArgumentList(Specialization->getTemplateSpecializationArgs());
const TemplateArgumentList *TemplArgs = TemplateArgumentList::CreateCopy(
Context, Specialization->getTemplateSpecializationArgs()->asArray());
FD->setFunctionTemplateSpecialization(
Specialization->getPrimaryTemplate(), TemplArgs, /*InsertPos=*/nullptr,
SpecInfo->getTemplateSpecializationKind(),
Expand Down
70 changes: 29 additions & 41 deletions clang/lib/Sema/SemaTemplateDeduction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2514,17 +2514,6 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
return Sema::TDK_Success;
}

static Sema::TemplateDeductionResult
DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
const TemplateArgumentList &ParamList,
const TemplateArgumentList &ArgList,
TemplateDeductionInfo &Info,
SmallVectorImpl<DeducedTemplateArgument> &Deduced) {
return DeduceTemplateArguments(S, TemplateParams, ParamList.asArray(),
ArgList.asArray(), Info, Deduced,
/*NumberOfArgumentsMustMatch=*/false);
}

/// Determine whether two template arguments are the same.
static bool isSameTemplateArg(ASTContext &Context,
TemplateArgument X,
Expand Down Expand Up @@ -2945,21 +2934,22 @@ CheckDeducedArgumentConstraints(Sema &S, TemplateDeclT *Template,
llvm::SmallVector<const Expr *, 3> AssociatedConstraints;
Template->getAssociatedConstraints(AssociatedConstraints);

bool NeedsReplacement = DeducedArgsNeedReplacement(Template);
TemplateArgumentList DeducedTAL{TemplateArgumentList::OnStack,
CanonicalDeducedArgs};
std::optional<ArrayRef<TemplateArgument>> Innermost;
// If we don't need to replace the deduced template arguments,
// we can add them immediately as the inner-most argument list.
if (!DeducedArgsNeedReplacement(Template))
Innermost = CanonicalDeducedArgs;

MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
Template, Template->getDeclContext(), /*Final=*/false,
/*InnerMost=*/NeedsReplacement ? nullptr : &DeducedTAL,
Template, Template->getDeclContext(), /*Final=*/false, Innermost,
/*RelativeToPrimary=*/true, /*Pattern=*/
nullptr, /*ForConstraintInstantiation=*/true);

// getTemplateInstantiationArgs picks up the non-deduced version of the
// template args when this is a variable template partial specialization and
// not class-scope explicit specialization, so replace with Deduced Args
// instead of adding to inner-most.
if (NeedsReplacement)
if (!Innermost)
MLTAL.replaceInnermostTemplateArguments(Template, CanonicalDeducedArgs);

if (S.CheckConstraintSatisfaction(Template, AssociatedConstraints, MLTAL,
Expand All @@ -2980,7 +2970,7 @@ static std::enable_if_t<IsPartialSpecialization<T>::value,
Sema::TemplateDeductionResult>
FinishTemplateArgumentDeduction(
Sema &S, T *Partial, bool IsPartialOrdering,
const TemplateArgumentList &TemplateArgs,
ArrayRef<TemplateArgument> TemplateArgs,
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
TemplateDeductionInfo &Info) {
// Unevaluated SFINAE context.
Expand Down Expand Up @@ -3073,7 +3063,7 @@ FinishTemplateArgumentDeduction(
// FIXME: Factor out duplication with partial specialization version above.
static Sema::TemplateDeductionResult FinishTemplateArgumentDeduction(
Sema &S, TemplateDecl *Template, bool PartialOrdering,
const TemplateArgumentList &TemplateArgs,
ArrayRef<TemplateArgument> TemplateArgs,
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
TemplateDeductionInfo &Info) {
// Unevaluated SFINAE context.
Expand Down Expand Up @@ -3122,7 +3112,7 @@ static Sema::TemplateDeductionResult FinishTemplateArgumentDeduction(
/// partial specialization per C++ [temp.class.spec.match].
Sema::TemplateDeductionResult
Sema::DeduceTemplateArguments(ClassTemplatePartialSpecializationDecl *Partial,
const TemplateArgumentList &TemplateArgs,
ArrayRef<TemplateArgument> TemplateArgs,
TemplateDeductionInfo &Info) {
if (Partial->isInvalidDecl())
return TDK_Invalid;
Expand All @@ -3144,11 +3134,10 @@ Sema::DeduceTemplateArguments(ClassTemplatePartialSpecializationDecl *Partial,

SmallVector<DeducedTemplateArgument, 4> Deduced;
Deduced.resize(Partial->getTemplateParameters()->size());
if (TemplateDeductionResult Result
= ::DeduceTemplateArguments(*this,
Partial->getTemplateParameters(),
Partial->getTemplateArgs(),
TemplateArgs, Info, Deduced))
if (TemplateDeductionResult Result = ::DeduceTemplateArguments(
*this, Partial->getTemplateParameters(),
Partial->getTemplateArgs().asArray(), TemplateArgs, Info, Deduced,
/*NumberOfArgumentsMustMatch=*/false))
return Result;

SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(), Deduced.end());
Expand All @@ -3174,7 +3163,7 @@ Sema::DeduceTemplateArguments(ClassTemplatePartialSpecializationDecl *Partial,
/// partial specialization per C++ [temp.class.spec.match].
Sema::TemplateDeductionResult
Sema::DeduceTemplateArguments(VarTemplatePartialSpecializationDecl *Partial,
const TemplateArgumentList &TemplateArgs,
ArrayRef<TemplateArgument> TemplateArgs,
TemplateDeductionInfo &Info) {
if (Partial->isInvalidDecl())
return TDK_Invalid;
Expand All @@ -3197,8 +3186,9 @@ Sema::DeduceTemplateArguments(VarTemplatePartialSpecializationDecl *Partial,
SmallVector<DeducedTemplateArgument, 4> Deduced;
Deduced.resize(Partial->getTemplateParameters()->size());
if (TemplateDeductionResult Result = ::DeduceTemplateArguments(
*this, Partial->getTemplateParameters(), Partial->getTemplateArgs(),
TemplateArgs, Info, Deduced))
*this, Partial->getTemplateParameters(),
Partial->getTemplateArgs().asArray(), TemplateArgs, Info, Deduced,
/*NumberOfArgumentsMustMatch=*/false))
return Result;

SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(), Deduced.end());
Expand Down Expand Up @@ -3427,15 +3417,15 @@ Sema::TemplateDeductionResult Sema::SubstituteExplicitTemplateArguments(
// specification.
SmallVector<QualType, 4> ExceptionStorage;
if (getLangOpts().CPlusPlus17 &&
SubstExceptionSpec(Function->getLocation(), EPI.ExceptionSpec,
ExceptionStorage,
getTemplateInstantiationArgs(
FunctionTemplate, nullptr, /*Final=*/true,
/*Innermost=*/SugaredExplicitArgumentList,
/*RelativeToPrimary=*/false,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/false,
/*SkipForSpecialization=*/true)))
SubstExceptionSpec(
Function->getLocation(), EPI.ExceptionSpec, ExceptionStorage,
getTemplateInstantiationArgs(
FunctionTemplate, nullptr, /*Final=*/true,
/*Innermost=*/SugaredExplicitArgumentList->asArray(),
/*RelativeToPrimary=*/false,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/false,
/*SkipForSpecialization=*/true)))
return TDK_SubstitutionFailure;

*FunctionType = BuildFunctionType(ResultType, ParamTypes,
Expand Down Expand Up @@ -5802,10 +5792,8 @@ static bool isAtLeastAsSpecializedAs(Sema &S, QualType T1, QualType T2,
bool AtLeastAsSpecialized;
S.runWithSufficientStackSpace(Info.getLocation(), [&] {
AtLeastAsSpecialized = !FinishTemplateArgumentDeduction(
S, P2, /*IsPartialOrdering=*/true,
TemplateArgumentList(TemplateArgumentList::OnStack,
TST1->template_arguments()),
Deduced, Info);
S, P2, /*IsPartialOrdering=*/true, TST1->template_arguments(), Deduced,
Info);
});
return AtLeastAsSpecialized;
}
Expand Down
Loading