[clang] [clang][AST] Add 'IgnoreTemplateParmDepth' to structural equivalence cache (PR #115518)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 8 09:05:56 PST 2024
https://github.com/balazske created https://github.com/llvm/llvm-project/pull/115518
Structural equivalence check uses a cache to store already found non-equivalent values. This cache can be reused for calls (ASTImporter does this). Value of "IgnoreTemplateParmDepth" can have an effect on the structural equivalence therefore it is wrong to reuse the same cache for checks with different values of 'IgnoreTemplateParmDepth'. The current change adds the 'IgnoreTemplateParmDepth' to the cache key to fix the problem.
>From cd92fe901e9a5668686a152586780d67f6e5d672 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Fri, 8 Nov 2024 17:52:18 +0100
Subject: [PATCH] [clang][AST] Add 'IgnoreTemplateParmDepth' to structural
equivalence cache
Structural equivalence check uses a cache to store already found non-equivalent values.
This cache can be reused for calls (ASTImporter does this).
Value of "IgnoreTemplateParmDepth" can have an effect on the structural equivalence
therefore it is wrong to reuse the same cache for checks with different values of
'IgnoreTemplateParmDepth'. The current change adds the 'IgnoreTemplateParmDepth'
to the cache key to fix the problem.
---
clang/include/clang/AST/ASTImporter.h | 3 +-
.../clang/AST/ASTStructuralEquivalence.h | 19 ++--
clang/lib/AST/ASTStructuralEquivalence.cpp | 6 +-
clang/lib/Sema/SemaType.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 2 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
.../AST/StructuralEquivalenceTest.cpp | 88 +++++++++++++++++--
7 files changed, 102 insertions(+), 20 deletions(-)
diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h
index 088a2bd0fdd407..8c3fa842ab8b9d 100644
--- a/clang/include/clang/AST/ASTImporter.h
+++ b/clang/include/clang/AST/ASTImporter.h
@@ -62,7 +62,8 @@ class TypeSourceInfo;
class ASTImporter {
friend class ASTNodeImporter;
public:
- using NonEquivalentDeclSet = llvm::DenseSet<std::pair<Decl *, Decl *>>;
+ using NonEquivalentDeclSet =
+ llvm::DenseSet<std::tuple<Decl *, Decl *, int>>;
using ImportedCXXBaseSpecifierMap =
llvm::DenseMap<const CXXBaseSpecifier *, CXXBaseSpecifier *>;
diff --git a/clang/include/clang/AST/ASTStructuralEquivalence.h b/clang/include/clang/AST/ASTStructuralEquivalence.h
index 029439c8e9a3ac..67aa0023c25d08 100644
--- a/clang/include/clang/AST/ASTStructuralEquivalence.h
+++ b/clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -39,6 +39,10 @@ enum class StructuralEquivalenceKind {
};
struct StructuralEquivalenceContext {
+ /// Store declaration pairs already found to be non-equivalent.
+ /// key: (from, to, IgnoreTemplateParmDepth)
+ using NonEquivalentDeclSet = llvm::DenseSet<std::tuple<Decl *, Decl *, int>>;
+
/// AST contexts for which we are checking structural equivalence.
ASTContext &FromCtx, &ToCtx;
@@ -52,7 +56,7 @@ struct StructuralEquivalenceContext {
/// Declaration (from, to) pairs that are known not to be equivalent
/// (which we have already complained about).
- llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls;
+ NonEquivalentDeclSet &NonEquivalentDecls;
StructuralEquivalenceKind EqKind;
@@ -72,12 +76,13 @@ struct StructuralEquivalenceContext {
/// Whether to ignore comparing the depth of template param(TemplateTypeParm)
bool IgnoreTemplateParmDepth;
- StructuralEquivalenceContext(
- ASTContext &FromCtx, ASTContext &ToCtx,
- llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls,
- StructuralEquivalenceKind EqKind, bool StrictTypeSpelling = false,
- bool Complain = true, bool ErrorOnTagTypeMismatch = false,
- bool IgnoreTemplateParmDepth = false)
+ StructuralEquivalenceContext(ASTContext &FromCtx, ASTContext &ToCtx,
+ NonEquivalentDeclSet &NonEquivalentDecls,
+ StructuralEquivalenceKind EqKind,
+ bool StrictTypeSpelling = false,
+ bool Complain = true,
+ bool ErrorOnTagTypeMismatch = false,
+ bool IgnoreTemplateParmDepth = false)
: FromCtx(FromCtx), ToCtx(ToCtx), NonEquivalentDecls(NonEquivalentDecls),
EqKind(EqKind), StrictTypeSpelling(StrictTypeSpelling),
ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch), Complain(Complain),
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 120ddc0f26c0d7..bf2f42932f25d4 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -2303,7 +2303,8 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
// Check whether we already know that these two declarations are not
// structurally equivalent.
- if (Context.NonEquivalentDecls.count(P))
+ if (Context.NonEquivalentDecls.count(
+ std::make_tuple(D1, D2, Context.IgnoreTemplateParmDepth)))
return false;
// Check if a check for these declarations is already pending.
@@ -2511,7 +2512,8 @@ bool StructuralEquivalenceContext::Finish() {
if (!Equivalent) {
// Note that these two declarations are not equivalent (and we already
// know about it).
- NonEquivalentDecls.insert(P);
+ NonEquivalentDecls.insert(
+ std::make_tuple(D1, D2, IgnoreTemplateParmDepth));
return true;
}
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index e526a11973975d..e26d422a31b49e 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -9043,7 +9043,7 @@ bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
}
bool Sema::hasStructuralCompatLayout(Decl *D, Decl *Suggested) {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls;
if (!Suggested)
return false;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 79615dc3c018ea..649bc318fa14c4 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9905,7 +9905,7 @@ void ASTReader::finishPendingActions() {
auto ExtensionsPair = PendingObjCExtensionIvarRedeclarations.back().first;
auto DuplicateIvars =
PendingObjCExtensionIvarRedeclarations.back().second;
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls;
StructuralEquivalenceContext Ctx(
ExtensionsPair.first->getASTContext(),
ExtensionsPair.second->getASTContext(), NonEquivalentDecls,
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 33fcddbbdb2f15..6ece3ba7af9f4b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4447,7 +4447,7 @@ namespace {
ObjCCategoryDecl *&Existing = NameCategoryMap[Cat->getDeclName()];
if (Existing && Reader.getOwningModuleFile(Existing) !=
Reader.getOwningModuleFile(Cat)) {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls;
StructuralEquivalenceContext Ctx(
Cat->getASTContext(), Existing->getASTContext(),
NonEquivalentDecls, StructuralEquivalenceKind::Default,
diff --git a/clang/unittests/AST/StructuralEquivalenceTest.cpp b/clang/unittests/AST/StructuralEquivalenceTest.cpp
index e994086c99d041..7cf52df9b14d05 100644
--- a/clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ b/clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -134,8 +134,8 @@ struct StructuralEquivalenceTest : ::testing::Test {
bool testStructuralMatch(Decl *D0, Decl *D1,
bool IgnoreTemplateParmDepth = false) {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls01;
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls10;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls01;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls10;
StructuralEquivalenceContext Ctx01(
D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls01,
StructuralEquivalenceKind::Default, /*StrictTypeSpelling=*/false,
@@ -153,8 +153,8 @@ struct StructuralEquivalenceTest : ::testing::Test {
}
bool testStructuralMatch(StmtWithASTContext S0, StmtWithASTContext S1) {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls01;
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls10;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls01;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls10;
StructuralEquivalenceContext Ctx01(
*S0.Context, *S1.Context, NonEquivalentDecls01,
StructuralEquivalenceKind::Default, false, false);
@@ -1792,7 +1792,7 @@ TEST_F(
EXPECT_FALSE(testStructuralMatch(t));
}
struct StructuralEquivalenceCacheTest : public StructuralEquivalenceTest {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls;
template <typename NodeType, typename MatcherType>
std::pair<NodeType *, NodeType *>
@@ -1804,8 +1804,10 @@ struct StructuralEquivalenceCacheTest : public StructuralEquivalenceTest {
}
template <typename NodeType>
- bool isInNonEqCache(std::pair<NodeType *, NodeType *> D) {
- return NonEquivalentDecls.count(D) > 0;
+ bool isInNonEqCache(std::pair<NodeType *, NodeType *> D,
+ bool IgnoreTemplateParmDepth = false) {
+ return NonEquivalentDecls.count(
+ std::make_tuple(D.first, D.second, IgnoreTemplateParmDepth)) > 0;
}
};
@@ -2015,6 +2017,78 @@ TEST_F(StructuralEquivalenceCacheTest, Cycle) {
findDeclPair<FunctionDecl>(TU, functionDecl(hasName("x")))));
}
+TEST_F(StructuralEquivalenceCacheTest, TemplateParmDepth) {
+ // In 'friend struct Y' ClassTemplateDecl has the TU as parent context.
+ // This declaration has template depth 1 (it is already inside a template).
+ // It has not a previous declaration and is an "undeclared" friend.
+ //
+ // Second TU has a specialization of 'struct X'.
+ // In this case 'friend struct Y' has the ClassTemplateSpecializationDecl as
+ // parent. It has template depth 0 (it is in the specialization). It has the
+ // first 'struct Y' declaration as previous declaration and canonical
+ // declaration.
+ //
+ // When these two 'friend struct Y' are compared, only the template depth is
+ // different.
+ // FIXME: Structural equivalence checks the depth only in types, not in
+ // TemplateParmDecl. For this reason the second 'A1' argument is needed (as a
+ // type) in the template to make the check fail.
+ auto TU = makeTuDecls(
+ R"(
+ template <class A1, A1>
+ struct Y;
+
+ template <class A>
+ struct X {
+ template <class A1, A1>
+ friend struct Y;
+ };
+ )",
+ R"(
+ template <class A1, A1>
+ struct Y;
+
+ template <class A>
+ struct X {
+ template <class A1, A1>
+ friend struct Y;
+ };
+
+ X<int> x;
+ )",
+ Lang_CXX03);
+
+ auto *D0 = LastDeclMatcher<ClassTemplateDecl>().match(
+ get<0>(TU), classTemplateDecl(hasName("Y"), unless(isImplicit())));
+ auto *D1 = LastDeclMatcher<ClassTemplateDecl>().match(
+ get<1>(TU), classTemplateDecl(hasName("Y"), unless(isImplicit())));
+ ASSERT_EQ(D0->getTemplateDepth(), 1u);
+ ASSERT_EQ(D1->getTemplateDepth(), 0u);
+
+ StructuralEquivalenceContext Ctx_NoIgnoreTemplateParmDepth(
+ get<0>(TU)->getASTContext(), get<1>(TU)->getASTContext(),
+ NonEquivalentDecls, StructuralEquivalenceKind::Default, false, false,
+ false, false);
+
+ EXPECT_FALSE(Ctx_NoIgnoreTemplateParmDepth.IsEquivalent(D0, D1));
+
+ Decl *NonEqDecl0 =
+ D0->getCanonicalDecl()->getTemplateParameters()->getParam(1);
+ Decl *NonEqDecl1 =
+ D1->getCanonicalDecl()->getTemplateParameters()->getParam(1);
+ EXPECT_TRUE(isInNonEqCache(std::make_pair(NonEqDecl0, NonEqDecl1), false));
+ EXPECT_FALSE(isInNonEqCache(std::make_pair(NonEqDecl0, NonEqDecl1), true));
+
+ StructuralEquivalenceContext Ctx_IgnoreTemplateParmDepth(
+ get<0>(TU)->getASTContext(), get<1>(TU)->getASTContext(),
+ NonEquivalentDecls, StructuralEquivalenceKind::Default, false, false,
+ false, true);
+
+ EXPECT_TRUE(Ctx_IgnoreTemplateParmDepth.IsEquivalent(D0, D1));
+
+ EXPECT_FALSE(isInNonEqCache(std::make_pair(NonEqDecl0, NonEqDecl1), true));
+}
+
struct StructuralEquivalenceStmtTest : StructuralEquivalenceTest {};
/// Fallback matcher to be used only when there is no specific matcher for a
More information about the cfe-commits
mailing list