diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1881,30 +1881,6 @@ } } - // We reorder declarations in RecordDecls because they may have another order - // in the "to" context than they have in the "from" context. This may happen - // e.g when we import a class like this: - // struct declToImport { - // int a = c + b; - // int b = 1; - // int c = 2; - // }; - // During the import of `a` we import first the dependencies in sequence, - // thus the order would be `c`, `b`, `a`. We will get the normal order by - // first removing the already imported members and then adding them in the - // order as they appear in the "from" context. - // - // Keeping field order is vital because it determines structure layout. - // - // Here and below, we cannot call field_begin() method and its callers on - // ToDC if it has an external storage. Calling field_begin() will - // automatically load all the fields by calling - // LoadFieldsFromExternalStorage(). LoadFieldsFromExternalStorage() would - // call ASTImporter::Import(). This is because the ExternalASTSource - // interface in LLDB is implemented by the means of the ASTImporter. However, - // calling an import at this point would result in an uncontrolled import, we - // must avoid that. - auto ToDCOrErr = Importer.ImportContext(FromDC); if (!ToDCOrErr) { consumeError(std::move(ChildErrors)); @@ -1912,23 +1888,26 @@ } DeclContext *ToDC = *ToDCOrErr; - // Remove all declarations, which may be in wrong order in the - // lexical DeclContext and then add them in the proper order. - for (auto *D : FromDC->decls()) { - if (!MightNeedReordering(D)) - continue; + // Import in-class initializers for fields. + for (auto *D : FromDC->decls()) { assert(D && "DC contains a null decl"); - if (Decl *ToD = Importer.GetAlreadyImportedOrNull(D)) { - // Remove only the decls which we successfully imported. - assert(ToDC == ToD->getLexicalDeclContext() && ToDC->containsDecl(ToD)); - // Remove the decl from its wrong place in the linked list. - ToDC->removeDecl(ToD); - // Add the decl to the end of the linked list. - // This time it will be at the proper place because the enclosing for - // loop iterates in the original (good) order of the decls. - ToDC->addDeclInternal(ToD); - } + auto *FromField = dyn_cast(D); + if (!FromField || !FromField->hasInClassInitializer()) + continue; + Decl *ToD = Importer.GetAlreadyImportedOrNull(D); + if (!ToD || ToDC != ToD->getLexicalDeclContext() || + !ToDC->containsDecl(ToD)) + continue; + auto *ToField = cast(ToD); + Error Err = Error::success(); + auto ToInitializer = importChecked(Err, FromField->getInClassInitializer()); + // Might already been imported. + if (ToField->getInClassInitializer()) + continue; + if (ToInitializer) + ToField->setInClassInitializer(ToInitializer); + HandleChildErrors.handleChildImportResult(ChildErrors, std::move(Err)); } // Import everything else. @@ -3925,7 +3904,7 @@ auto ToTInfo = importChecked(Err, D->getTypeSourceInfo()); auto ToBitWidth = importChecked(Err, D->getBitWidth()); auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart()); - auto ToInitializer = importChecked(Err, D->getInClassInitializer()); + // Deferring in-class initializer to avoid circular refs if (Err) return std::move(Err); const Type *ToCapturedVLAType = nullptr; @@ -3948,8 +3927,6 @@ return std::move(Err); ToField->setAccess(D->getAccess()); ToField->setLexicalDeclContext(LexicalDC); - if (ToInitializer) - ToField->setInClassInitializer(ToInitializer); ToField->setImplicit(D->isImplicit()); if (ToCapturedVLAType) ToField->setCapturedVLAType(cast(ToCapturedVLAType)); @@ -7394,10 +7371,15 @@ if (Err) return std::move(Err); - return UnaryOperator::Create( - Importer.getToContext(), ToSubExpr, E->getOpcode(), ToType, - E->getValueKind(), E->getObjectKind(), ToOperatorLoc, E->canOverflow(), - E->getFPOptionsOverride()); + auto *UO = UnaryOperator::CreateEmpty(Importer.getToContext(), + E->hasStoredFPFeatures()); + UO->setType(ToType); + UO->setSubExpr(ToSubExpr); + UO->setOpcode(E->getOpcode()); + UO->setOperatorLoc(ToOperatorLoc); + UO->setCanOverflow(E->canOverflow()); + + return UO; } ExpectedStmt diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/RecordLayout.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/ADT/StringMap.h" #include "llvm/Support/SmallVectorMemoryBuffer.h" @@ -1539,6 +1540,83 @@ Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"})))); } +TEST_P(ASTImporterOptionSpecificTestBase, FieldCircularRef) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"s( + struct declToImport { + int a{a}; + int b = c; + int c = b + c + a; + }; + )s", + Lang_CXX11, "", Lang_CXX11); + + auto FieldBWithInit = has(fieldDecl( + hasName("b"), + hasInClassInitializer(ignoringImpCasts(memberExpr( + hasObjectExpression(cxxThisExpr()), member(hasName("c"))))))); + auto FieldCWithInit = has(fieldDecl( + hasName("c"), + hasInClassInitializer(binaryOperator(hasLHS(binaryOperator()))))); + auto Pattern = cxxRecordDecl(FieldBWithInit, FieldCWithInit, + hasFieldOrder({"a", "b", "c"})); + MatchVerifier Verifier; + ASSERT_TRUE(Verifier.match(From, Pattern)); + EXPECT_TRUE(Verifier.match(To, Pattern)); +} + +TEST_P(ASTImporterOptionSpecificTestBase, + ImportFieldDirectlyWithInitializerContainingCircularRef) { + auto Code = R"s( + struct declToImport { + int a{a}; + int b = c; + int c = b + c + a; + }; + )s"; + + auto *FromField = FirstDeclMatcher().match( + getTuDecl(Code, Lang_CXX11), fieldDecl(hasName("b"))); + auto *ToField = Import(FromField, Lang_CXX11); + + auto Pattern = fieldDecl( + hasName("b"), + hasInClassInitializer(ignoringImpCasts(memberExpr( + hasObjectExpression(cxxThisExpr()), member(hasName("c")))))); + + MatchVerifier Verifier; + ASSERT_TRUE(Verifier.match(FromField, Pattern)); + EXPECT_TRUE(Verifier.match(ToField, Pattern)); +} + +TEST_P(ASTImporterOptionSpecificTestBase, FieldCircularIndirectRef) { + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + R"s( + static int ref_A(); + static int ref_B(); + struct A { + int a = ref_B(); + }; + struct B { + int b = ref_A(); + }; + int ref_B() { B b; return b.b; } + int ref_A() { A a; return a.a; } + )s", + Lang_CXX11, "", Lang_CXX11, "A"); + + auto InitByRefB = + hasInClassInitializer(callExpr(callee(functionDecl(hasName("ref_B"))))); + auto FieldAWithInit = has(fieldDecl(hasName("a"), InitByRefB)); + auto Pattern = + cxxRecordDecl(hasName("A"), FieldAWithInit, hasFieldOrder({"a"})); + MatchVerifier Verifier; + ASSERT_TRUE(Verifier.match(From, Pattern)); + EXPECT_TRUE(Verifier.match(To, Pattern)); +} + TEST_P(ASTImporterOptionSpecificTestBase, CXXRecordDeclFieldAndIndirectFieldOrder) { Decl *From, *To; @@ -8028,6 +8106,45 @@ Import(FromF, Lang_CXX11); } +TEST_P(ASTImporterOptionSpecificTestBase, + ImportCirularRefFieldsWithoutCorruptedRecordLayoutCacheTest) { + // Import sequence: A => A.b => B => B.f() => ... => UnaryOperator(&) => ... + // + // UnaryOperator(&) force computing RecordLayout of 'A' while it's still not + // completely imported. + auto Code = + R"( + class B; + class A { + B* b; + int c; + }; + class B { + A *f() { return &((B *)0)->a; } + A a; + }; + )"; + + auto *FromR = FirstDeclMatcher().match( + getTuDecl(Code, Lang_CXX11), cxxRecordDecl(hasName("A"))); + FromR = FromR->getDefinition(); + auto &FromAST = FromR->getASTContext(); + auto *ToR = Import(FromR, Lang_CXX11); + auto &ToAST = ToR->getASTContext(); + + uint64_t SecondFieldOffset = FromAST.getTypeSize(FromAST.VoidPtrTy); + + EXPECT_TRUE(FromR->isCompleteDefinition()); + const auto &FromLayout = FromAST.getASTRecordLayout(FromR); + EXPECT_TRUE(FromLayout.getFieldOffset(0) == 0); + EXPECT_TRUE(FromLayout.getFieldOffset(1) == SecondFieldOffset); + + EXPECT_TRUE(ToR->isCompleteDefinition()); + const auto &ToLayout = ToAST.getASTRecordLayout(ToR); + EXPECT_TRUE(ToLayout.getFieldOffset(0) == 0); + EXPECT_TRUE(ToLayout.getFieldOffset(1) == SecondFieldOffset); +} + TEST_P(ASTImporterOptionSpecificTestBase, ImportRecordWithLayoutRequestingExpr) { TranslationUnitDecl *FromTU = getTuDecl(