Author: a.sidorin Date: Mon Oct 29 14:46:18 2018 New Revision: 345545 URL: http://llvm.org/viewvc/llvm-project?rev=345545&view=rev Log: [ASTImporter] Reorder fields after structure import is finished
There are multiple reasons why field structures can be imported in wrong order. The simplest is the ability of field initializers and method bodies to refer fields not in order they are listed in. Unfortunately, there is no clean solution for that currently so I'm leaving a FIXME. Differential Revision: https://reviews.llvm.org/D44100 Modified: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=345545&r1=345544&r2=345545&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Oct 29 14:46:18 2018 @@ -1658,13 +1658,53 @@ ASTNodeImporter::ImportDeclContext(DeclC auto ToDCOrErr = Importer.ImportContext(FromDC); return ToDCOrErr.takeError(); } - llvm::SmallVector<Decl *, 8> ImportedDecls; + + const auto *FromRD = dyn_cast<RecordDecl>(FromDC); for (auto *From : FromDC->decls()) { ExpectedDecl ImportedOrErr = import(From); - if (!ImportedOrErr) + if (!ImportedOrErr) { + // For RecordDecls, failed import of a field will break the layout of the + // structure. Handle it as an error. + if (FromRD) + return ImportedOrErr.takeError(); // Ignore the error, continue with next Decl. // FIXME: Handle this case somehow better. - consumeError(ImportedOrErr.takeError()); + else + consumeError(ImportedOrErr.takeError()); + } + } + + // Reorder declarations in RecordDecls because they may have another + // order. Keeping field order is vitable because it determines structure + // layout. + // FIXME: This is an ugly fix. Unfortunately, I cannot come with better + // solution for this issue. We cannot defer expression import here because + // type import can depend on them. + if (!FromRD) + return Error::success(); + + auto ImportedDC = import(cast<Decl>(FromDC)); + assert(ImportedDC); + auto *ToRD = cast<RecordDecl>(*ImportedDC); + + for (auto *D : FromRD->decls()) { + if (isa<FieldDecl>(D) || isa<FriendDecl>(D)) { + Decl *ToD = Importer.GetAlreadyImportedOrNull(D); + assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD)); + ToRD->removeDecl(ToD); + } + } + + assert(ToRD->field_empty()); + + for (auto *D : FromRD->decls()) { + if (isa<FieldDecl>(D) || isa<FriendDecl>(D)) { + Decl *ToD = Importer.GetAlreadyImportedOrNull(D); + assert(ToRD == ToD->getDeclContext()); + assert(ToRD == ToD->getLexicalDeclContext()); + assert(!ToRD->containsDecl(ToD)); + ToRD->addDeclInternal(ToD); + } } return Error::success(); Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=345545&r1=345544&r2=345545&view=diff ============================================================================== --- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original) +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Oct 29 14:46:18 2018 @@ -1457,7 +1457,7 @@ TEST_P(ASTImporterTestBase, CXXRecordDec } TEST_P(ASTImporterTestBase, - DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) { + CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) { Decl *From, *To; std::tie(From, To) = getImportedDecl( // The original recursive algorithm of ASTImporter first imports 'c' then @@ -3767,5 +3767,16 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTes INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables, DefaultTestValuesForRunOptions, ); +TEST_P(ImportDecl, ImportFieldOrder) { + MatchVerifier<Decl> Verifier; + testImport("struct declToImport {" + " int b = a + 2;" + " int a = 5;" + "};", + Lang_CXX11, "", Lang_CXX11, Verifier, + recordDecl(hasFieldOrder({"b", "a"}))); +} + + } // end namespace ast_matchers } // end namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
