From 2e1ad93961a3f444659c5d02d800e3144acccdb4 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Wed, 12 Jun 2024 22:40:39 -0300 Subject: [PATCH] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (#95202) This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa 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. --- clang/include/clang/AST/ASTContext.h | 7 +++ clang/include/clang/AST/TemplateName.h | 4 +- clang/include/clang/AST/Type.h | 11 ++-- clang/lib/AST/ASTContext.cpp | 25 ++++++--- clang/lib/AST/TemplateName.cpp | 9 ---- clang/unittests/AST/CMakeLists.txt | 1 + clang/unittests/AST/ProfilingTest.cpp | 73 ++++++++++++++++++++++++++ 7 files changed, 107 insertions(+), 23 deletions(-) create mode 100644 clang/unittests/AST/ProfilingTest.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 8bce4812f0d482e..f1f20fca477a473 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase { 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; diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 988a55acd22525e..24a7fde76195dd0 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -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(); } diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 9eb3f6c09e3d3b1..fab233b62d8d1bd 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -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()); } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index cd76b8aa271dabf..aa22825602a40fb 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -5926,11 +5926,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; @@ -5941,7 +5939,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"); @@ -5950,6 +5949,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 { diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp index 11544dbb56e31d6..d4e8a8971a971a5 100644 --- a/clang/lib/AST/TemplateName.cpp +++ b/clang/lib/AST/TemplateName.cpp @@ -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) { diff --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt index 29d2b39cff8b158..dcc9bc0f39ac2c0 100644 --- a/clang/unittests/AST/CMakeLists.txt +++ b/clang/unittests/AST/CMakeLists.txt @@ -31,6 +31,7 @@ add_clang_unittest(ASTTests EvaluateAsRValueTest.cpp ExternalASTSourceTest.cpp NamedDeclPrinterTest.cpp + ProfilingTest.cpp RandstructTest.cpp RecursiveASTVisitorTest.cpp SizelessTypesTest.cpp diff --git a/clang/unittests/AST/ProfilingTest.cpp b/clang/unittests/AST/ProfilingTest.cpp new file mode 100644 index 000000000000000..ed81f4e1c5f2fb5 --- /dev/null +++ b/clang/unittests/AST/ProfilingTest.cpp @@ -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 + +namespace clang { +namespace { +using namespace ast_matchers; + +static auto getClassTemplateRedecls() { + std::string Code = R"cpp( + template struct A; + template struct A; + template struct A; + )cpp"; + auto AST = tooling::buildASTFromCode(Code); + ASTContext &Ctx = AST->getASTContext(); + + auto MatchResults = match(classTemplateDecl().bind("id"), Ctx); + SmallVector Res; + for (BoundNodes &N : MatchResults) { + if (auto *CTD = const_cast( + N.getNodeAs("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 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(T1->getCanonicalTypeInternal()); + auto *CT2 = cast(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( + Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD1), QualType(), + false)); + auto *T2 = cast( + Ctx.getDeducedTemplateSpecializationType(TemplateName(CTD2), QualType(), + false)); + testTypeNode(T1, T2); +} + +} // namespace +} // namespace clang