[clang][ASTImporter] Allow import of similar friend template with different depth

[balazske]

The fix applies to a case that occurs when the AST contains a friend template that is contained within another template and this (outer) template has specialization. (See the added test code in the commit.)
The "friend" class can have a declaration before in the AST. The friend template has the same parent as this first declaration (and is otherwise similar), but has different template depth. At AST import these declarations can be compared and import should not fail with name conflict in this case.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)

Changes

The fix applies to a case that occurs when the AST contains a friend template that is contained within another template and this (outer) template has specialization. (See the added test code in the commit.)
The "friend" class can have a declaration before in the AST. The friend template has the same parent as this first declaration (and is otherwise similar), but has different template depth. At AST import these declarations can be compared and import should not fail with name conflict in this case.

---

Full diff: https://github.com/llvm/llvm-project/pull/115734.diff

2 Files Affected:

• (modified) clang/lib/AST/ASTImporter.cpp (+14-2)
• (modified) clang/unittests/AST/ASTImporterTest.cpp (+105)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 35aba41f0052af..1f4ae74f3c3659 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6095,8 +6095,7 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
                                               Decl::IDNS_TagFriend))
         continue;
 
-      Decl *Found = FoundDecl;
-      auto *FoundTemplate = dyn_cast<ClassTemplateDecl>(Found);
+      auto *FoundTemplate = dyn_cast<ClassTemplateDecl>(FoundDecl);
       if (FoundTemplate) {
         if (!hasSameVisibilityContextAndLinkage(FoundTemplate, D))
           continue;
@@ -6120,6 +6119,19 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
           // see ASTTests test ImportExistingFriendClassTemplateDef.
           continue;
         }
+        // When importing a friend, it is possible that multiple declarations
+        // with same name can co-exist in specific cases (if a template contains
+        // a friend template and has a specialization). For this case the
+        // declarations should match, except that the "template depth" is
+        // different. No linking of previous declaration is needed in this case.
+        // FIXME: This condition may need refinement.
+        if (D->getFriendObjectKind() != Decl::FOK_None &&
+            FoundTemplate->getFriendObjectKind() != Decl::FOK_None &&
+            D->getFriendObjectKind() != FoundTemplate->getFriendObjectKind() &&
+            IsStructuralMatch(D, FoundTemplate, /*Complain=*/false,
+                              /*IgnoreTemplateParmDepth=*/true))
+          continue;
+
         ConflictingDecls.push_back(FoundDecl);
       }
     }
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index bf7313f882e455..49aef7441727c9 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -10181,6 +10181,111 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
       FromD, FromDInherited);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+       ExistingUndeclaredImportDeclaredFriend) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+        template <class A, A>
+        struct foo;
+
+        template <class A>
+        struct X {
+          template <class A1, A1>
+          friend struct foo;
+        };
+      )",
+      Lang_CXX11);
+  Decl *FromTU = getTuDecl(
+      R"(
+        template <class A, A>
+        struct foo;
+
+        template <class A>
+        struct X {
+          template <class A1, A1>
+          friend struct foo1;
+        };
+
+        X<int> x;
+      )",
+      Lang_CXX11);
+
+  auto *ToFr1 = FirstDeclMatcher<FriendDecl>().match(ToTU, friendDecl());
+  auto *ToFrD1 = ToFr1->getFriendDecl();
+
+  auto *FromFr1 = FirstDeclMatcher<FriendDecl>().match(FromTU, friendDecl());
+  auto *FromFr2 = LastDeclMatcher<FriendDecl>().match(FromTU, friendDecl());
+
+  auto *FromFrD1 = FromFr1->getFriendDecl();
+  auto *FromFrD2 = FromFr2->getFriendDecl();
+
+  auto *Ctx1 = cast<Decl>(FromFrD1->getDeclContext());
+  auto *Ctx2 = cast<Decl>(FromFrD2->getDeclContext());
+
+  ASSERT_EQ(Ctx1, Ctx2);
+  ASSERT_EQ(ToFrD1->getTemplateDepth(), 1u);
+  ASSERT_EQ(FromFrD2->getTemplateDepth(), 0u);
+  ASSERT_EQ(ToFrD1->getFriendObjectKind(), Decl::FOK_Undeclared);
+  ASSERT_EQ(FromFrD2->getFriendObjectKind(), Decl::FOK_Declared);
+
+  auto *ToFr2Imp = Import(FromFr2, Lang_CXX11);
+
+  EXPECT_TRUE(ToFr2Imp);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+       ExistingDeclaredImportUndeclaredFriend) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+        template <class A, A>
+        struct foo;
+
+        template <class A>
+        struct X {
+          template <class A1, A1>
+          friend struct foo;
+        };
+
+        X<int> x;
+      )",
+      Lang_CXX11);
+  Decl *FromTU = getTuDecl(
+      R"(
+        template <class A, A>
+        struct foo;
+
+        template <class A>
+        struct X {
+          template <class A1, A1>
+          friend struct foo;
+        };
+      )",
+      Lang_CXX11);
+
+  auto *ToFr1 = FirstDeclMatcher<FriendDecl>().match(ToTU, friendDecl());
+  auto *ToFr2 = LastDeclMatcher<FriendDecl>().match(ToTU, friendDecl());
+
+  auto *ToFrD1 = ToFr1->getFriendDecl();
+  auto *ToFrD2 = ToFr2->getFriendDecl();
+
+  auto *FromFr1 = FirstDeclMatcher<FriendDecl>().match(FromTU, friendDecl());
+  auto *FromFrD1 = FromFr1->getFriendDecl();
+
+  auto *Ctx1 = cast<Decl>(ToFrD1->getDeclContext());
+  auto *Ctx2 = cast<Decl>(ToFrD2->getDeclContext());
+
+  ASSERT_EQ(Ctx1, Ctx2);
+  ASSERT_EQ(FromFrD1->getTemplateDepth(), 1u);
+  ASSERT_EQ(ToFrD2->getTemplateDepth(), 0u);
+  ASSERT_EQ(FromFrD1->getFriendObjectKind(), Decl::FOK_Undeclared);
+  ASSERT_EQ(ToFrD2->getFriendObjectKind(), Decl::FOK_Declared);
+
+  auto *ToFr1Imp = Import(FromFr1, Lang_CXX11);
+
+  EXPECT_TRUE(ToFr1Imp);
+  EXPECT_EQ(ToFr1Imp, ToFr1);
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 

[balazske]

Currently one of the tests fail, the problem is fixed if #115518 is applied.

[Michael137]

It feels like the logic on 6103 and this here could be merged together? Should IgnoreTemplateParmDepth on 6104 just be changed to also be set if the FriendObjectKind matches? Then I think everything should work out as expected

[balazske]

I did not want to modify an already working code part when it is not obvious that the change has no effect on the old behavior. In the new case we already know that there is no structural match and depth was not ignored, then we can check if these match with ignored depth (and this second structural equivalence check looks required to not import non-matching friends). We do not need any of the actions in the if branch at line 6108, only a continue (and it is not "dependent friend").

[jcsxky]

LGTM!