[clang] [clang][ASTImporter] Improve import of friend class templates. (PR #74627)

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 9 08:53:28 PST 2024


https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/74627

>From cbcb81ebdbc49e3bd11b6f716ac14658a729b787 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 5 Dec 2023 15:23:37 +0100
Subject: [PATCH 1/4] [clang][ASTImporter] Improve import of friend class
 templates.

A friend template that is in a dependent context is not linked into
declaration chains (for example with the definition of the befriended
template). This condition was not correctly handled by ASTImporter.
---
 clang/lib/AST/ASTImporter.cpp           |  26 +++-
 clang/unittests/AST/ASTImporterTest.cpp | 162 ++++++++++++++++++++++++
 2 files changed, 182 insertions(+), 6 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..a243bf65cca274 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5919,15 +5919,26 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   if (ToD)
     return ToD;
 
-  bool IsFriendTemplate = D->getFriendObjectKind() != Decl::FOK_None;
-  bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
+  // Should check if a declaration is friend in a dependent context.
+  // Such templates are not linked together in a declaration chain.
+  // The ASTImporter strategy is to map existing forward declarations to
+  // imported ones only if strictly necessary, otherwise import these as new
+  // forward declarations. In case of the "dependent friend" declarations, new
+  // declarations are created, but not linked in a declaration chain.
+  auto IsDependentFriend = [](ClassTemplateDecl *TD) {
+    bool IsFriendTemplate = TD->getFriendObjectKind() != Decl::FOK_None;
+    DeclContext *DC = TD->getDeclContext();
+    DeclContext *LexicalDC = TD->getLexicalDeclContext();
+    bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
                                             : DC->isDependentContext();
-  bool DependentFriend = IsFriendTemplate && IsDependentContext;
+    return IsFriendTemplate && IsDependentContext;
+  };
+  bool DependentFriend = IsDependentFriend(D);
 
   ClassTemplateDecl *FoundByLookup = nullptr;
 
   // We may already have a template of the same name; try to find and match it.
-  if (!DependentFriend && !DC->isFunctionOrMethod()) {
+  if (!DC->isFunctionOrMethod()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
     for (auto *FoundDecl : FoundDecls) {
@@ -5943,10 +5954,13 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 
         // FIXME: sufficient conditon for 'IgnoreTemplateParmDepth'?
         bool IgnoreTemplateParmDepth =
-            FoundTemplate->getFriendObjectKind() != Decl::FOK_None &&
-            !D->specializations().empty();
+            (FoundTemplate->getFriendObjectKind() != Decl::FOK_None) !=
+            (D->getFriendObjectKind() != Decl::FOK_None);
         if (IsStructuralMatch(D, FoundTemplate, /*Complain=*/true,
                               IgnoreTemplateParmDepth)) {
+          if (DependentFriend || IsDependentFriend(FoundTemplate))
+            continue;
+
           ClassTemplateDecl *TemplateWithDef =
               getTemplateDefinition(FoundTemplate);
           if (D->isThisDeclarationADefinition() && TemplateWithDef)
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..f9ee73626c948e 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4528,6 +4528,168 @@ TEST_P(ImportFriendClasses, ImportOfClassDefinitionAndFwdFriendShouldBeLinked) {
   EXPECT_EQ(ImportedFwd, ImportedDef->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses,
+       ImportFriendTemplatesInDependentContext_DefToFriend) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template<class T1>
+      struct Y {};
+      )",
+      Lang_CXX03, "input0.cc");
+  auto *FromYDef = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYDef = Import(FromYDef, Lang_CXX03);
+  EXPECT_TRUE(ImportedYDef);
+  EXPECT_FALSE(ImportedYDef->getPreviousDecl());
+  EXPECT_NE(ImportedYDef, ToYFriend);
+}
+
+TEST_P(ImportFriendClasses,
+       ImportFriendTemplatesInDependentContext_DefToFriend_NE) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template<class T1, class T2>
+      struct Y {};
+      )",
+      Lang_CXX03, "input0.cc");
+  auto *FromYDef = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYDef = Import(FromYDef, Lang_CXX03);
+  EXPECT_FALSE(ImportedYDef);
+}
+
+TEST_P(ImportFriendClasses,
+       ImportFriendTemplatesInDependentContext_FriendToFriend) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03, "input0.cc");
+  auto *FromYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYFriend = Import(FromYFriend, Lang_CXX03);
+  EXPECT_TRUE(ImportedYFriend);
+  EXPECT_FALSE(ImportedYFriend->getPreviousDecl());
+  EXPECT_NE(ImportedYFriend, ToYFriend);
+}
+
+TEST_P(ImportFriendClasses,
+       ImportFriendTemplatesInDependentContext_FriendToFriend_NE) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2, class T3>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03, "input0.cc");
+  auto *FromYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYFriend = Import(FromYFriend, Lang_CXX03);
+  EXPECT_FALSE(ImportedYFriend);
+}
+
+TEST_P(ImportFriendClasses,
+       ImportFriendTemplatesInDependentContext_FriendToDef) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template<class T1>
+      struct Y {};
+      )",
+      Lang_CXX03);
+  auto *ToYDef = FirstDeclMatcher<ClassTemplateDecl>().match(
+      ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03, "input0.cc");
+  auto *FromYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYFriend = Import(FromYFriend, Lang_CXX03);
+  EXPECT_TRUE(ImportedYFriend);
+  EXPECT_FALSE(ImportedYFriend->getPreviousDecl());
+  EXPECT_NE(ImportedYFriend, ToYDef);
+}
+
+TEST_P(ImportFriendClasses,
+       ImportFriendTemplatesInDependentContext_FriendToDef_NE) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template<class T1>
+      struct Y {};
+      )",
+      Lang_CXX03);
+  auto *ToYDef = FirstDeclMatcher<ClassTemplateDecl>().match(
+      ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template<class T1>
+      struct X {
+        template<class T2, class T3>
+        friend struct Y;
+      };
+      )",
+      Lang_CXX03, "input0.cc");
+  auto *FromYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYFriend = Import(FromYFriend, Lang_CXX03);
+  EXPECT_FALSE(ImportedYFriend);
+}
+
 TEST_P(ImportFriendClasses, ImportOfRepeatedFriendType) {
   const char *Code =
       R"(

>From 5f4ec0cf23607474535d4b4e8f674cf2d006d10f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 6 Dec 2023 18:21:21 +0100
Subject: [PATCH 2/4] fix formatting error

---
 clang/lib/AST/ASTImporter.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index a243bf65cca274..da8960e6d34a0c 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5930,7 +5930,7 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
     DeclContext *DC = TD->getDeclContext();
     DeclContext *LexicalDC = TD->getLexicalDeclContext();
     bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
-                                            : DC->isDependentContext();
+                                              : DC->isDependentContext();
     return IsFriendTemplate && IsDependentContext;
   };
   bool DependentFriend = IsDependentFriend(D);

