From: Gabor Marton Date: Mon, 26 Nov 2018 15:54:08 +0000 (+0000) Subject: [ASTImporter][Structural Eq] Check for isBeingDefined X-Git-Tag: llvmorg-8.0.0-rc1~3609 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=17d39677e015e8c4398949dd089dfd3fab136222;p=platform%2Fupstream%2Fllvm.git [ASTImporter][Structural Eq] Check for isBeingDefined Summary: If one definition is currently being defined, we do not compare for equality and we assume that the decls are equal. Reviewers: a_sidorin, a.sidorin, shafik Reviewed By: a_sidorin Subscribers: gamesh411, shafik, rnkovacs, dkrupp, Szelethus, cfe-commits Differential Revision: https://reviews.llvm.org/D53697 llvm-svn: 347564 --- diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 187c781..77a1a7f 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1740,8 +1740,9 @@ Error ASTNodeImporter::ImportDefinition( return Err; // Add base classes. - if (auto *ToCXX = dyn_cast(To)) { - auto *FromCXX = cast(From); + auto *ToCXX = dyn_cast(To); + auto *FromCXX = dyn_cast(From); + if (ToCXX && FromCXX && ToCXX->dataPtr() && FromCXX->dataPtr()) { struct CXXRecordDecl::DefinitionData &ToData = ToCXX->data(); struct CXXRecordDecl::DefinitionData &FromData = FromCXX->data(); diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp index 9149006..b6ed757 100644 --- a/clang/lib/AST/ASTStructuralEquivalence.cpp +++ b/clang/lib/AST/ASTStructuralEquivalence.cpp @@ -1016,7 +1016,8 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, 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 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, 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(D1)) { if (auto *D2CXX = dyn_cast(D2)) { if (D1CXX->hasExternalLexicalStorage() && diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 7cc43b5..3a0ee5d 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -3725,6 +3725,45 @@ TEST_P(ImportFunctionTemplateSpecializations, DefinitionThenPrototype) { EXPECT_EQ(To1->getPreviousDecl(), To0); } +TEST_P(ASTImporterTestBase, + ImportShouldNotReportFalseODRErrorWhenRecordIsBeingDefined) { + { + Decl *FromTU = getTuDecl( + R"( + template + struct B; + )", + Lang_CXX, "input0.cc"); + auto *FromD = FirstDeclMatcher().match( + FromTU, classTemplateDecl(hasName("B"))); + + Import(FromD, Lang_CXX); + } + + { + Decl *FromTU = getTuDecl( + R"( + template + struct B { + void f(); + B* b; + }; + )", + Lang_CXX, "input1.cc"); + FunctionDecl *FromD = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("f"))); + Import(FromD, Lang_CXX); + auto *FromCTD = FirstDeclMatcher().match( + FromTU, classTemplateDecl(hasName("B"))); + auto *ToCTD = cast(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()), );