Skip to content

Commit 2e1ad93

Browse files
authored
[clang] fix broken canonicalization of DeducedTemplateSpecializationType (#95202)
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 b06da39 commit 2e1ad93

File tree

7 files changed

+107
-23
lines changed

7 files changed

+107
-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
@@ -5926,11 +5926,9 @@ QualType ASTContext::getUnconstrainedType(QualType T) const {
59265926
return T;
59275927
}
59285928

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

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

5952+
/// Return the uniqued reference to the deduced template specialization type
5953+
/// which has been deduced to the given type, or to the canonical undeduced
5954+
/// such type, or the canonical deduced-but-dependent such type.
5955+
QualType ASTContext::getDeducedTemplateSpecializationType(
5956+
TemplateName Template, QualType DeducedType, bool IsDependent) const {
5957+
QualType Canon = DeducedType.isNull()
5958+
? getDeducedTemplateSpecializationTypeInternal(
5959+
getCanonicalTemplateName(Template), QualType(),
5960+
IsDependent, QualType())
5961+
: DeducedType.getCanonicalType();
5962+
return getDeducedTemplateSpecializationTypeInternal(Template, DeducedType,
5963+
IsDependent, Canon);
5964+
}
5965+
59535966
/// getAtomicType - Return the uniqued reference to the atomic type for
59545967
/// the given value type.
59555968
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: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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 = cast<T>(T1->getCanonicalTypeInternal());
50+
auto *CT2 = cast<T>(T2->getCanonicalTypeInternal());
51+
{
52+
llvm::FoldingSetNodeID ID1, ID2;
53+
CT1->Profile(ID1);
54+
CT2->Profile(ID2);
55+
ASSERT_EQ(ID1, ID2);
56+
}
57+
}
58+
59+
TEST(Profiling, DeducedTemplateSpecializationType_Name) {
60+
auto [AST, CTD1, CTD2] = getClassTemplateRedecls();
61+
ASTContext &Ctx = AST->getASTContext();
62+
63+
auto *T1 = cast<DeducedTemplateSpecializationType>(
64+
Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD1), QualType(),
65+
false));
66+
auto *T2 = cast<DeducedTemplateSpecializationType>(
67+
Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD2), QualType(),
68+
false));
69+
testTypeNode(T1, T2);
70+
}
71+
72+
} // namespace
73+
} // namespace clang

0 commit comments

Comments
 (0)