[clang][AST] Add 'IgnoreTemplateParmDepth' to structural equivalence cache (#115518)

balazske · web-flow · commit 7a1fdbb9c0f3 · 2024-11-13T09:25:22.000+01:00
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.
diff --git 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
@@ -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
@@ -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
@@ -9046,7 +9046,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
@@ -9907,7 +9907,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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -9046,7 +9046,7 @@ bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,`
`9046`	`9046`	`}`
`9047`	`9047`
`9048`	`9048`	`bool Sema::hasStructuralCompatLayout(Decl D, Decl Suggested) {`
`9049`		`- llvm::DenseSet<std::pair<Decl , Decl >> NonEquivalentDecls;`
	`9049`	`+ StructuralEquivalenceContext::NonEquivalentDeclSet NonEquivalentDecls;`
`9050`	`9050`	`if (!Suggested)`
`9051`	`9051`	`return false;`
`9052`	`9052`