Author: spyffe Date: Fri May 12 19:46:33 2017 New Revision: 302975 URL: http://llvm.org/viewvc/llvm-project?rev=302975&view=rev Log: [ASTImporter] Improve handling of incomplete types
ASTImporter has some bugs when it's importing types that themselves come from an ExternalASTSource. This is exposed particularly in the behavior when comparing complete TagDecls with forward declarations. This patch does several things: - Adds a test case making sure that conflicting forward-declarations are resolved correctly; - Extends the clang-import-test harness to test two-level importing, so that we make sure we complete types when necessary; and - Fixes a few bugs I found this way. Failure to complete types was one; however, I also discovered that complete RecordDecls aren't properly added to the redecls chain for existing forward declarations. Added: cfe/trunk/test/Import/conflicting-struct/ cfe/trunk/test/Import/conflicting-struct/Inputs/ cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp cfe/trunk/test/Import/conflicting-struct/test.cpp Modified: cfe/trunk/include/clang/AST/ExternalASTMerger.h cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp cfe/trunk/lib/AST/ExternalASTMerger.cpp cfe/trunk/tools/clang-import-test/clang-import-test.cpp Modified: cfe/trunk/include/clang/AST/ExternalASTMerger.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTMerger.h?rev=302975&r1=302974&r2=302975&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/ExternalASTMerger.h (original) +++ cfe/trunk/include/clang/AST/ExternalASTMerger.h Fri May 12 19:46:33 2017 @@ -44,6 +44,8 @@ public: FindExternalLexicalDecls(const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant, SmallVectorImpl<Decl *> &Result) override; + + void CompleteType(TagDecl *Tag) override; }; } // end namespace clang Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=302975&r1=302974&r2=302975&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Fri May 12 19:46:33 2017 @@ -1622,10 +1622,18 @@ Decl *ASTNodeImporter::VisitRecordDecl(R // We may already have a record of the same name; try to find and match it. RecordDecl *AdoptDecl = nullptr; + RecordDecl *PrevDecl = nullptr; if (!DC->isFunctionOrMethod()) { SmallVector<NamedDecl *, 4> ConflictingDecls; SmallVector<NamedDecl *, 2> FoundDecls; DC->getRedeclContext()->localUncachedLookup(SearchName, FoundDecls); + + if (!FoundDecls.empty()) { + // We're going to have to compare D against potentially conflicting Decls, so complete it. + if (D->hasExternalLexicalStorage() && !D->isCompleteDefinition()) + D->getASTContext().getExternalSource()->CompleteType(D); + } + for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) { if (!FoundDecls[I]->isInIdentifierNamespace(IDNS)) continue; @@ -1652,6 +1660,8 @@ Decl *ASTNodeImporter::VisitRecordDecl(R } } + PrevDecl = FoundRecord; + if (RecordDecl *FoundDef = FoundRecord->getDefinition()) { if ((SearchName && !D->isCompleteDefinition()) || (D->isCompleteDefinition() && @@ -1744,6 +1754,10 @@ Decl *ASTNodeImporter::VisitRecordDecl(R LexicalDC->addDeclInternal(D2); if (D->isAnonymousStructOrUnion()) D2->setAnonymousStructOrUnion(true); + if (PrevDecl) { + // FIXME: do this for all Redeclarables, not just RecordDecls. + D2->setPreviousDecl(PrevDecl); + } } Importer.Imported(D, D2); Modified: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp?rev=302975&r1=302974&r2=302975&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp (original) +++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp Fri May 12 19:46:33 2017 @@ -855,6 +855,11 @@ static bool IsStructurallyEquivalent(Str if (CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(D1)) { if (CXXRecordDecl *D2CXX = dyn_cast<CXXRecordDecl>(D2)) { + if (D1CXX->hasExternalLexicalStorage() && + !D1CXX->isCompleteDefinition()) { + D1CXX->getASTContext().getExternalSource()->CompleteType(D1CXX); + } + if (D1CXX->getNumBases() != D2CXX->getNumBases()) { if (Context.Complain) { Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent) Modified: cfe/trunk/lib/AST/ExternalASTMerger.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTMerger.cpp?rev=302975&r1=302974&r2=302975&view=diff ============================================================================== --- cfe/trunk/lib/AST/ExternalASTMerger.cpp (original) +++ cfe/trunk/lib/AST/ExternalASTMerger.cpp Fri May 12 19:46:33 2017 @@ -178,3 +178,9 @@ void ExternalASTMerger::FindExternalLexi } }); } + +void ExternalASTMerger::CompleteType(TagDecl *Tag) { + SmallVector<Decl *, 0> Result; + FindExternalLexicalDecls(Tag, [](Decl::Kind) { return true; }, Result); + Tag->setHasExternalLexicalStorage(false); +} Added: cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp?rev=302975&view=auto ============================================================================== --- cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp (added) +++ cfe/trunk/test/Import/conflicting-struct/Inputs/S1.cpp Fri May 12 19:46:33 2017 @@ -0,0 +1,6 @@ +class T; + +class S { + T *t; + int a; +}; Added: cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp?rev=302975&view=auto ============================================================================== --- cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp (added) +++ cfe/trunk/test/Import/conflicting-struct/Inputs/S2.cpp Fri May 12 19:46:33 2017 @@ -0,0 +1,7 @@ +class U { + int b; +}; + +class T { + U u; +}; Added: cfe/trunk/test/Import/conflicting-struct/test.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/conflicting-struct/test.cpp?rev=302975&view=auto ============================================================================== --- cfe/trunk/test/Import/conflicting-struct/test.cpp (added) +++ cfe/trunk/test/Import/conflicting-struct/test.cpp Fri May 12 19:46:33 2017 @@ -0,0 +1,7 @@ +// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s +void expr() { + S MyS; + T MyT; + MyS.a = 3; + MyT.u.b = 2; +} Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=302975&r1=302974&r2=302975&view=diff ============================================================================== --- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original) +++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Fri May 12 19:46:33 2017 @@ -42,6 +42,10 @@ static llvm::cl::list<std::string> Imports("import", llvm::cl::ZeroOrMore, llvm::cl::desc("Path to a file containing declarations to import")); +static llvm::cl::opt<bool> + Direct("direct", llvm::cl::Optional, + llvm::cl::desc("Use the parsed declarations without indirection")); + static llvm::cl::list<std::string> ClangArgs("Xcc", llvm::cl::ZeroOrMore, llvm::cl::desc("Argument to pass to the CompilerInvocation"), @@ -172,6 +176,14 @@ BuildCompilerInstance(ArrayRef<const cha return Ins; } +std::unique_ptr<CompilerInstance> +BuildCompilerInstance(ArrayRef<std::string> ClangArgs) { + std::vector<const char *> ClangArgv(ClangArgs.size()); + std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(), + [](const std::string &s) -> const char * { return s.data(); }); + return init_convenience::BuildCompilerInstance(ClangArgv); +} + std::unique_ptr<ASTContext> BuildASTContext(CompilerInstance &CI, SelectorTable &ST, Builtin::Context &BC) { auto AST = llvm::make_unique<ASTContext>( @@ -205,6 +217,21 @@ void AddExternalSource( CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage(); } +std::unique_ptr<CompilerInstance> BuildIndirect(std::unique_ptr<CompilerInstance> &CI) { + std::vector<const char *> ClangArgv(ClangArgs.size()); + std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(), + [](const std::string &s) -> const char * { return s.data(); }); + std::unique_ptr<CompilerInstance> IndirectCI = + init_convenience::BuildCompilerInstance(ClangArgv); + auto ST = llvm::make_unique<SelectorTable>(); + auto BC = llvm::make_unique<Builtin::Context>(); + std::unique_ptr<ASTContext> AST = + init_convenience::BuildASTContext(*IndirectCI, *ST, *BC); + IndirectCI->setASTContext(AST.release()); + AddExternalSource(*IndirectCI, CI); + return IndirectCI; +} + llvm::Error ParseSource(const std::string &Path, CompilerInstance &CI, CodeGenerator &CG) { SourceManager &SM = CI.getSourceManager(); @@ -231,7 +258,8 @@ Parse(const std::string &Path, std::unique_ptr<ASTContext> AST = init_convenience::BuildASTContext(*CI, *ST, *BC); CI->setASTContext(AST.release()); - AddExternalSource(*CI, Imports); + if (Imports.size()) + AddExternalSource(*CI, Imports); auto LLVMCtx = llvm::make_unique<llvm::LLVMContext>(); std::unique_ptr<CodeGenerator> CG = @@ -268,8 +296,21 @@ int main(int argc, const char **argv) { ImportCIs.push_back(std::move(*ImportCI)); } } + std::vector<std::unique_ptr<CompilerInstance>> IndirectCIs; + if (!Direct) { + for (auto &ImportCI : ImportCIs) { + llvm::Expected<std::unique_ptr<CompilerInstance>> IndirectCI = + BuildIndirect(ImportCI); + if (auto E = IndirectCI.takeError()) { + llvm::errs() << llvm::toString(std::move(E)); + exit(-1); + } else { + IndirectCIs.push_back(std::move(*IndirectCI)); + } + } + } llvm::Expected<std::unique_ptr<CompilerInstance>> ExpressionCI = - Parse(Expression, ImportCIs); + Parse(Expression, Direct ? ImportCIs : IndirectCIs); if (auto E = ExpressionCI.takeError()) { llvm::errs() << llvm::toString(std::move(E)); exit(-1); @@ -277,3 +318,4 @@ int main(int argc, const char **argv) { return 0; } } + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits