martong created this revision.
martong added a reviewer: a_sidorin.
Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs.
martong added a dependency: D53693: [ASTImporter] Typedef import brings in the
complete type.
If one definition is currently being defined, we do not compare for
equality and we assume that the decls are equal.
Repository:
rC Clang
https://reviews.llvm.org/D53697
Files:
lib/AST/ASTStructuralEquivalence.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3726,6 +3726,77 @@
EXPECT_EQ(To1->getPreviousDecl(), To0);
}
+TEST_P(ASTImporterTestBase, ImportingTypedefShouldImportTheCompleteType) {
+ // We already have an incomplete underlying type in the "To" context.
+ auto Code =
+ R"(
+ template <typename T>
+ struct S {
+ void foo();
+ };
+ using U = S<int>;
+ )";
+ Decl *ToTU = getToTuDecl(Code, Lang_CXX11);
+ auto *ToD = FirstDeclMatcher<TypedefNameDecl>().match(ToTU,
+ typedefNameDecl(hasName("U")));
+ ASSERT_TRUE(ToD->getUnderlyingType()->isIncompleteType());
+
+ // The "From" context has the same typedef, but the underlying type is
+ // complete this time.
+ Decl *FromTU = getTuDecl(std::string(Code) +
+ R"(
+ void foo(U* u) {
+ u->foo();
+ }
+ )", Lang_CXX11);
+ auto *FromD = FirstDeclMatcher<TypedefNameDecl>().match(FromTU,
+ typedefNameDecl(hasName("U")));
+ ASSERT_FALSE(FromD->getUnderlyingType()->isIncompleteType());
+
+ // The imported type should be complete.
+ auto *ImportedD = cast<TypedefNameDecl>(Import(FromD, Lang_CXX11));
+ EXPECT_FALSE(ImportedD->getUnderlyingType()->isIncompleteType());
+}
+
+TEST_P(ASTImporterTestBase,
+ ImportShouldNotReportFalseODRErrorWhenRecordIsBeingDefined) {
+ {
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <typename T>
+ struct B;
+ )",
+ Lang_CXX, "input0.cc");
+ auto *FromD = FirstDeclMatcher<ClassTemplateDecl>().match(
+ FromTU, classTemplateDecl(hasName("B")));
+
+ Import(FromD, Lang_CXX);
+ }
+
+ {
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <typename T>
+ struct B {
+ void f();
+ B* b;
+ };
+ )",
+ Lang_CXX, "input1.cc");
+ FunctionDecl *FromD = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+ Import(FromD, Lang_CXX);
+ auto *FromCTD = FirstDeclMatcher<ClassTemplateDecl>().match(
+ FromTU, classTemplateDecl(hasName("B")));
+ auto *ToCTD = cast<ClassTemplateDecl>(Import(FromCTD, Lang_CXX));
+ EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+
+ // We expect no (ODR) warning during the import.
+ auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+ EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+ }
+}
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
::testing::Values(ArgVector()), );
Index: lib/AST/ASTStructuralEquivalence.cpp
===================================================================
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1016,7 +1016,8 @@
return false;
// Compare the definitions of these two records. If either or both are
- // incomplete, we assume that they are equivalent.
+ // incomplete (i.e. it is a forward decl), we assume that they are
+ // equivalent.
D1 = D1->getDefinition();
D2 = D2->getDefinition();
if (!D1 || !D2)
@@ -1031,6 +1032,11 @@
if (D1->hasExternalLexicalStorage() || D2->hasExternalLexicalStorage())
return true;
+ // If one definition is currently being defined, we do not compare for
+ // equality and we assume that the decls are equal.
+ if (D1->isBeingDefined() || D2->isBeingDefined())
+ return true;
+
if (auto *D1CXX = dyn_cast<CXXRecordDecl>(D1)) {
if (auto *D2CXX = dyn_cast<CXXRecordDecl>(D2)) {
if (D1CXX->hasExternalLexicalStorage() &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
