-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Reland: [clang] Improved canonicalization for template specialization types #135414
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
Conversation
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis changes the TemplateArgument representation to hold a flag indicating whether a tempalte argument of expression type is supposed to be canonical or not. This gets one step closer to solving #92292 This still doesn't try to unique as-written TSTs. While this would increase the amount of memory savings and make code dealing with the AST more well-behaved, profiling template argument lists is still too expensive for this to be worthwhile, at least for now. This also fixes the context creation of TSTs, so that they don't in some cases get incorrectly flagged as sugar over their own canonical form. This is captured in the test expectation change of some AST dumps. This fixes some places which were unnecessarily canonicalizing these TSTs. Patch is 90.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135414.diff 29 Files Affected:
diff --git a/.ci/compute_projects.py b/.ci/compute_projects.py
index ff43547c9bbe5..9930998b2e94b 100644
--- a/.ci/compute_projects.py
+++ b/.ci/compute_projects.py
@@ -49,7 +49,7 @@
},
"lld": {"bolt", "cross-project-tests"},
# TODO(issues/132795): LLDB should be enabled on clang changes.
- "clang": {"clang-tools-extra", "compiler-rt", "cross-project-tests"},
+ "clang": {"clang-tools-extra", "compiler-rt", "cross-project-tests", "lldb"},
"clang-tools-extra": {"libc"},
"mlir": {"flang"},
}
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 66b587f00ff4a..3b991e5e9013f 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -439,7 +439,8 @@ QualType declaredType(const TypeDecl *D) {
if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
if (const auto *Args = CTSD->getTemplateArgsAsWritten())
return Context.getTemplateSpecializationType(
- TemplateName(CTSD->getSpecializedTemplate()), Args->arguments());
+ TemplateName(CTSD->getSpecializedTemplate()), Args->arguments(),
+ /*CanonicalArgs=*/std::nullopt);
return Context.getTypeDeclType(D);
}
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c45965dc4d82..11f62bc881b03 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -303,6 +303,8 @@ Improvements to Clang's diagnostics
- Clang now better preserves the sugared types of pointers to member.
- Clang now better preserves the presence of the template keyword with dependent
prefixes.
+- Clang now in more cases avoids printing 'type-parameter-X-X' instead of the name of
+ the template parameter.
- Clang now respects the current language mode when printing expressions in
diagnostics. This fixes a bunch of `bool` being printed as `_Bool`, and also
a bunch of HLSL types being printed as their C++ equivalents.
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index b1e6344576eb5..b8ea2af9215d2 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -367,9 +367,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
const ASTContext&>
CanonTemplateTemplateParms;
- TemplateTemplateParmDecl *
- getCanonicalTemplateTemplateParmDecl(TemplateTemplateParmDecl *TTP) const;
-
/// The typedef for the __int128_t type.
mutable TypedefDecl *Int128Decl = nullptr;
@@ -1811,22 +1808,26 @@ class ASTContext : public RefCountedBase<ASTContext> {
bool ParameterPack,
TemplateTypeParmDecl *ParmDecl = nullptr) const;
- QualType getTemplateSpecializationType(TemplateName T,
- ArrayRef<TemplateArgument> Args,
- QualType Canon = QualType()) const;
+ QualType getCanonicalTemplateSpecializationType(
+ TemplateName T, ArrayRef<TemplateArgument> CanonicalArgs) const;
QualType
- getCanonicalTemplateSpecializationType(TemplateName T,
- ArrayRef<TemplateArgument> Args) const;
+ getTemplateSpecializationType(TemplateName T,
+ ArrayRef<TemplateArgument> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs,
+ QualType Underlying = QualType()) const;
- QualType getTemplateSpecializationType(TemplateName T,
- ArrayRef<TemplateArgumentLoc> Args,
- QualType Canon = QualType()) const;
+ QualType
+ getTemplateSpecializationType(TemplateName T,
+ ArrayRef<TemplateArgumentLoc> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs,
+ QualType Canon = QualType()) const;
- TypeSourceInfo *
- getTemplateSpecializationTypeInfo(TemplateName T, SourceLocation TLoc,
- const TemplateArgumentListInfo &Args,
- QualType Canon = QualType()) const;
+ TypeSourceInfo *getTemplateSpecializationTypeInfo(
+ TemplateName T, SourceLocation TLoc,
+ const TemplateArgumentListInfo &SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs,
+ QualType Canon = QualType()) const;
QualType getParenType(QualType NamedType) const;
@@ -2942,6 +2943,21 @@ class ASTContext : public RefCountedBase<ASTContext> {
TemplateArgument getCanonicalTemplateArgument(const TemplateArgument &Arg)
const;
+ /// Canonicalize the given template argument list.
+ ///
+ /// Returns true if any arguments were non-canonical, false otherwise.
+ bool
+ canonicalizeTemplateArguments(MutableArrayRef<TemplateArgument> Args) const;
+
+ /// Canonicalize the given TemplateTemplateParmDecl.
+ TemplateTemplateParmDecl *
+ getCanonicalTemplateTemplateParmDecl(TemplateTemplateParmDecl *TTP) const;
+
+ TemplateTemplateParmDecl *findCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *TTP) const;
+ TemplateTemplateParmDecl *insertCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *CanonTTP) const;
+
/// Type Query functions. If the type is an instance of the specified class,
/// return the Type pointer for the underlying maximally pretty type. This
/// is a member of ASTContext because this may need to do some amount of
diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 90537d47dd9c9..33336d57b6298 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -877,11 +877,14 @@ let Class = PropertyTypeCase<TemplateArgument, "Expression"> in {
def : Property<"expression", ExprRef> {
let Read = [{ node.getAsExpr() }];
}
+ def : Property<"IsCanonical", Bool> {
+ let Read = [{ node.isCanonicalExpr() }];
+ }
def : Property<"isDefaulted", Bool> {
let Read = [{ node.getIsDefaulted() }];
}
def : Creator<[{
- return TemplateArgument(expression, isDefaulted);
+ return TemplateArgument(expression, IsCanonical, isDefaulted);
}]>;
}
let Class = PropertyTypeCase<TemplateArgument, "Pack"> in {
diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h
index bea624eb04942..279feb858e665 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -167,6 +167,8 @@ class TemplateArgument {
unsigned Kind : 31;
LLVM_PREFERRED_TYPE(bool)
unsigned IsDefaulted : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned IsCanonicalExpr : 1;
uintptr_t V;
};
union {
@@ -187,7 +189,8 @@ class TemplateArgument {
public:
/// Construct an empty, invalid template argument.
- constexpr TemplateArgument() : TypeOrValue({Null, 0, /* IsDefaulted */ 0}) {}
+ constexpr TemplateArgument()
+ : TypeOrValue{Null, /*IsDefaulted=*/0, /*IsCanonicalExpr=*/0, /*V=*/0} {}
/// Construct a template type argument.
TemplateArgument(QualType T, bool isNullPtr = false,
@@ -262,9 +265,10 @@ class TemplateArgument {
/// This form of template argument only occurs in template argument
/// lists used for dependent types and for expression; it will not
/// occur in a non-dependent, canonical template argument list.
- explicit TemplateArgument(Expr *E, bool IsDefaulted = false) {
+ TemplateArgument(Expr *E, bool IsCanonical, bool IsDefaulted = false) {
TypeOrValue.Kind = Expression;
TypeOrValue.IsDefaulted = IsDefaulted;
+ TypeOrValue.IsCanonicalExpr = IsCanonical;
TypeOrValue.V = reinterpret_cast<uintptr_t>(E);
}
@@ -407,6 +411,11 @@ class TemplateArgument {
return reinterpret_cast<Expr *>(TypeOrValue.V);
}
+ bool isCanonicalExpr() const {
+ assert(getKind() == Expression && "Unexpected kind");
+ return TypeOrValue.IsCanonicalExpr;
+ }
+
/// Iterator that traverses the elements of a template argument pack.
using pack_iterator = const TemplateArgument *;
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9f6189440fabf..dc57170bf9160 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6676,10 +6676,9 @@ class TemplateSpecializationType : public Type, public llvm::FoldingSetNode {
/// replacement must, recursively, be one of these).
TemplateName Template;
- TemplateSpecializationType(TemplateName T,
+ TemplateSpecializationType(TemplateName T, bool IsAlias,
ArrayRef<TemplateArgument> Args,
- QualType Canon,
- QualType Aliased);
+ QualType Underlying);
public:
/// Determine whether any of the given template arguments are dependent.
@@ -6747,7 +6746,7 @@ class TemplateSpecializationType : public Type, public llvm::FoldingSetNode {
void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Ctx);
static void Profile(llvm::FoldingSetNodeID &ID, TemplateName T,
- ArrayRef<TemplateArgument> Args,
+ ArrayRef<TemplateArgument> Args, QualType Underlying,
const ASTContext &Context);
static bool classof(const Type *T) {
diff --git a/clang/include/clang/AST/TypeProperties.td b/clang/include/clang/AST/TypeProperties.td
index 66d490850678a..3bf9239e9cbf5 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -737,39 +737,19 @@ let Class = DependentAddressSpaceType in {
}
let Class = TemplateSpecializationType in {
- def : Property<"dependent", Bool> {
- let Read = [{ node->isDependentType() }];
- }
def : Property<"templateName", TemplateName> {
let Read = [{ node->getTemplateName() }];
}
- def : Property<"templateArguments", Array<TemplateArgument>> {
+ def : Property<"args", Array<TemplateArgument>> {
let Read = [{ node->template_arguments() }];
}
- def : Property<"underlyingType", Optional<QualType>> {
- let Read = [{
- node->isTypeAlias()
- ? std::optional<QualType>(node->getAliasedType())
- : node->isCanonicalUnqualified()
- ? std::nullopt
- : std::optional<QualType>(node->getCanonicalTypeInternal())
- }];
+ def : Property<"UnderlyingType", QualType> {
+ let Read = [{ node->isCanonicalUnqualified() ? QualType() :
+ node->desugar() }];
}
def : Creator<[{
- QualType result;
- if (!underlyingType) {
- result = ctx.getCanonicalTemplateSpecializationType(templateName,
- templateArguments);
- } else {
- result = ctx.getTemplateSpecializationType(templateName,
- templateArguments,
- *underlyingType);
- }
- if (dependent)
- const_cast<Type *>(result.getTypePtr())
- ->addDependence(TypeDependence::DependentInstantiation);
- return result;
+ return ctx.getTemplateSpecializationType(templateName, args, std::nullopt, UnderlyingType);
}]>;
}
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 00e2fa267a460..b8e6245230475 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -844,6 +844,31 @@ ASTContext::getCanonicalTemplateTemplateParmDecl(
return CanonTTP;
}
+TemplateTemplateParmDecl *
+ASTContext::findCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *TTP) const {
+ llvm::FoldingSetNodeID ID;
+ CanonicalTemplateTemplateParm::Profile(ID, *this, TTP);
+ void *InsertPos = nullptr;
+ CanonicalTemplateTemplateParm *Canonical =
+ CanonTemplateTemplateParms.FindNodeOrInsertPos(ID, InsertPos);
+ return Canonical ? Canonical->getParam() : nullptr;
+}
+
+TemplateTemplateParmDecl *
+ASTContext::insertCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *CanonTTP) const {
+ llvm::FoldingSetNodeID ID;
+ CanonicalTemplateTemplateParm::Profile(ID, *this, CanonTTP);
+ void *InsertPos = nullptr;
+ if (auto *Existing =
+ CanonTemplateTemplateParms.FindNodeOrInsertPos(ID, InsertPos))
+ return Existing->getParam();
+ CanonTemplateTemplateParms.InsertNode(
+ new (*this) CanonicalTemplateTemplateParm(CanonTTP), InsertPos);
+ return CanonTTP;
+}
+
/// Check if a type can have its sanitizer instrumentation elided based on its
/// presence within an ignorelist.
bool ASTContext::isTypeIgnoredBySanitizer(const SanitizerMask &Mask,
@@ -3083,12 +3108,19 @@ static auto getCanonicalTemplateArguments(const ASTContext &C,
ArrayRef<TemplateArgument> Args,
bool &AnyNonCanonArgs) {
SmallVector<TemplateArgument, 16> CanonArgs(Args);
- for (auto &Arg : CanonArgs) {
+ AnyNonCanonArgs |= C.canonicalizeTemplateArguments(CanonArgs);
+ return CanonArgs;
+}
+
+bool ASTContext::canonicalizeTemplateArguments(
+ MutableArrayRef<TemplateArgument> Args) const {
+ bool AnyNonCanonArgs = false;
+ for (auto &Arg : Args) {
TemplateArgument OrigArg = Arg;
- Arg = C.getCanonicalTemplateArgument(Arg);
+ Arg = getCanonicalTemplateArgument(Arg);
AnyNonCanonArgs |= !Arg.structurallyEquals(OrigArg);
}
- return CanonArgs;
+ return AnyNonCanonArgs;
}
//===----------------------------------------------------------------------===//
@@ -5538,129 +5570,118 @@ QualType ASTContext::getTemplateTypeParmType(unsigned Depth, unsigned Index,
return QualType(TypeParm, 0);
}
-TypeSourceInfo *
-ASTContext::getTemplateSpecializationTypeInfo(TemplateName Name,
- SourceLocation NameLoc,
- const TemplateArgumentListInfo &Args,
- QualType Underlying) const {
- assert(!Name.getAsDependentTemplateName() &&
- "No dependent template names here!");
- QualType TST =
- getTemplateSpecializationType(Name, Args.arguments(), Underlying);
+TypeSourceInfo *ASTContext::getTemplateSpecializationTypeInfo(
+ TemplateName Name, SourceLocation NameLoc,
+ const TemplateArgumentListInfo &SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs, QualType Underlying) const {
+ QualType TST = getTemplateSpecializationType(Name, SpecifiedArgs.arguments(),
+ CanonicalArgs, Underlying);
TypeSourceInfo *DI = CreateTypeSourceInfo(TST);
TemplateSpecializationTypeLoc TL =
DI->getTypeLoc().castAs<TemplateSpecializationTypeLoc>();
TL.setTemplateKeywordLoc(SourceLocation());
TL.setTemplateNameLoc(NameLoc);
- TL.setLAngleLoc(Args.getLAngleLoc());
- TL.setRAngleLoc(Args.getRAngleLoc());
+ TL.setLAngleLoc(SpecifiedArgs.getLAngleLoc());
+ TL.setRAngleLoc(SpecifiedArgs.getRAngleLoc());
for (unsigned i = 0, e = TL.getNumArgs(); i != e; ++i)
- TL.setArgLocInfo(i, Args[i].getLocInfo());
+ TL.setArgLocInfo(i, SpecifiedArgs[i].getLocInfo());
return DI;
}
-QualType
-ASTContext::getTemplateSpecializationType(TemplateName Template,
- ArrayRef<TemplateArgumentLoc> Args,
- QualType Underlying) const {
- assert(!Template.getAsDependentTemplateName() &&
- "No dependent template names here!");
+QualType ASTContext::getTemplateSpecializationType(
+ TemplateName Template, ArrayRef<TemplateArgumentLoc> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs, QualType Underlying) const {
+ SmallVector<TemplateArgument, 4> SpecifiedArgVec;
+ SpecifiedArgVec.reserve(SpecifiedArgs.size());
+ for (const TemplateArgumentLoc &Arg : SpecifiedArgs)
+ SpecifiedArgVec.push_back(Arg.getArgument());
- SmallVector<TemplateArgument, 4> ArgVec;
- ArgVec.reserve(Args.size());
- for (const TemplateArgumentLoc &Arg : Args)
- ArgVec.push_back(Arg.getArgument());
-
- return getTemplateSpecializationType(Template, ArgVec, Underlying);
+ return getTemplateSpecializationType(Template, SpecifiedArgVec, CanonicalArgs,
+ Underlying);
}
-#ifndef NDEBUG
-static bool hasAnyPackExpansions(ArrayRef<TemplateArgument> Args) {
+[[maybe_unused]] static bool
+hasAnyPackExpansions(ArrayRef<TemplateArgument> Args) {
for (const TemplateArgument &Arg : Args)
if (Arg.isPackExpansion())
return true;
-
- return true;
+ return false;
}
-#endif
-QualType
-ASTContext::getTemplateSpecializationType(TemplateName Template,
- ArrayRef<TemplateArgument> Args,
- QualType Underlying) const {
- assert(!Template.getAsDependentTemplateName() &&
- "No dependent template names here!");
+QualType ASTContext::getCanonicalTemplateSpecializationType(
+ TemplateName Template, ArrayRef<TemplateArgument> Args) const {
+ assert(Template ==
+ getCanonicalTemplateName(Template, /*IgnoreDeduced=*/true));
+ assert(!Args.empty());
+#ifndef NDEBUG
+ for (const auto &Arg : Args)
+ assert(Arg.structurallyEquals(getCanonicalTemplateArgument(Arg)));
+#endif
- const auto *TD = Template.getAsTemplateDecl(/*IgnoreDeduced=*/true);
- bool IsTypeAlias = TD && TD->isTypeAlias();
- QualType CanonType;
- if (!Underlying.isNull())
- CanonType = getCanonicalType(Underlying);
- else {
- // We can get here with an alias template when the specialization contains
- // a pack expansion that does not match up with a parameter pack.
- assert((!IsTypeAlias || hasAnyPackExpansions(Args)) &&
- "Caller must compute aliased type");
- IsTypeAlias = false;
- CanonType = getCanonicalTemplateSpecializationType(Template, Args);
- }
+ llvm::FoldingSetNodeID ID;
+ TemplateSpecializationType::Profile(ID, Template, Args, QualType(), *this);
+ void *InsertPos = nullptr;
+ if (auto *T = TemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos))
+ return QualType(T, 0);
- // Allocate the (non-canonical) template specialization type, but don't
- // try to unique it: these types typically have location information that
- // we don't unique and don't want to lose.
void *Mem = Allocate(sizeof(TemplateSpecializationType) +
- sizeof(TemplateArgument) * Args.size() +
- (IsTypeAlias ? sizeof(QualType) : 0),
+ sizeof(TemplateArgument) * Args.size(),
alignof(TemplateSpecializationType));
- auto *Spec
- = new (Mem) TemplateSpecializationType(Template, Args, CanonType,
- IsTypeAlias ? Underlying : QualType());
-
+ auto *Spec = new (Mem)
+ TemplateSpecializationType(Template, /*IsAlias=*/false, Args, QualType());
+ assert(Spec->isDependentType() &&
+ "canonical template specialization must be dependent");
Types.push_back(Spec);
+ TemplateSpecializationTypes.InsertNode(Spec, InsertPos);
return QualType(Spec, 0);
}
-QualType ASTContext::getCanonicalTemplateSpecializationType(
- TemplateName Template, ArrayRef<TemplateArgument> Args) const {
- assert(!Template.getAsDependentTemplateName() &&
+QualType ASTContext::getTemplateSpecializationType(
+ TemplateName Template, ArrayRef<TemplateArgument> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs, QualType Underlying) const {
+ assert(!Template.getUnderlying().getAsDependentTemplateName() &&
"No dependent template names here!");
- // Build the canonical template specialization type.
- // Any DeducedTemplateNames are ignored, because the effective name of a TST
- // accounts for the TST arguments laid over any default arguments contained in
- // its name.
- TemplateName CanonTemplate =
- getCanonicalTemplateName(Template, /*IgnoreDeduced=*/true);
-
- bool AnyNonCanonArgs = false;
- auto CanonArgs =
- ::getCanonicalTemplateArguments(*this, Args, AnyNonCanonArgs);
-
- // Determine whether this canonical template specialization type already
- // exists.
- llvm::FoldingSetNodeID ID;
- Template...
[truncated]
|
@llvm/pr-subscribers-clang-tools-extra Author: Matheus Izvekov (mizvekov) ChangesThis changes the TemplateArgument representation to hold a flag indicating whether a tempalte argument of expression type is supposed to be canonical or not. This gets one step closer to solving #92292 This still doesn't try to unique as-written TSTs. While this would increase the amount of memory savings and make code dealing with the AST more well-behaved, profiling template argument lists is still too expensive for this to be worthwhile, at least for now. This also fixes the context creation of TSTs, so that they don't in some cases get incorrectly flagged as sugar over their own canonical form. This is captured in the test expectation change of some AST dumps. This fixes some places which were unnecessarily canonicalizing these TSTs. Patch is 90.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135414.diff 29 Files Affected:
diff --git a/.ci/compute_projects.py b/.ci/compute_projects.py
index ff43547c9bbe5..9930998b2e94b 100644
--- a/.ci/compute_projects.py
+++ b/.ci/compute_projects.py
@@ -49,7 +49,7 @@
},
"lld": {"bolt", "cross-project-tests"},
# TODO(issues/132795): LLDB should be enabled on clang changes.
- "clang": {"clang-tools-extra", "compiler-rt", "cross-project-tests"},
+ "clang": {"clang-tools-extra", "compiler-rt", "cross-project-tests", "lldb"},
"clang-tools-extra": {"libc"},
"mlir": {"flang"},
}
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 66b587f00ff4a..3b991e5e9013f 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -439,7 +439,8 @@ QualType declaredType(const TypeDecl *D) {
if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
if (const auto *Args = CTSD->getTemplateArgsAsWritten())
return Context.getTemplateSpecializationType(
- TemplateName(CTSD->getSpecializedTemplate()), Args->arguments());
+ TemplateName(CTSD->getSpecializedTemplate()), Args->arguments(),
+ /*CanonicalArgs=*/std::nullopt);
return Context.getTypeDeclType(D);
}
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c45965dc4d82..11f62bc881b03 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -303,6 +303,8 @@ Improvements to Clang's diagnostics
- Clang now better preserves the sugared types of pointers to member.
- Clang now better preserves the presence of the template keyword with dependent
prefixes.
+- Clang now in more cases avoids printing 'type-parameter-X-X' instead of the name of
+ the template parameter.
- Clang now respects the current language mode when printing expressions in
diagnostics. This fixes a bunch of `bool` being printed as `_Bool`, and also
a bunch of HLSL types being printed as their C++ equivalents.
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index b1e6344576eb5..b8ea2af9215d2 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -367,9 +367,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
const ASTContext&>
CanonTemplateTemplateParms;
- TemplateTemplateParmDecl *
- getCanonicalTemplateTemplateParmDecl(TemplateTemplateParmDecl *TTP) const;
-
/// The typedef for the __int128_t type.
mutable TypedefDecl *Int128Decl = nullptr;
@@ -1811,22 +1808,26 @@ class ASTContext : public RefCountedBase<ASTContext> {
bool ParameterPack,
TemplateTypeParmDecl *ParmDecl = nullptr) const;
- QualType getTemplateSpecializationType(TemplateName T,
- ArrayRef<TemplateArgument> Args,
- QualType Canon = QualType()) const;
+ QualType getCanonicalTemplateSpecializationType(
+ TemplateName T, ArrayRef<TemplateArgument> CanonicalArgs) const;
QualType
- getCanonicalTemplateSpecializationType(TemplateName T,
- ArrayRef<TemplateArgument> Args) const;
+ getTemplateSpecializationType(TemplateName T,
+ ArrayRef<TemplateArgument> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs,
+ QualType Underlying = QualType()) const;
- QualType getTemplateSpecializationType(TemplateName T,
- ArrayRef<TemplateArgumentLoc> Args,
- QualType Canon = QualType()) const;
+ QualType
+ getTemplateSpecializationType(TemplateName T,
+ ArrayRef<TemplateArgumentLoc> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs,
+ QualType Canon = QualType()) const;
- TypeSourceInfo *
- getTemplateSpecializationTypeInfo(TemplateName T, SourceLocation TLoc,
- const TemplateArgumentListInfo &Args,
- QualType Canon = QualType()) const;
+ TypeSourceInfo *getTemplateSpecializationTypeInfo(
+ TemplateName T, SourceLocation TLoc,
+ const TemplateArgumentListInfo &SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs,
+ QualType Canon = QualType()) const;
QualType getParenType(QualType NamedType) const;
@@ -2942,6 +2943,21 @@ class ASTContext : public RefCountedBase<ASTContext> {
TemplateArgument getCanonicalTemplateArgument(const TemplateArgument &Arg)
const;
+ /// Canonicalize the given template argument list.
+ ///
+ /// Returns true if any arguments were non-canonical, false otherwise.
+ bool
+ canonicalizeTemplateArguments(MutableArrayRef<TemplateArgument> Args) const;
+
+ /// Canonicalize the given TemplateTemplateParmDecl.
+ TemplateTemplateParmDecl *
+ getCanonicalTemplateTemplateParmDecl(TemplateTemplateParmDecl *TTP) const;
+
+ TemplateTemplateParmDecl *findCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *TTP) const;
+ TemplateTemplateParmDecl *insertCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *CanonTTP) const;
+
/// Type Query functions. If the type is an instance of the specified class,
/// return the Type pointer for the underlying maximally pretty type. This
/// is a member of ASTContext because this may need to do some amount of
diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 90537d47dd9c9..33336d57b6298 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -877,11 +877,14 @@ let Class = PropertyTypeCase<TemplateArgument, "Expression"> in {
def : Property<"expression", ExprRef> {
let Read = [{ node.getAsExpr() }];
}
+ def : Property<"IsCanonical", Bool> {
+ let Read = [{ node.isCanonicalExpr() }];
+ }
def : Property<"isDefaulted", Bool> {
let Read = [{ node.getIsDefaulted() }];
}
def : Creator<[{
- return TemplateArgument(expression, isDefaulted);
+ return TemplateArgument(expression, IsCanonical, isDefaulted);
}]>;
}
let Class = PropertyTypeCase<TemplateArgument, "Pack"> in {
diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h
index bea624eb04942..279feb858e665 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -167,6 +167,8 @@ class TemplateArgument {
unsigned Kind : 31;
LLVM_PREFERRED_TYPE(bool)
unsigned IsDefaulted : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned IsCanonicalExpr : 1;
uintptr_t V;
};
union {
@@ -187,7 +189,8 @@ class TemplateArgument {
public:
/// Construct an empty, invalid template argument.
- constexpr TemplateArgument() : TypeOrValue({Null, 0, /* IsDefaulted */ 0}) {}
+ constexpr TemplateArgument()
+ : TypeOrValue{Null, /*IsDefaulted=*/0, /*IsCanonicalExpr=*/0, /*V=*/0} {}
/// Construct a template type argument.
TemplateArgument(QualType T, bool isNullPtr = false,
@@ -262,9 +265,10 @@ class TemplateArgument {
/// This form of template argument only occurs in template argument
/// lists used for dependent types and for expression; it will not
/// occur in a non-dependent, canonical template argument list.
- explicit TemplateArgument(Expr *E, bool IsDefaulted = false) {
+ TemplateArgument(Expr *E, bool IsCanonical, bool IsDefaulted = false) {
TypeOrValue.Kind = Expression;
TypeOrValue.IsDefaulted = IsDefaulted;
+ TypeOrValue.IsCanonicalExpr = IsCanonical;
TypeOrValue.V = reinterpret_cast<uintptr_t>(E);
}
@@ -407,6 +411,11 @@ class TemplateArgument {
return reinterpret_cast<Expr *>(TypeOrValue.V);
}
+ bool isCanonicalExpr() const {
+ assert(getKind() == Expression && "Unexpected kind");
+ return TypeOrValue.IsCanonicalExpr;
+ }
+
/// Iterator that traverses the elements of a template argument pack.
using pack_iterator = const TemplateArgument *;
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9f6189440fabf..dc57170bf9160 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6676,10 +6676,9 @@ class TemplateSpecializationType : public Type, public llvm::FoldingSetNode {
/// replacement must, recursively, be one of these).
TemplateName Template;
- TemplateSpecializationType(TemplateName T,
+ TemplateSpecializationType(TemplateName T, bool IsAlias,
ArrayRef<TemplateArgument> Args,
- QualType Canon,
- QualType Aliased);
+ QualType Underlying);
public:
/// Determine whether any of the given template arguments are dependent.
@@ -6747,7 +6746,7 @@ class TemplateSpecializationType : public Type, public llvm::FoldingSetNode {
void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Ctx);
static void Profile(llvm::FoldingSetNodeID &ID, TemplateName T,
- ArrayRef<TemplateArgument> Args,
+ ArrayRef<TemplateArgument> Args, QualType Underlying,
const ASTContext &Context);
static bool classof(const Type *T) {
diff --git a/clang/include/clang/AST/TypeProperties.td b/clang/include/clang/AST/TypeProperties.td
index 66d490850678a..3bf9239e9cbf5 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -737,39 +737,19 @@ let Class = DependentAddressSpaceType in {
}
let Class = TemplateSpecializationType in {
- def : Property<"dependent", Bool> {
- let Read = [{ node->isDependentType() }];
- }
def : Property<"templateName", TemplateName> {
let Read = [{ node->getTemplateName() }];
}
- def : Property<"templateArguments", Array<TemplateArgument>> {
+ def : Property<"args", Array<TemplateArgument>> {
let Read = [{ node->template_arguments() }];
}
- def : Property<"underlyingType", Optional<QualType>> {
- let Read = [{
- node->isTypeAlias()
- ? std::optional<QualType>(node->getAliasedType())
- : node->isCanonicalUnqualified()
- ? std::nullopt
- : std::optional<QualType>(node->getCanonicalTypeInternal())
- }];
+ def : Property<"UnderlyingType", QualType> {
+ let Read = [{ node->isCanonicalUnqualified() ? QualType() :
+ node->desugar() }];
}
def : Creator<[{
- QualType result;
- if (!underlyingType) {
- result = ctx.getCanonicalTemplateSpecializationType(templateName,
- templateArguments);
- } else {
- result = ctx.getTemplateSpecializationType(templateName,
- templateArguments,
- *underlyingType);
- }
- if (dependent)
- const_cast<Type *>(result.getTypePtr())
- ->addDependence(TypeDependence::DependentInstantiation);
- return result;
+ return ctx.getTemplateSpecializationType(templateName, args, std::nullopt, UnderlyingType);
}]>;
}
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 00e2fa267a460..b8e6245230475 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -844,6 +844,31 @@ ASTContext::getCanonicalTemplateTemplateParmDecl(
return CanonTTP;
}
+TemplateTemplateParmDecl *
+ASTContext::findCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *TTP) const {
+ llvm::FoldingSetNodeID ID;
+ CanonicalTemplateTemplateParm::Profile(ID, *this, TTP);
+ void *InsertPos = nullptr;
+ CanonicalTemplateTemplateParm *Canonical =
+ CanonTemplateTemplateParms.FindNodeOrInsertPos(ID, InsertPos);
+ return Canonical ? Canonical->getParam() : nullptr;
+}
+
+TemplateTemplateParmDecl *
+ASTContext::insertCanonicalTemplateTemplateParmDeclInternal(
+ TemplateTemplateParmDecl *CanonTTP) const {
+ llvm::FoldingSetNodeID ID;
+ CanonicalTemplateTemplateParm::Profile(ID, *this, CanonTTP);
+ void *InsertPos = nullptr;
+ if (auto *Existing =
+ CanonTemplateTemplateParms.FindNodeOrInsertPos(ID, InsertPos))
+ return Existing->getParam();
+ CanonTemplateTemplateParms.InsertNode(
+ new (*this) CanonicalTemplateTemplateParm(CanonTTP), InsertPos);
+ return CanonTTP;
+}
+
/// Check if a type can have its sanitizer instrumentation elided based on its
/// presence within an ignorelist.
bool ASTContext::isTypeIgnoredBySanitizer(const SanitizerMask &Mask,
@@ -3083,12 +3108,19 @@ static auto getCanonicalTemplateArguments(const ASTContext &C,
ArrayRef<TemplateArgument> Args,
bool &AnyNonCanonArgs) {
SmallVector<TemplateArgument, 16> CanonArgs(Args);
- for (auto &Arg : CanonArgs) {
+ AnyNonCanonArgs |= C.canonicalizeTemplateArguments(CanonArgs);
+ return CanonArgs;
+}
+
+bool ASTContext::canonicalizeTemplateArguments(
+ MutableArrayRef<TemplateArgument> Args) const {
+ bool AnyNonCanonArgs = false;
+ for (auto &Arg : Args) {
TemplateArgument OrigArg = Arg;
- Arg = C.getCanonicalTemplateArgument(Arg);
+ Arg = getCanonicalTemplateArgument(Arg);
AnyNonCanonArgs |= !Arg.structurallyEquals(OrigArg);
}
- return CanonArgs;
+ return AnyNonCanonArgs;
}
//===----------------------------------------------------------------------===//
@@ -5538,129 +5570,118 @@ QualType ASTContext::getTemplateTypeParmType(unsigned Depth, unsigned Index,
return QualType(TypeParm, 0);
}
-TypeSourceInfo *
-ASTContext::getTemplateSpecializationTypeInfo(TemplateName Name,
- SourceLocation NameLoc,
- const TemplateArgumentListInfo &Args,
- QualType Underlying) const {
- assert(!Name.getAsDependentTemplateName() &&
- "No dependent template names here!");
- QualType TST =
- getTemplateSpecializationType(Name, Args.arguments(), Underlying);
+TypeSourceInfo *ASTContext::getTemplateSpecializationTypeInfo(
+ TemplateName Name, SourceLocation NameLoc,
+ const TemplateArgumentListInfo &SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs, QualType Underlying) const {
+ QualType TST = getTemplateSpecializationType(Name, SpecifiedArgs.arguments(),
+ CanonicalArgs, Underlying);
TypeSourceInfo *DI = CreateTypeSourceInfo(TST);
TemplateSpecializationTypeLoc TL =
DI->getTypeLoc().castAs<TemplateSpecializationTypeLoc>();
TL.setTemplateKeywordLoc(SourceLocation());
TL.setTemplateNameLoc(NameLoc);
- TL.setLAngleLoc(Args.getLAngleLoc());
- TL.setRAngleLoc(Args.getRAngleLoc());
+ TL.setLAngleLoc(SpecifiedArgs.getLAngleLoc());
+ TL.setRAngleLoc(SpecifiedArgs.getRAngleLoc());
for (unsigned i = 0, e = TL.getNumArgs(); i != e; ++i)
- TL.setArgLocInfo(i, Args[i].getLocInfo());
+ TL.setArgLocInfo(i, SpecifiedArgs[i].getLocInfo());
return DI;
}
-QualType
-ASTContext::getTemplateSpecializationType(TemplateName Template,
- ArrayRef<TemplateArgumentLoc> Args,
- QualType Underlying) const {
- assert(!Template.getAsDependentTemplateName() &&
- "No dependent template names here!");
+QualType ASTContext::getTemplateSpecializationType(
+ TemplateName Template, ArrayRef<TemplateArgumentLoc> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs, QualType Underlying) const {
+ SmallVector<TemplateArgument, 4> SpecifiedArgVec;
+ SpecifiedArgVec.reserve(SpecifiedArgs.size());
+ for (const TemplateArgumentLoc &Arg : SpecifiedArgs)
+ SpecifiedArgVec.push_back(Arg.getArgument());
- SmallVector<TemplateArgument, 4> ArgVec;
- ArgVec.reserve(Args.size());
- for (const TemplateArgumentLoc &Arg : Args)
- ArgVec.push_back(Arg.getArgument());
-
- return getTemplateSpecializationType(Template, ArgVec, Underlying);
+ return getTemplateSpecializationType(Template, SpecifiedArgVec, CanonicalArgs,
+ Underlying);
}
-#ifndef NDEBUG
-static bool hasAnyPackExpansions(ArrayRef<TemplateArgument> Args) {
+[[maybe_unused]] static bool
+hasAnyPackExpansions(ArrayRef<TemplateArgument> Args) {
for (const TemplateArgument &Arg : Args)
if (Arg.isPackExpansion())
return true;
-
- return true;
+ return false;
}
-#endif
-QualType
-ASTContext::getTemplateSpecializationType(TemplateName Template,
- ArrayRef<TemplateArgument> Args,
- QualType Underlying) const {
- assert(!Template.getAsDependentTemplateName() &&
- "No dependent template names here!");
+QualType ASTContext::getCanonicalTemplateSpecializationType(
+ TemplateName Template, ArrayRef<TemplateArgument> Args) const {
+ assert(Template ==
+ getCanonicalTemplateName(Template, /*IgnoreDeduced=*/true));
+ assert(!Args.empty());
+#ifndef NDEBUG
+ for (const auto &Arg : Args)
+ assert(Arg.structurallyEquals(getCanonicalTemplateArgument(Arg)));
+#endif
- const auto *TD = Template.getAsTemplateDecl(/*IgnoreDeduced=*/true);
- bool IsTypeAlias = TD && TD->isTypeAlias();
- QualType CanonType;
- if (!Underlying.isNull())
- CanonType = getCanonicalType(Underlying);
- else {
- // We can get here with an alias template when the specialization contains
- // a pack expansion that does not match up with a parameter pack.
- assert((!IsTypeAlias || hasAnyPackExpansions(Args)) &&
- "Caller must compute aliased type");
- IsTypeAlias = false;
- CanonType = getCanonicalTemplateSpecializationType(Template, Args);
- }
+ llvm::FoldingSetNodeID ID;
+ TemplateSpecializationType::Profile(ID, Template, Args, QualType(), *this);
+ void *InsertPos = nullptr;
+ if (auto *T = TemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos))
+ return QualType(T, 0);
- // Allocate the (non-canonical) template specialization type, but don't
- // try to unique it: these types typically have location information that
- // we don't unique and don't want to lose.
void *Mem = Allocate(sizeof(TemplateSpecializationType) +
- sizeof(TemplateArgument) * Args.size() +
- (IsTypeAlias ? sizeof(QualType) : 0),
+ sizeof(TemplateArgument) * Args.size(),
alignof(TemplateSpecializationType));
- auto *Spec
- = new (Mem) TemplateSpecializationType(Template, Args, CanonType,
- IsTypeAlias ? Underlying : QualType());
-
+ auto *Spec = new (Mem)
+ TemplateSpecializationType(Template, /*IsAlias=*/false, Args, QualType());
+ assert(Spec->isDependentType() &&
+ "canonical template specialization must be dependent");
Types.push_back(Spec);
+ TemplateSpecializationTypes.InsertNode(Spec, InsertPos);
return QualType(Spec, 0);
}
-QualType ASTContext::getCanonicalTemplateSpecializationType(
- TemplateName Template, ArrayRef<TemplateArgument> Args) const {
- assert(!Template.getAsDependentTemplateName() &&
+QualType ASTContext::getTemplateSpecializationType(
+ TemplateName Template, ArrayRef<TemplateArgument> SpecifiedArgs,
+ ArrayRef<TemplateArgument> CanonicalArgs, QualType Underlying) const {
+ assert(!Template.getUnderlying().getAsDependentTemplateName() &&
"No dependent template names here!");
- // Build the canonical template specialization type.
- // Any DeducedTemplateNames are ignored, because the effective name of a TST
- // accounts for the TST arguments laid over any default arguments contained in
- // its name.
- TemplateName CanonTemplate =
- getCanonicalTemplateName(Template, /*IgnoreDeduced=*/true);
-
- bool AnyNonCanonArgs = false;
- auto CanonArgs =
- ::getCanonicalTemplateArguments(*this, Args, AnyNonCanonArgs);
-
- // Determine whether this canonical template specialization type already
- // exists.
- llvm::FoldingSetNodeID ID;
- Template...
[truncated]
|
Adding lldb back to the ci script, as otherwise lldb cannot be tested locally. |
Note lldb-x86_64-debian and lldb-aarch64-windows did not fail because they used gcc and msvc to build lldb. But failed lldb tests required the latest clang (the cross build). I will test this patch on the environment similar to lldb-remote-linux-win. |
10f5545
to
2898180
Compare
I have tested the initial commit f8e6b60 The issue is still there
I will rebuild the debug version to get the useful stack trace. |
Thanks, yeah this relanding was just to try to get LLDB CI up and running, which I am still trying. |
c490e8d
to
5290f09
Compare
I tried the single test Here is the debug stack trace:
|
5290f09
to
2d4b74f
Compare
Thanks, that narrows it down a lot! I still wish I could test lldb locally. I will keep trying. |
Please note lldb uses the system's |
Thanks. You can see I made changes to our CI in this PR, and I managed to enable lldb, enable python testing and everything, but everything still passes in the lldb test suite. This is a linux system too, so it should be capable to reproduce this, right? Can you help get this CI up to reproducing this problem? |
2d4b74f
to
4f5c7ec
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
4f5c7ec
to
01b1863
Compare
Nevermind, I managed to do it :) |
105942e
to
2b33b96
Compare
935935a
to
338a836
Compare
… types This changes the TemplateArgument representation to hold a flag indicating whether a tempalte argument of expression type is supposed to be canonical or not. This gets one step closer to solving #92292 This still doesn't try to unique as-written TSTs. While this would increase the amount of memory savings and make code dealing with the AST more well-behaved, profiling template argument lists is still too expensive for this to be worthwhile, at least for now. This also fixes the context creation of TSTs, so that they don't in some cases get incorrectly flagged as sugar over their own canonical form. This is captured in the test expectation change of some AST dumps. This fixes some places which were unnecessarily canonicalizing these TSTs.
This makes sure UnaryTransformTypes are all uniqued, which makes the AST node more well behaved, and helps with aka suppression in diagnostics. This also makes sure a canonical UnaryTransformType round trips back to a canonical type when recreated from its components.
This makes sure a canonical DecltypeType round-trips back to a canonical type when rebuilt from its components.
338a836
to
d1a9483
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/10397 Here is the relevant piece of the build log for the reference
|
… types (llvm#135414) This relands llvm#135119, after fixing crashes seen in LLDB CI reported here: llvm#135119 (comment) Fixes llvm#135119 This changes the TemplateArgument representation to hold a flag indicating whether a tempalte argument of expression type is supposed to be canonical or not. This gets one step closer to solving llvm#92292 This still doesn't try to unique as-written TSTs. While this would increase the amount of memory savings and make code dealing with the AST more well-behaved, profiling template argument lists is still too expensive for this to be worthwhile, at least for now. This also fixes the context creation of TSTs, so that they don't in some cases get incorrectly flagged as sugar over their own canonical form. This is captured in the test expectation change of some AST dumps. This fixes some places which were unnecessarily canonicalizing these TSTs.
This relands #135119, after fixing crashes seen in LLDB CI reported here: #135119 (comment)
Fixes #135119
This changes the TemplateArgument representation to hold a flag indicating whether a tempalte argument of expression type is supposed to be canonical or not.
This gets one step closer to solving #92292
This still doesn't try to unique as-written TSTs. While this would increase the amount of memory savings and make code dealing with the AST more well-behaved, profiling template argument lists is still too expensive for this to be worthwhile, at least for now.
This also fixes the context creation of TSTs, so that they don't in some cases get incorrectly flagged as sugar over their own canonical form. This is captured in the test expectation change of some AST dumps.
This fixes some places which were unnecessarily canonicalizing these TSTs.