>From 0e63ed17b6f05eca4350be6f568cac48cd667433 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 13 Dec 2023 14:51:39 +0100
Subject: [PATCH 3/4] removed unused variables

---
 clang/unittests/AST/ASTImporterTest.cpp | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index f9ee73626c948e..a77129a5c3301a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4557,7 +4557,7 @@ TEST_P(ImportFriendClasses,
 
 TEST_P(ImportFriendClasses,
        ImportFriendTemplatesInDependentContext_DefToFriend_NE) {
-  Decl *ToTU = getToTuDecl(
+  getToTuDecl(
       R"(
       template<class T1>
       struct X {
@@ -4566,8 +4566,6 @@ TEST_P(ImportFriendClasses,
       };
       )",
       Lang_CXX03);
-  auto *ToYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
-      ToTU, classTemplateDecl(hasName("Y")));
   Decl *FromTU = getTuDecl(
       R"(
       template<class T1, class T2>
@@ -4612,7 +4610,7 @@ TEST_P(ImportFriendClasses,
 
 TEST_P(ImportFriendClasses,
        ImportFriendTemplatesInDependentContext_FriendToFriend_NE) {
-  Decl *ToTU = getToTuDecl(
+  getToTuDecl(
       R"(
       template<class T1>
       struct X {
@@ -4621,8 +4619,6 @@ TEST_P(ImportFriendClasses,
       };
       )",
       Lang_CXX03);
-  auto *ToYFriend = FirstDeclMatcher<ClassTemplateDecl>().match(
-      ToTU, classTemplateDecl(hasName("Y")));
   Decl *FromTU = getTuDecl(
       R"(
       template<class T1>
@@ -4667,14 +4663,12 @@ TEST_P(ImportFriendClasses,
 
 TEST_P(ImportFriendClasses,
        ImportFriendTemplatesInDependentContext_FriendToDef_NE) {
-  Decl *ToTU = getToTuDecl(
+  getToTuDecl(
       R"(
       template<class T1>
       struct Y {};
       )",
       Lang_CXX03);
-  auto *ToYDef = FirstDeclMatcher<ClassTemplateDecl>().match(
-      ToTU, classTemplateDecl(hasName("Y")));
   Decl *FromTU = getTuDecl(
       R"(
       template<class T1>

>From 459603bd4dbbe6a6aa8aeb597714002495de3ea8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 9 Jan 2024 17:52:30 +0100
Subject: [PATCH 4/4] simplified 'IsDependentFriend'

---
 clang/lib/AST/ASTImporter.cpp | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index da8960e6d34a0c..bf8a60840dbd28 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5926,12 +5926,8 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   // forward declarations. In case of the "dependent friend" declarations, new
   // declarations are created, but not linked in a declaration chain.
   auto IsDependentFriend = [](ClassTemplateDecl *TD) {
-    bool IsFriendTemplate = TD->getFriendObjectKind() != Decl::FOK_None;
-    DeclContext *DC = TD->getDeclContext();
-    DeclContext *LexicalDC = TD->getLexicalDeclContext();
-    bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
-                                              : DC->isDependentContext();
-    return IsFriendTemplate && IsDependentContext;
+    return TD->getFriendObjectKind() != Decl::FOK_None &&
+           TD->getLexicalDeclContext()->isDependentContext();
   };
   bool DependentFriend = IsDependentFriend(D);
 



More information about the cfe-commits mailing list