Skip to content

Commit

Permalink
[clang] fix broken canonicalization of DeducedTemplateSpecializationType
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mizvekov committed Jun 13, 2024
1 parent 4748b49 commit 8bd63f1
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 23 deletions.
7 changes: 7 additions & 0 deletions clang/include/clang/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase<ASTContext> {
QualType DeducedType,
bool IsDependent) const;

private:
QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
QualType DeducedType,
bool IsDependent,
QualType Canon) const;

public:
/// Return the unique reference to the type for the specified TagDecl
/// (struct/union/class/enum) decl.
QualType getTagDeclType(const TagDecl *Decl) const;
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/AST/TemplateName.h
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,9 @@ class TemplateName {
/// error.
void dump() const;

void Profile(llvm::FoldingSetNodeID &ID);
void Profile(llvm::FoldingSetNodeID &ID) {
ID.AddPointer(Storage.getOpaqueValue());
}

/// Retrieve the template name as a void pointer.
void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }
Expand Down
11 changes: 4 additions & 7 deletions clang/include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public DeducedType,

DeducedTemplateSpecializationType(TemplateName Template,
QualType DeducedAsType,
bool IsDeducedAsDependent)
bool IsDeducedAsDependent, QualType Canon)
: DeducedType(DeducedTemplateSpecialization, DeducedAsType,
toTypeDependence(Template.getDependence()) |
(IsDeducedAsDependent
? TypeDependence::DependentInstantiation
: TypeDependence::None),
DeducedAsType.isNull() ? QualType(this, 0)
: DeducedAsType.getCanonicalType()),
Canon),
Template(Template) {}

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

void Profile(llvm::FoldingSetNodeID &ID) {
void Profile(llvm::FoldingSetNodeID &ID) const {
Profile(ID, getTemplateName(), getDeducedType(), isDependentType());
}

static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
QualType Deduced, bool IsDependent) {
Template.Profile(ID);
QualType CanonicalType =
Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
ID.AddPointer(CanonicalType.getAsOpaquePtr());
Deduced.Profile(ID);
ID.AddBoolean(IsDependent || Template.isDependent());
}

Expand Down
25 changes: 19 additions & 6 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5925,11 +5925,9 @@ QualType ASTContext::getUnconstrainedType(QualType T) const {
return T;
}

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

auto *DTST = new (*this, alignof(DeducedTemplateSpecializationType))
DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
DeducedTemplateSpecializationType(Template, DeducedType, IsDependent,
Canon);
llvm::FoldingSetNodeID TempID;
DTST->Profile(TempID);
assert(ID == TempID && "ID does not match");
Expand All @@ -5949,6 +5948,20 @@ QualType ASTContext::getDeducedTemplateSpecializationType(
return QualType(DTST, 0);
}

/// Return the uniqued reference to the deduced template specialization type
/// which has been deduced to the given type, or to the canonical undeduced
/// such type, or the canonical deduced-but-dependent such type.
QualType ASTContext::getDeducedTemplateSpecializationType(
TemplateName Template, QualType DeducedType, bool IsDependent) const {
QualType Canon = DeducedType.isNull()
? getDeducedTemplateSpecializationTypeInternal(
getCanonicalTemplateName(Template), QualType(),
IsDependent, QualType())
: DeducedType.getCanonicalType();
return getDeducedTemplateSpecializationTypeInternal(Template, DeducedType,
IsDependent, Canon);
}

/// getAtomicType - Return the uniqued reference to the atomic type for
/// the given value type.
QualType ASTContext::getAtomicType(QualType T) const {
Expand Down
9 changes: 0 additions & 9 deletions clang/lib/AST/TemplateName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,15 +264,6 @@ bool TemplateName::containsUnexpandedParameterPack() const {
return getDependence() & TemplateNameDependence::UnexpandedPack;
}

void TemplateName::Profile(llvm::FoldingSetNodeID &ID) {
if (const auto* USD = getAsUsingShadowDecl())
ID.AddPointer(USD->getCanonicalDecl());
else if (const auto *TD = getAsTemplateDecl())
ID.AddPointer(TD->getCanonicalDecl());
else
ID.AddPointer(Storage.getOpaqueValue());
}

void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
Qualified Qual) const {
auto handleAnonymousTTP = [](TemplateDecl *TD, raw_ostream &OS) {
Expand Down
1 change: 1 addition & 0 deletions clang/unittests/AST/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ add_clang_unittest(ASTTests
EvaluateAsRValueTest.cpp
ExternalASTSourceTest.cpp
NamedDeclPrinterTest.cpp
ProfilingTest.cpp
RandstructTest.cpp
RecursiveASTVisitorTest.cpp
SizelessTypesTest.cpp
Expand Down
73 changes: 73 additions & 0 deletions clang/unittests/AST/ProfilingTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//===- unittests/AST/ProfilingTest.cpp --- Tests for Profiling ------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
#include <utility>

namespace clang {
namespace {
using namespace ast_matchers;

static auto getClassTemplateRedecls() {
std::string Code = R"cpp(
template <class> struct A;
template <class> struct A;
template <class> struct A;
)cpp";
auto AST = tooling::buildASTFromCode(Code);
ASTContext &Ctx = AST->getASTContext();

auto MatchResults = match(classTemplateDecl().bind("id"), Ctx);
SmallVector<ClassTemplateDecl *, 3> Res;
for (BoundNodes &N : MatchResults) {
if (auto *CTD = const_cast<ClassTemplateDecl *>(
N.getNodeAs<ClassTemplateDecl>("id")))
Res.push_back(CTD);
}
assert(Res.size() == 3);
for (auto &&I : Res)
assert(I->getCanonicalDecl() == Res[0]);
return std::make_tuple(std::move(AST), Res[1], Res[2]);
}

template <class T> static void testTypeNode(const T *T1, const T *T2) {
{
llvm::FoldingSetNodeID ID1, ID2;
T1->Profile(ID1);
T2->Profile(ID2);
ASSERT_NE(ID1, ID2);
}
auto *CT1 = cast<T>(T1->getCanonicalTypeInternal());
auto *CT2 = cast<T>(T2->getCanonicalTypeInternal());
{
llvm::FoldingSetNodeID ID1, ID2;
CT1->Profile(ID1);
CT2->Profile(ID2);
ASSERT_EQ(ID1, ID2);
}
}

TEST(Profiling, DeducedTemplateSpecializationType_Name) {
auto [AST, CTD1, CTD2] = getClassTemplateRedecls();
ASTContext &Ctx = AST->getASTContext();

auto *T1 = cast<DeducedTemplateSpecializationType>(
Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD1), QualType(),
false));
auto *T2 = cast<DeducedTemplateSpecializationType>(
Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD2), QualType(),
false));
testTypeNode(T1, T2);
}

} // namespace
} // namespace clang

0 comments on commit 8bd63f1

Please sign in to comment.