Skip to content

Commit dadb924

Browse files
committed
[clang] fix broken canonicalization of DeducedTemplateSpecializationType
This reverts the functional elements of commit 3e78fa8 and redoes it, by fixing the true root cause of #61317. A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST. The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules. The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either. --- No changelog entry because #61317 was already claimed as fixed in previous release.
1 parent 4748b49 commit dadb924

File tree

7 files changed

+109
-23
lines changed

7 files changed

+109
-23
lines changed

clang/include/clang/AST/ASTContext.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase<ASTContext> {
17711771
QualType DeducedType,
17721772
bool IsDependent) const;
17731773

1774+
private:
1775+
QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
1776+
QualType DeducedType,
1777+
bool IsDependent,
1778+
QualType Canon) const;
1779+
1780+
public:
17741781
/// Return the unique reference to the type for the specified TagDecl
17751782
/// (struct/union/class/enum) decl.
17761783
QualType getTagDeclType(const TagDecl *Decl) const;

clang/include/clang/AST/TemplateName.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,9 @@ class TemplateName {
346346
/// error.
347347
void dump() const;
348348

349-
void Profile(llvm::FoldingSetNodeID &ID);
349+
void Profile(llvm::FoldingSetNodeID &ID) {
350+
ID.AddPointer(Storage.getOpaqueValue());
351+
}
350352

351353
/// Retrieve the template name as a void pointer.
352354
void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }

clang/include/clang/AST/Type.h

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public DeducedType,
60506050

60516051
DeducedTemplateSpecializationType(TemplateName Template,
60526052
QualType DeducedAsType,
6053-
bool IsDeducedAsDependent)
6053+
bool IsDeducedAsDependent, QualType Canon)
60546054
: DeducedType(DeducedTemplateSpecialization, DeducedAsType,
60556055
toTypeDependence(Template.getDependence()) |
60566056
(IsDeducedAsDependent
60576057
? TypeDependence::DependentInstantiation
60586058
: TypeDependence::None),
6059-
DeducedAsType.isNull() ? QualType(this, 0)
6060-
: DeducedAsType.getCanonicalType()),
6059+
Canon),
60616060
Template(Template) {}
60626061

60636062
public:
60646063
/// Retrieve the name of the template that we are deducing.
60656064
TemplateName getTemplateName() const { return Template;}
60666065

6067-
void Profile(llvm::FoldingSetNodeID &ID) {
6066+
void Profile(llvm::FoldingSetNodeID &ID) const {
60686067
Profile(ID, getTemplateName(), getDeducedType(), isDependentType());
60696068
}
60706069

60716070
static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
60726071
QualType Deduced, bool IsDependent) {
60736072
Template.Profile(ID);
6074-
QualType CanonicalType =
6075-
Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
6076-
ID.AddPointer(CanonicalType.getAsOpaquePtr());
6073+
Deduced.Profile(ID);
60776074
ID.AddBoolean(IsDependent || Template.isDependent());
60786075
}
60796076

clang/lib/AST/ASTContext.cpp

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5925,11 +5925,9 @@ QualType ASTContext::getUnconstrainedType(QualType T) const {
59255925
return T;
59265926
}
59275927

5928-
/// Return the uniqued reference to the deduced template specialization type
5929-
/// which has been deduced to the given type, or to the canonical undeduced
5930-
/// such type, or the canonical deduced-but-dependent such type.
5931-
QualType ASTContext::getDeducedTemplateSpecializationType(
5932-
TemplateName Template, QualType DeducedType, bool IsDependent) const {
5928+
QualType ASTContext::getDeducedTemplateSpecializationTypeInternal(
5929+
TemplateName Template, QualType DeducedType, bool IsDependent,
5930+
QualType Canon) const {
59335931
// Look in the folding set for an existing type.
59345932
void *InsertPos = nullptr;
59355933
llvm::FoldingSetNodeID ID;
@@ -5940,7 +5938,8 @@ QualType ASTContext::getDeducedTemplateSpecializationType(
59405938
return QualType(DTST, 0);
59415939

59425940
auto *DTST = new (*this, alignof(DeducedTemplateSpecializationType))
5943-
DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
5941+
DeducedTemplateSpecializationType(Template, DeducedType, IsDependent,
5942+
Canon);
59445943
llvm::FoldingSetNodeID TempID;
59455944
DTST->Profile(TempID);
59465945
assert(ID == TempID && "ID does not match");
@@ -5949,6 +5948,20 @@ QualType ASTContext::getDeducedTemplateSpecializationType(
59495948
return QualType(DTST, 0);
59505949
}
59515950

5951+
/// Return the uniqued reference to the deduced template specialization type
5952+
/// which has been deduced to the given type, or to the canonical undeduced
5953+
/// such type, or the canonical deduced-but-dependent such type.
5954+
QualType ASTContext::getDeducedTemplateSpecializationType(
5955+
TemplateName Template, QualType DeducedType, bool IsDependent) const {
5956+
QualType Canon = DeducedType.isNull()
5957+
? getDeducedTemplateSpecializationTypeInternal(
5958+
getCanonicalTemplateName(Template), QualType(),
5959+
IsDependent, QualType())
5960+
: DeducedType.getCanonicalType();
5961+
return getDeducedTemplateSpecializationTypeInternal(Template, DeducedType,
5962+
IsDependent, Canon);
5963+
}
5964+
59525965
/// getAtomicType - Return the uniqued reference to the atomic type for
59535966
/// the given value type.
59545967
QualType ASTContext::getAtomicType(QualType T) const {

clang/lib/AST/TemplateName.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -264,15 +264,6 @@ bool TemplateName::containsUnexpandedParameterPack() const {
264264
return getDependence() & TemplateNameDependence::UnexpandedPack;
265265
}
266266

267-
void TemplateName::Profile(llvm::FoldingSetNodeID &ID) {
268-
if (const auto* USD = getAsUsingShadowDecl())
269-
ID.AddPointer(USD->getCanonicalDecl());
270-
else if (const auto *TD = getAsTemplateDecl())
271-
ID.AddPointer(TD->getCanonicalDecl());
272-
else
273-
ID.AddPointer(Storage.getOpaqueValue());
274-
}
275-
276267
void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
277268
Qualified Qual) const {
278269
auto handleAnonymousTTP = [](TemplateDecl *TD, raw_ostream &OS) {

clang/unittests/AST/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ add_clang_unittest(ASTTests
3131
EvaluateAsRValueTest.cpp
3232
ExternalASTSourceTest.cpp
3333
NamedDeclPrinterTest.cpp
34+
ProfilingTest.cpp
3435
RandstructTest.cpp
3536
RecursiveASTVisitorTest.cpp
3637
SizelessTypesTest.cpp

clang/unittests/AST/ProfilingTest.cpp

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//===- unittests/AST/ProfilingTest.cpp --- Tests for Profiling ------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "clang/AST/ASTContext.h"
10+
#include "clang/ASTMatchers/ASTMatchFinder.h"
11+
#include "clang/ASTMatchers/ASTMatchers.h"
12+
#include "clang/Tooling/Tooling.h"
13+
#include "gtest/gtest.h"
14+
#include <utility>
15+
16+
namespace clang {
17+
namespace {
18+
using namespace ast_matchers;
19+
20+
static auto getClassTemplateRedecls() {
21+
std::string Code = R"cpp(
22+
template <class> struct A;
23+
template <class> struct A;
24+
template <class> struct A;
25+
)cpp";
26+
auto AST = tooling::buildASTFromCode(Code);
27+
ASTContext &Ctx = AST->getASTContext();
28+
29+
auto MatchResults = match(classTemplateDecl().bind("id"), Ctx);
30+
SmallVector<ClassTemplateDecl *, 3> Res;
31+
for (BoundNodes &N : MatchResults) {
32+
if (auto *CTD = const_cast<ClassTemplateDecl *>(
33+
N.getNodeAs<ClassTemplateDecl>("id")))
34+
Res.push_back(CTD);
35+
}
36+
assert(Res.size() == 3);
37+
for (auto &&I : Res)
38+
assert(I->getCanonicalDecl() == Res[0]);
39+
return std::make_tuple(std::move(AST), Res[1], Res[2]);
40+
}
41+
42+
template <class T> static void testTypeNode(const T *T1, const T *T2) {
43+
{
44+
llvm::FoldingSetNodeID ID1, ID2;
45+
T1->Profile(ID1);
46+
T2->Profile(ID2);
47+
ASSERT_NE(ID1, ID2);
48+
}
49+
auto *CT1 =
50+
cast<DeducedTemplateSpecializationType>(T1->getCanonicalTypeInternal());
51+
auto *CT2 =
52+
cast<DeducedTemplateSpecializationType>(T2->getCanonicalTypeInternal());
53+
{
54+
llvm::FoldingSetNodeID ID1, ID2;
55+
CT1->Profile(ID1);
56+
CT2->Profile(ID2);
57+
ASSERT_EQ(ID1, ID2);
58+
}
59+
}
60+
61+
TEST(Profiling, DeducedTemplateSpecializationType_Name) {
62+
auto [AST, CTD1, CTD2] = getClassTemplateRedecls();
63+
ASTContext &Ctx = AST->getASTContext();
64+
65+
auto *T1 = cast<DeducedTemplateSpecializationType>(
66+
Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD1), QualType(),
67+
false));
68+
auto *T2 = cast<DeducedTemplateSpecializationType>(
69+
Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD2), QualType(),
70+
false));
71+
testTypeNode(T1, T2);
72+
}
73+
74+
} // namespace
75+
} // namespace clang

0 commit comments

Comments
 (0)