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.k...@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.k...@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.k...@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.k...@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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits