danix800 created this revision.
danix800 added reviewers: balazske, aaron.ballman.
danix800 added a project: clang.
Herald added subscribers: pengfei, martong, kristof.beyls.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
danix800 requested review of this revision.
Herald added a subscriber: cfe-commits.
1. Repeated friends of template class/function in template class are partially
imported:
template <class T>
class Container {
template <class U> friend void m();
template <class U> friend void m();
};
Only one `m` can be imported,
`FromTu`:
ClassTemplateDecl 0x5590cff47ae0 <input.cc:2:9, line:6:9> line:3:15 Container
|-TemplateTypeParmDecl 0x5590cff47990 <line:2:19, col:25> col:25 class depth
0 index 0 T
`-CXXRecordDecl 0x5590cff47a50 <line:3:9, line:6:9> line:3:15 class Container
definition
|-DefinitionData empty aggregate standard_layout trivially_copyable pod
trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |-DefaultConstructor exists trivial constexpr needs_implicit
defaulted_is_constexpr
| |-CopyConstructor simple trivial has_const_param needs_implicit
implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment simple trivial has_const_param needs_implicit
implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
|-CXXRecordDecl 0x5590cff47d30 <col:9, col:15> col:15 implicit class
Container
|-FriendDecl 0x5590cff48048 <line:4:11, col:44> col:42
| `-FunctionTemplateDecl 0x5590cff47f80 parent 0x5590cfefeda8 <col:11,
col:44> col:42 m
| |-TemplateTypeParmDecl 0x5590cff47dc0 <col:21, col:27> col:27 class
depth 1 index 0 U
| `-FunctionDecl 0x5590cff47ec8 parent 0x5590cfefeda8 <col:30, col:44>
col:42 m 'void ()'
`-FriendDecl 0x5590cff48260 <line:5:11, col:44> col:42
`-FunctionTemplateDecl 0x5590cff481f8 parent 0x5590cfefeda8 <col:11,
col:44> col:42 m
|-TemplateTypeParmDecl 0x5590cff48088 <col:21, col:27> col:27 class
depth 1 index 0 U
`-FunctionDecl 0x5590cff48140 parent 0x5590cfefeda8 <col:30, col:44>
col:42 m 'void ()'
`ToTu`:
ClassTemplateDecl 0x5590cffe3ad8 <input.cc:2:9, line:6:9> line:3:15 Container
|-TemplateTypeParmDecl 0x5590cffe3950 <line:2:19, col:25> col:25 class depth
0 index 0 T
`-CXXRecordDecl 0x5590cffe3a10 <line:3:9, line:6:9> line:3:15 class Container
definition
|-DefinitionData empty aggregate standard_layout trivially_copyable pod
trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |-DefaultConstructor exists trivial constexpr needs_implicit
defaulted_is_constexpr
| |-CopyConstructor simple trivial has_const_param needs_implicit
implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment simple trivial has_const_param needs_implicit
implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
|-FunctionTemplateDecl 0x5590cffe3ef8 parent 0x5590cff9b128 <line:4:11,
col:44> col:42 m // extranous node
| |-TemplateTypeParmDecl 0x5590cffe3d20 <col:21, col:27> col:27 class depth
1 index 0 U
| `-FunctionDecl 0x5590cffe3e28 parent 0x5590cff9b128 <col:30, col:44>
col:42 m 'void ()'
|-FriendDecl 0x5590cffe3f60 <col:11, col:44> col:42
| `-FunctionTemplateDecl 0x5590cffe3ef8 parent 0x5590cff9b128 <col:11,
col:44> col:42 m
| |-TemplateTypeParmDecl 0x5590cffe3d20 <col:21, col:27> col:27 class
depth 1 index 0 U
| `-FunctionDecl 0x5590cffe3e28 parent 0x5590cff9b128 <col:30, col:44>
col:42 m 'void ()'
`-CXXRecordDecl 0x5590cffe3fa0 <line:3:9, col:15> col:15 implicit class
Container
Also note that an extranous `FunctionTemplateDecl 0x5590cffe3ef8` is added into
this `ClassTemplateDecl 0x5590cffe3ad8` lexical context, which
renders it NOT identical to the original DeclContext, even after the second
friend is successfully imported, this extra node would crash
`clang-import-test`.
Plan to fix it in another revision.
2. Import single friend node `m` into existing context would crash with an
assertion failure:
./build/tools/clang/unittests/AST/ASTTests
--gtest_filter="*ImportFriendClasses.ImportOfRepeatedFriendFunctionTemplateDecl*"
|& sed 's/danis/xxxxx/'
Note: Google Test filter =
*ImportFriendClasses.ImportOfRepeatedFriendFunctionTemplateDecl*
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from ParameterizedTests/ImportFriendClasses
[ RUN ]
ParameterizedTests/ImportFriendClasses.ImportOfRepeatedFriendFunctionTemplateDecl/0
ASTTests:
/home/xxxxx/Sources/llvm-project-main/clang/lib/AST/ASTImporter.cpp:4139:
clang::ExpectedDecl clang::ASTNodeImporter::VisitFriendDecl(clang::FriendDecl
*): Assertion `ImportedEquivalentFriends.size() <= CountAndPosition.TotalCount
&& "Class with non-matching friends is imported, ODR check wrong?"' failed.
#0 0x00007f1f53bf812a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int)
/home/xxxxx/Sources/llvm-project-main/llvm/lib/Support/Unix/Signals.inc:602:11
#1 0x00007f1f53bf82db PrintStackTraceSignalHandler(void*)
/home/xxxxx/Sources/llvm-project-main/llvm/lib/Support/Unix/Signals.inc:675:1
#2 0x00007f1f53bf6846 llvm::sys::RunSignalHandlers()
/home/xxxxx/Sources/llvm-project-main/llvm/lib/Support/Signals.cpp:104:5
#3 0x00007f1f53bf8af5 SignalHandler(int)
/home/xxxxx/Sources/llvm-project-main/llvm/lib/Support/Unix/Signals.inc:413:1
#4 0x00007f1f5365afd0 (/lib/x86_64-linux-gnu/libc.so.6+0x3bfd0)
#5 0x00007f1f536a9d3c __pthread_kill_implementation
./nptl/./nptl/pthread_kill.c:44:76
#6 0x00007f1f5365af32 raise ./signal/../sysdeps/posix/raise.c:27:6
#7 0x00007f1f53645472 abort ./stdlib/./stdlib/abort.c:81:7
#8 0x00007f1f53645395 _nl_load_domain ./intl/./intl/loadmsgcat.c:1177:9
#9 0x00007f1f53653e32 (/lib/x86_64-linux-gnu/libc.so.6+0x34e32)
#10 0x00007f1f555c7179
clang::ASTNodeImporter::VisitFriendDecl(clang::FriendDecl*)
/home/xxxxx/Sources/llvm-project-main/clang/lib/AST/ASTImporter.cpp:4140:7
#11 0x00007f1f5561f564 clang::declvisitor::Base<std::add_pointer,
clang::ASTNodeImporter, llvm::Expected<clang::Decl*> >::Visit(clang::Decl*)
/home/xxxxx/Sources/llvm-project-main/build/tools/clang/include/clang/AST/DeclNodes.inc:71:1
#12 0x00007f1f555eaf3d clang::ASTImporter::ImportImpl(clang::Decl*)
/home/xxxxx/Sources/llvm-project-main/clang/lib/AST/ASTImporter.cpp:8768:19
#13 0x00007f1f555ce605 clang::ASTImporter::Import(clang::Decl*)
/home/xxxxx/Sources/llvm-project-main/clang/lib/AST/ASTImporter.cpp:9167:8
#14 0x0000562aa520bd4a
clang::ast_matchers::ASTImporterTestBase::TU::import(std::shared_ptr<clang::ASTImporterSharedState>
const&, clang::ASTUnit*, clang::Decl*)
/home/xxxxx/Sources/llvm-project-main/clang/unittests/AST/ASTImporterFixtures.cpp:83:12
#15 0x0000562aa520c9c9
clang::ast_matchers::ASTImporterTestBase::Import(clang::Decl*,
clang::TestLanguage)
/home/xxxxx/Sources/llvm-project-main/clang/unittests/AST/ASTImporterFixtures.cpp:199:9
#16 0x0000562aa531f023 clang::FriendDecl*
clang::ast_matchers::ASTImporterTestBase::Import<clang::FriendDecl>(clang::FriendDecl*,
clang::TestLanguage)
/home/xxxxx/Sources/llvm-project-main/clang/unittests/AST/ASTImporterFixtures.h:185:32
#17 0x0000562aa52af6a2
clang::ast_matchers::ImportFriendClasses::testRepeatedFriendImport(char const*)
/home/xxxxx/Sources/llvm-project-main/clang/unittests/AST/ASTImporterTest.cpp:4068:37
#18 0x0000562aa5255524
clang::ast_matchers::ImportFriendClasses_ImportOfRepeatedFriendFunctionTemplateDecl_Test::TestBody()
/home/xxxxx/Sources/llvm-project-main/clang/unittests/AST/ASTImporterTest.cpp:4388:1
#19 0x00007f1f56ddb76b void
testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*)
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2433:3
#20 0x00007f1f56dc0e07 void
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*)
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2488:5
#21 0x00007f1f56da9963 testing::Test::Run()
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2515:3
#22 0x00007f1f56daa1ba testing::TestInfo::Run()
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2687:12
#23 0x00007f1f56daa71b testing::TestSuite::Run()
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2815:44
#24 0x00007f1f56db2ef9 testing::internal::UnitTestImpl::RunAllTests()
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:5337:24
#25 0x00007f1f56ddea3b bool
testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool>(testing::internal::UnitTestImpl*, bool
(testing::internal::UnitTestImpl::*)(), char const*)
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2433:3
#26 0x00007f1f56dc3247 bool
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool>(testing::internal::UnitTestImpl*, bool
(testing::internal::UnitTestImpl::*)(), char const*)
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:2488:5
#27 0x00007f1f56db2adf testing::UnitTest::Run()
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/src/gtest.cc:4925:10
#28 0x00007f1f578add71 RUN_ALL_TESTS()
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/googletest/include/gtest/gtest.h:2472:3
#29 0x00007f1f578adcb4 main
/home/xxxxx/Sources/llvm-project-main/third-party/unittest/UnitTestMain/TestMain.cpp:55:3
#30 0x00007f1f536461ca __libc_start_call_main
./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#31 0x00007f1f53646285 call_init ./csu/../csu/libc-start.c:128:20
#32 0x00007f1f53646285 __libc_start_main ./csu/../csu/libc-start.c:347:5
#33 0x0000562aa51aed31 _start
(./build/tools/clang/unittests/AST/ASTTests+0x490d31)
`FriendCountAndPosition` computation for `FromContext` by pointer comparison is
not reliable in this case.
As `ToContext` counts what's already imported, we could use
`ASTStructuralEquivalence` which is more reliable.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157684
Files:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4054,6 +4054,24 @@
->lookup(ToRecordOfFriend->getDeclName())
.empty());
}
+
+ void testRepeatedFriendImport(const char *Code) {
+ Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
+ Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
+
+ auto *ToFriend1 = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
+ auto *ToFriend2 = LastDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
+ auto *FromFriend1 =
+ FirstDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
+ auto *FromFriend2 = LastDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
+
+ FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
+ FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+
+ EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+ EXPECT_EQ(ToFriend1, ToImportedFriend1);
+ EXPECT_EQ(ToFriend2, ToImportedFriend2);
+ }
};
TEST_P(ImportFriendClasses, ImportOfFriendRecordDoesNotMergeDefinition) {
@@ -4343,21 +4361,7 @@
friend class X;
};
)";
- Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
- Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
- auto *ToFriend1 = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
- auto *ToFriend2 = LastDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
- auto *FromFriend1 =
- FirstDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
- auto *FromFriend2 = LastDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
-
- FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
- FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
-
- EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
- EXPECT_EQ(ToFriend1, ToImportedFriend1);
- EXPECT_EQ(ToFriend2, ToImportedFriend2);
+ testRepeatedFriendImport(Code);
}
TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
@@ -4368,21 +4372,31 @@
friend void f();
};
)";
- Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
- Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
- auto *ToFriend1 = FirstDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
- auto *ToFriend2 = LastDeclMatcher<FriendDecl>().match(ToTu, friendDecl());
- auto *FromFriend1 =
- FirstDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
- auto *FromFriend2 = LastDeclMatcher<FriendDecl>().match(FromTu, friendDecl());
+ testRepeatedFriendImport(Code);
+}
- FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
- FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendFunctionTemplateDecl) {
+ const char *Code =
+ R"(
+ template <class T>
+ class Container {
+ template <class U> friend void m();
+ template <class U> friend void m();
+ };
+ )";
+ testRepeatedFriendImport(Code);
+}
- EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
- EXPECT_EQ(ToFriend1, ToImportedFriend1);
- EXPECT_EQ(ToFriend2, ToImportedFriend2);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendClassTemplateDecl) {
+ const char *Code =
+ R"(
+ template <class T>
+ class Container {
+ template <class U> friend class X;
+ template <class U> friend class X;
+ };
+ )";
+ testRepeatedFriendImport(Code);
}
TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4064,22 +4064,32 @@
unsigned int IndexOfDecl;
};
-template <class T>
-static FriendCountAndPosition getFriendCountAndPosition(
- const FriendDecl *FD,
- llvm::function_ref<T(const FriendDecl *)> GetCanTypeOrDecl) {
+static bool IsEquivalentFriend(ASTImporter &Importer, FriendDecl *FD1,
+ FriendDecl *FD2) {
+ if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
+ return false;
+
+ if (auto *TSI = FD1->getFriendType())
+ return Importer.IsStructurallyEquivalent(
+ TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);
+
+ StructuralEquivalenceContext Ctx(
+ Importer.getFromContext(), Importer.getToContext(),
+ Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
+ return Ctx.IsEquivalent(FD1, FD2);
+}
+
+static FriendCountAndPosition getFriendCountAndPosition(ASTImporter &Importer,
+ FriendDecl *FD) {
unsigned int FriendCount = 0;
std::optional<unsigned int> FriendPosition;
const auto *RD = cast<CXXRecordDecl>(FD->getLexicalDeclContext());
- T TypeOrDecl = GetCanTypeOrDecl(FD);
-
- for (const FriendDecl *FoundFriend : RD->friends()) {
+ for (auto *FoundFriend : RD->friends()) {
if (FoundFriend == FD) {
FriendPosition = FriendCount;
++FriendCount;
- } else if (!FoundFriend->getFriendDecl() == !FD->getFriendDecl() &&
- GetCanTypeOrDecl(FoundFriend) == TypeOrDecl) {
+ } else if (IsEquivalentFriend(Importer, FD, FoundFriend)) {
++FriendCount;
}
}
@@ -4089,21 +4099,6 @@
return {FriendCount, *FriendPosition};
}
-static FriendCountAndPosition getFriendCountAndPosition(const FriendDecl *FD) {
- if (FD->getFriendType())
- return getFriendCountAndPosition<QualType>(FD, [](const FriendDecl *F) {
- if (TypeSourceInfo *TSI = F->getFriendType())
- return TSI->getType().getCanonicalType();
- llvm_unreachable("Wrong friend object type.");
- });
- else
- return getFriendCountAndPosition<Decl *>(FD, [](const FriendDecl *F) {
- if (Decl *D = F->getFriendDecl())
- return D->getCanonicalDecl();
- llvm_unreachable("Wrong friend object type.");
- });
-}
-
ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
// Import the major distinguishing characteristics of a declaration.
DeclContext *DC, *LexicalDC;
@@ -4114,26 +4109,13 @@
// FriendDecl is not a NamedDecl so we cannot use lookup.
// We try to maintain order and count of redundant friend declarations.
const auto *RD = cast<CXXRecordDecl>(DC);
- FriendDecl *ImportedFriend = RD->getFirstFriend();
SmallVector<FriendDecl *, 2> ImportedEquivalentFriends;
-
- while (ImportedFriend) {
- bool Match = false;
- if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
- Match =
- IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(),
- /*Complain=*/false);
- } else if (D->getFriendType() && ImportedFriend->getFriendType()) {
- Match = Importer.IsStructurallyEquivalent(
- D->getFriendType()->getType(),
- ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
- }
- if (Match)
+ for (auto *ImportedFriend : RD->friends())
+ if (IsEquivalentFriend(Importer, D, ImportedFriend))
ImportedEquivalentFriends.push_back(ImportedFriend);
- ImportedFriend = ImportedFriend->getNextFriend();
- }
- FriendCountAndPosition CountAndPosition = getFriendCountAndPosition(D);
+ FriendCountAndPosition CountAndPosition =
+ getFriendCountAndPosition(Importer, D);
assert(ImportedEquivalentFriends.size() <= CountAndPosition.TotalCount &&
"Class with non-matching friends is imported, ODR check wrong?");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
