Skip to content

Commit 91cdd35

Browse files
mizvekovmizvekov
authored
[clang] Improve nested name specifier AST representation (#147835)
This is a major change on how we represent nested name qualifications in the AST. * The nested name specifier itself and how it's stored is changed. The prefixes for types are handled within the type hierarchy, which makes canonicalization for them super cheap, no memory allocation required. Also translating a type into nested name specifier form becomes a no-op. An identifier is stored as a DependentNameType. The nested name specifier gains a lightweight handle class, to be used instead of passing around pointers, which is similar to what is implemented for TemplateName. There is still one free bit available, and this handle can be used within a PointerUnion and PointerIntPair, which should keep bit-packing aficionados happy. * The ElaboratedType node is removed, all type nodes in which it could previously apply to can now store the elaborated keyword and name qualifier, tail allocating when present. * TagTypes can now point to the exact declaration found when producing these, as opposed to the previous situation of there only existing one TagType per entity. This increases the amount of type sugar retained, and can have several applications, for example in tracking module ownership, and other tools which care about source file origins, such as IWYU. These TagTypes are lazily allocated, in order to limit the increase in AST size. This patch offers a great performance benefit. It greatly improves compilation time for [stdexec](https://github.com/NVIDIA/stdexec). For one datapoint, for `test_on2.cpp` in that project, which is the slowest compiling test, this patch improves `-c` compilation time by about 7.2%, with the `-fsyntax-only` improvement being at ~12%. This has great results on compile-time-tracker as well: ![image](https://github.com/user-attachments/assets/700dce98-2cab-4aa8-97d1-b038c0bee831) This patch also further enables other optimziations in the future, and will reduce the performance impact of template specialization resugaring when that lands. It has some other miscelaneous drive-by fixes. About the review: Yes the patch is huge, sorry about that. Part of the reason is that I started by the nested name specifier part, before the ElaboratedType part, but that had a huge performance downside, as ElaboratedType is a big performance hog. I didn't have the steam to go back and change the patch after the fact. There is also a lot of internal API changes, and it made sense to remove ElaboratedType in one go, versus removing it from one type at a time, as that would present much more churn to the users. Also, the nested name specifier having a different API avoids missing changes related to how prefixes work now, which could make existing code compile but not work. How to review: The important changes are all in `clang/include/clang/AST` and `clang/lib/AST`, with also important changes in `clang/lib/Sema/TreeTransform.h`. The rest and bulk of the changes are mostly consequences of the changes in API. PS: TagType::getDecl is renamed to `getOriginalDecl` in this patch, just for easier to rebasing. I plan to rename it back after this lands. Fixes #136624 Fixes #43179 Fixes #68670 Fixes #92757
1 parent fc44a4f commit 91cdd35

File tree

539 files changed

+11195
-10586
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

539 files changed

+11195
-10586
lines changed

‎clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp‎

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,24 +31,9 @@ llvm::SmallVector<llvm::StringRef, 4> splitSymbolName(llvm::StringRef Name) {
3131
return Splitted;
3232
}
3333

34-
SourceLocation startLocationForType(TypeLoc TLoc) {
35-
// For elaborated types (e.g. `struct a::A`) we want the portion after the
36-
// `struct` but including the namespace qualifier, `a::`.
37-
if (TLoc.getTypeLocClass() == TypeLoc::Elaborated) {
38-
NestedNameSpecifierLoc NestedNameSpecifier =
39-
TLoc.castAs<ElaboratedTypeLoc>().getQualifierLoc();
40-
if (NestedNameSpecifier.getNestedNameSpecifier())
41-
return NestedNameSpecifier.getBeginLoc();
42-
TLoc = TLoc.getNextTypeLoc();
43-
}
44-
return TLoc.getBeginLoc();
45-
}
46-
4734
SourceLocation endLocationForType(TypeLoc TLoc) {
48-
// Dig past any namespace or keyword qualifications.
49-
while (TLoc.getTypeLocClass() == TypeLoc::Elaborated ||
50-
TLoc.getTypeLocClass() == TypeLoc::Qualified)
51-
TLoc = TLoc.getNextTypeLoc();
35+
if (auto QTL = TLoc.getAs<QualifiedTypeLoc>())
36+
TLoc = QTL.getUnqualifiedLoc();
5237

5338
// The location for template specializations (e.g. Foo<int>) includes the
5439
// templated types in its location range. We want to restrict this to just
@@ -550,8 +535,8 @@ void ChangeNamespaceTool::run(
550535
Result.Nodes.getNodeAs<NestedNameSpecifierLoc>(
551536
"nested_specifier_loc")) {
552537
SourceLocation Start = Specifier->getBeginLoc();
553-
SourceLocation End = endLocationForType(Specifier->getTypeLoc());
554-
fixTypeLoc(Result, Start, End, Specifier->getTypeLoc());
538+
SourceLocation End = endLocationForType(Specifier->castAsTypeLoc());
539+
fixTypeLoc(Result, Start, End, Specifier->castAsTypeLoc());
555540
} else if (const auto *BaseInitializer =
556541
Result.Nodes.getNodeAs<CXXCtorInitializer>(
557542
"base_initializer")) {
@@ -562,19 +547,16 @@ void ChangeNamespaceTool::run(
562547
// filtered by matchers in some cases, e.g. the type is templated. We should
563548
// handle the record type qualifier instead.
564549
TypeLoc Loc = *TLoc;
565-
while (Loc.getTypeLocClass() == TypeLoc::Qualified)
566-
Loc = Loc.getNextTypeLoc();
567-
if (Loc.getTypeLocClass() == TypeLoc::Elaborated) {
568-
NestedNameSpecifierLoc NestedNameSpecifier =
569-
Loc.castAs<ElaboratedTypeLoc>().getQualifierLoc();
570-
// FIXME: avoid changing injected class names.
571-
if (auto *NNS = NestedNameSpecifier.getNestedNameSpecifier()) {
572-
const Type *SpecifierType = NNS->getAsType();
573-
if (SpecifierType && SpecifierType->isRecordType())
574-
return;
575-
}
576-
}
577-
fixTypeLoc(Result, startLocationForType(Loc), endLocationForType(Loc), Loc);
550+
if (auto QTL = Loc.getAs<QualifiedTypeLoc>())
551+
Loc = QTL.getUnqualifiedLoc();
552+
// FIXME: avoid changing injected class names.
553+
if (NestedNameSpecifier NestedNameSpecifier =
554+
Loc.getPrefix().getNestedNameSpecifier();
555+
NestedNameSpecifier.getKind() == NestedNameSpecifier::Kind::Type &&
556+
NestedNameSpecifier.getAsType()->isRecordType())
557+
return;
558+
fixTypeLoc(Result, Loc.getNonElaboratedBeginLoc(), endLocationForType(Loc),
559+
Loc);
578560
} else if (const auto *VarRef =
579561
Result.Nodes.getNodeAs<DeclRefExpr>("var_ref")) {
580562
const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var_decl");
@@ -588,10 +570,9 @@ void ChangeNamespaceTool::run(
588570
} else if (const auto *EnumConstRef =
589571
Result.Nodes.getNodeAs<DeclRefExpr>("enum_const_ref")) {
590572
// Do not rename the reference if it is already scoped by the EnumDecl name.
591-
if (EnumConstRef->hasQualifier() &&
592-
EnumConstRef->getQualifier()->getKind() ==
593-
NestedNameSpecifier::SpecifierKind::TypeSpec &&
594-
EnumConstRef->getQualifier()->getAsType()->isEnumeralType())
573+
if (NestedNameSpecifier Qualifier = EnumConstRef->getQualifier();
574+
Qualifier.getKind() == NestedNameSpecifier::Kind::Type &&
575+
Qualifier.getAsType()->isEnumeralType())
595576
return;
596577
const auto *EnumConstDecl =
597578
Result.Nodes.getNodeAs<EnumConstantDecl>("enum_const_decl");

‎clang-tools-extra/clang-doc/Serialize.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -902,8 +902,8 @@ parseBases(RecordInfo &I, const CXXRecordDecl *D, bool IsFileInRootDir,
902902
return;
903903
for (const CXXBaseSpecifier &B : D->bases()) {
904904
if (const RecordType *Ty = B.getType()->getAs<RecordType>()) {
905-
if (const CXXRecordDecl *Base =
906-
cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition())) {
905+
if (const CXXRecordDecl *Base = cast_or_null<CXXRecordDecl>(
906+
Ty->getOriginalDecl()->getDefinition())) {
907907
// Initialized without USR and name, this will be set in the following
908908
// if-else stmt.
909909
BaseRecordInfo BI(

‎clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,7 @@ void FindAllSymbols::registerMatchers(MatchFinder *MatchFinder) {
216216
// Uses of most types: just look at what the typeLoc refers to.
217217
MatchFinder->addMatcher(
218218
typeLoc(isExpansionInMainFile(),
219-
loc(qualType(allOf(unless(elaboratedType()),
220-
hasDeclaration(Types.bind("use")))))),
219+
loc(qualType(hasDeclaration(Types.bind("use"))))),
221220
this);
222221
// Uses of typedefs: these are often transparent to hasDeclaration, so we need
223222
// to handle them explicitly.

‎clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,8 @@ void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) {
533533
Builder << reinterpret_cast<const NamedDecl *>(Info.getRawArg(Index));
534534
break;
535535
case clang::DiagnosticsEngine::ak_nestednamespec:
536-
Builder << reinterpret_cast<NestedNameSpecifier *>(Info.getRawArg(Index));
536+
Builder << NestedNameSpecifier::getFromVoidPointer(
537+
reinterpret_cast<void *>(Info.getRawArg(Index)));
537538
break;
538539
case clang::DiagnosticsEngine::ak_declcontext:
539540
Builder << reinterpret_cast<DeclContext *>(Info.getRawArg(Index));

‎clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ static bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
4343
return false;
4444
}
4545

46-
return FriendType->getType()->getAsCXXRecordDecl() == Derived;
46+
return declaresSameEntity(FriendType->getType()->getAsCXXRecordDecl(),
47+
Derived);
4748
});
4849
}
4950

@@ -55,7 +56,8 @@ getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
5556
CRTP->getTemplateArgs().asArray(), [&](const TemplateArgument &Arg) {
5657
++Idx;
5758
return Arg.getKind() == TemplateArgument::Type &&
58-
Arg.getAsType()->getAsCXXRecordDecl() == Derived;
59+
declaresSameEntity(Arg.getAsType()->getAsCXXRecordDecl(),
60+
Derived);
5961
});
6062

6163
return AnyOf ? CRTP->getSpecializedTemplate()

‎clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ approximateImplicitConversion(const TheCheck &Check, QualType LType,
577577
ImplicitConversionModellingMode ImplicitMode);
578578

579579
static inline bool isUselessSugar(const Type *T) {
580-
return isa<AttributedType, DecayedType, ElaboratedType, ParenType>(T);
580+
return isa<AttributedType, DecayedType, ParenType>(T);
581581
}
582582

583583
namespace {
@@ -1040,7 +1040,9 @@ approximateStandardConversionSequence(const TheCheck &Check, QualType From,
10401040
const auto *ToRecord = To->getAsCXXRecordDecl();
10411041
if (isDerivedToBase(FromRecord, ToRecord)) {
10421042
LLVM_DEBUG(llvm::dbgs() << "--- approximateStdConv. Derived To Base.\n");
1043-
WorkType = QualType{ToRecord->getTypeForDecl(), FastQualifiersToApply};
1043+
WorkType = QualType{
1044+
ToRecord->getASTContext().getCanonicalTagType(ToRecord)->getTypePtr(),
1045+
FastQualifiersToApply};
10441046
}
10451047

10461048
if (Ctx.getLangOpts().CPlusPlus17 && FromPtr && ToPtr) {
@@ -1072,9 +1074,9 @@ approximateStandardConversionSequence(const TheCheck &Check, QualType From,
10721074
WorkType = To;
10731075
}
10741076

1075-
if (WorkType == To) {
1077+
if (Ctx.hasSameType(WorkType, To)) {
10761078
LLVM_DEBUG(llvm::dbgs() << "<<< approximateStdConv. Reached 'To' type.\n");
1077-
return {WorkType};
1079+
return {Ctx.getCommonSugaredType(WorkType, To)};
10781080
}
10791081

10801082
LLVM_DEBUG(llvm::dbgs() << "<<< approximateStdConv. Did not reach 'To'.\n");
@@ -1219,7 +1221,7 @@ tryConversionOperators(const TheCheck &Check, const CXXRecordDecl *RD,
12191221

12201222
if (std::optional<UserDefinedConversionSelector::PreparedConversion>
12211223
SelectedConversion = ConversionSet()) {
1222-
QualType RecordType{RD->getTypeForDecl(), 0};
1224+
CanQualType RecordType = RD->getASTContext().getCanonicalTagType(RD);
12231225

12241226
ConversionSequence Result{RecordType, ToType};
12251227
// The conversion from the operator call's return type to ToType was
@@ -1270,7 +1272,7 @@ tryConvertingConstructors(const TheCheck &Check, QualType FromType,
12701272

12711273
if (std::optional<UserDefinedConversionSelector::PreparedConversion>
12721274
SelectedConversion = ConversionSet()) {
1273-
QualType RecordType{RD->getTypeForDecl(), 0};
1275+
CanQualType RecordType = RD->getASTContext().getCanonicalTagType(RD);
12741276

12751277
ConversionSequence Result{FromType, RecordType};
12761278
Result.AfterFirstStandard = SelectedConversion->Seq.AfterFirstStandard;

‎clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,9 @@ void ForwardDeclarationNamespaceCheck::check(
6969
// struct B { friend A; };
7070
// \endcode
7171
// `A` will not be marked as "referenced" in the AST.
72-
if (const TypeSourceInfo *Tsi = Decl->getFriendType()) {
73-
QualType Desugared = Tsi->getType().getDesugaredType(*Result.Context);
74-
FriendTypes.insert(Desugared.getTypePtr());
75-
}
72+
if (const TypeSourceInfo *Tsi = Decl->getFriendType())
73+
FriendTypes.insert(
74+
Tsi->getType()->getCanonicalTypeUnqualified().getTypePtr());
7675
}
7776
}
7877

@@ -119,7 +118,9 @@ void ForwardDeclarationNamespaceCheck::onEndOfTranslationUnit() {
119118
if (CurDecl->hasDefinition() || CurDecl->isReferenced()) {
120119
continue; // Skip forward declarations that are used/referenced.
121120
}
122-
if (FriendTypes.contains(CurDecl->getTypeForDecl())) {
121+
if (FriendTypes.contains(CurDecl->getASTContext()
122+
.getCanonicalTagType(CurDecl)
123+
->getTypePtr())) {
123124
continue; // Skip forward declarations referenced as friend.
124125
}
125126
if (CurDecl->getLocation().isMacroID() ||

‎clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,17 @@ AST_MATCHER(QualType, isEnableIf) {
3333
BaseType = BaseType->getPointeeType().getTypePtr();
3434
}
3535
// Case: type parameter dependent (enable_if<is_integral<T>>).
36-
if (const auto *Dependent = BaseType->getAs<DependentNameType>()) {
37-
BaseType = Dependent->getQualifier()->getAsType();
38-
}
36+
if (const auto *Dependent = BaseType->getAs<DependentNameType>())
37+
BaseType = Dependent->getQualifier().getAsType();
3938
if (!BaseType)
4039
return false;
4140
if (CheckTemplate(BaseType->getAs<TemplateSpecializationType>()))
4241
return true; // Case: enable_if_t< >.
43-
if (const auto *Elaborated = BaseType->getAs<ElaboratedType>()) {
44-
if (const auto *Q = Elaborated->getQualifier())
45-
if (const auto *Qualifier = Q->getAsType()) {
46-
if (CheckTemplate(Qualifier->getAs<TemplateSpecializationType>())) {
47-
return true; // Case: enable_if< >::type.
48-
}
49-
}
50-
}
42+
if (const auto *TT = BaseType->getAs<TypedefType>())
43+
if (NestedNameSpecifier Q = TT->getQualifier();
44+
Q.getKind() == NestedNameSpecifier::Kind::Type)
45+
if (CheckTemplate(Q.getAsType()->getAs<TemplateSpecializationType>()))
46+
return true; // Case: enable_if< >::type.
5147
return false;
5248
}
5349
AST_MATCHER_P(TemplateTypeParmDecl, hasDefaultArgument,

‎clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,27 +32,22 @@ AST_MATCHER_P(TemplateTypeParmDecl, hasUnnamedDefaultArgument,
3232
void IncorrectEnableIfCheck::registerMatchers(MatchFinder *Finder) {
3333
Finder->addMatcher(
3434
templateTypeParmDecl(
35-
hasUnnamedDefaultArgument(
36-
elaboratedTypeLoc(
37-
hasNamedTypeLoc(templateSpecializationTypeLoc(
38-
loc(qualType(hasDeclaration(namedDecl(
39-
hasName("::std::enable_if"))))))
40-
.bind("enable_if_specialization")))
41-
.bind("elaborated")))
35+
hasUnnamedDefaultArgument(templateSpecializationTypeLoc(
36+
loc(qualType(hasDeclaration(namedDecl(
37+
hasName("::std::enable_if"))))))
38+
.bind("enable_if_specialization")))
4239
.bind("enable_if"),
4340
this);
4441
}
4542

4643
void IncorrectEnableIfCheck::check(const MatchFinder::MatchResult &Result) {
4744
const auto *EnableIf =
4845
Result.Nodes.getNodeAs<TemplateTypeParmDecl>("enable_if");
49-
const auto *ElaboratedLoc =
50-
Result.Nodes.getNodeAs<ElaboratedTypeLoc>("elaborated");
5146
const auto *EnableIfSpecializationLoc =
5247
Result.Nodes.getNodeAs<TemplateSpecializationTypeLoc>(
5348
"enable_if_specialization");
5449

55-
if (!EnableIf || !ElaboratedLoc || !EnableIfSpecializationLoc)
50+
if (!EnableIf || !EnableIfSpecializationLoc)
5651
return;
5752

5853
const SourceManager &SM = *Result.SourceManager;
@@ -62,8 +57,10 @@ void IncorrectEnableIfCheck::check(const MatchFinder::MatchResult &Result) {
6257
auto Diag = diag(EnableIf->getBeginLoc(),
6358
"incorrect std::enable_if usage detected; use "
6459
"'typename std::enable_if<...>::type'");
60+
// FIXME: This should handle the enable_if specialization already having an
61+
// elaborated keyword.
6562
if (!getLangOpts().CPlusPlus20) {
66-
Diag << FixItHint::CreateInsertion(ElaboratedLoc->getBeginLoc(),
63+
Diag << FixItHint::CreateInsertion(EnableIfSpecializationLoc->getBeginLoc(),
6764
"typename ");
6865
}
6966
Diag << FixItHint::CreateInsertion(RAngleLoc.getLocWithOffset(1), "::type");

‎clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,15 @@ class FindEnumMember : public TypeVisitor<FindEnumMember, bool> {
6767
return Visit(T->getElementType().getTypePtr());
6868
}
6969
bool VisitEnumType(const EnumType *T) {
70-
if (isCompleteAndHasNoZeroValue(T->getDecl())) {
70+
if (isCompleteAndHasNoZeroValue(T->getOriginalDecl())) {
7171
FoundEnum = T;
7272
return true;
7373
}
7474
return false;
7575
}
7676
bool VisitRecordType(const RecordType *T) {
77-
const RecordDecl *RD = T->getDecl();
78-
if (RD->isUnion())
77+
const RecordDecl *RD = T->getOriginalDecl()->getDefinition();
78+
if (!RD || RD->isUnion())
7979
return false;
8080
auto VisitField = [this](const FieldDecl *F) {
8181
return Visit(F->getType().getTypePtr());
@@ -125,7 +125,7 @@ void InvalidEnumDefaultInitializationCheck::check(
125125
if (!Finder.Visit(InitList->getArrayFiller()->getType().getTypePtr()))
126126
return;
127127
InitExpr = InitList;
128-
Enum = Finder.FoundEnum->getDecl();
128+
Enum = Finder.FoundEnum->getOriginalDecl();
129129
}
130130

131131
if (!InitExpr || !Enum)

0 commit comments

Comments
 (0)