Index: include/clang/AST/ASTImporter.h =================================================================== --- include/clang/AST/ASTImporter.h +++ include/clang/AST/ASTImporter.h @@ -76,10 +76,6 @@ /// the "from" source manager to the corresponding CXXBasesSpecifier /// in the "to" source manager. ImportedCXXBaseSpecifierMap ImportedCXXBaseSpecifiers; - - /// \brief Imported, anonymous tag declarations that are missing their - /// corresponding typedefs. - SmallVector AnonTagsWithPendingTypedefs; /// \brief Declaration (from, to) pairs that are known not to be equivalent /// (which we have already complained about). @@ -129,6 +125,9 @@ /// \returns the equivalent declaration in the "to" context, or a NULL type /// if an error occurred. Decl *Import(Decl *FromD); + Decl *Import(const Decl *FromD) { + return Import(const_cast(FromD)); + } /// \brief Return the copy of the given declaration in the "to" context if /// it has already been imported from the "from" context. Otherwise return Index: lib/AST/ASTImporter.cpp =================================================================== --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -1664,6 +1664,11 @@ if (T.isNull()) return nullptr; + // Some nodes (like anonymous tags referred by typedefs) are allowed to + // import their enclosing typedef directly. Check if this is the case. + if (Decl *AlreadyImported = Importer.GetAlreadyImportedOrNull(D)) + return AlreadyImported; + // Create the new typedef node. TypeSourceInfo *TInfo = Importer.Import(D->getTypeSourceInfo()); SourceLocation StartL = Importer.Import(D->getLocStart()); @@ -6574,23 +6579,13 @@ if (TagDecl *FromTag = dyn_cast(FromD)) { // Keep track of anonymous tags that have an associated typedef. - if (FromTag->getTypedefNameForAnonDecl()) - AnonTagsWithPendingTypedefs.push_back(FromTag); - } else if (TypedefNameDecl *FromTypedef = dyn_cast(FromD)) { - // When we've finished transforming a typedef, see whether it was the - // typedef for an anonymous tag. - for (SmallVectorImpl::iterator - FromTag = AnonTagsWithPendingTypedefs.begin(), - FromTagEnd = AnonTagsWithPendingTypedefs.end(); - FromTag != FromTagEnd; ++FromTag) { - if ((*FromTag)->getTypedefNameForAnonDecl() == FromTypedef) { - if (TagDecl *ToTag = cast_or_null(Import(*FromTag))) { - // We found the typedef for an anonymous tag; link them. - ToTag->setTypedefNameForAnonDecl(cast(ToD)); - AnonTagsWithPendingTypedefs.erase(FromTag); - break; - } - } + if (TypedefNameDecl *AnonTypedef = FromTag->getTypedefNameForAnonDecl()) { + auto *ToTypedef = cast_or_null(Import(AnonTypedef)); + if (!ToTypedef) + return nullptr; + + TagDecl *ToTag = cast(ToD); + ToTag->setTypedefNameForAnonDecl(ToTypedef); } } Index: unittests/AST/ASTImporterTest.cpp =================================================================== --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -18,10 +18,15 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/Tooling.h" #include "gtest/gtest.h" +#include "llvm/ADT/StringMap.h" namespace clang { namespace ast_matchers { +using internal::Matcher; +using internal::BindableMatcher; +using llvm::StringMap; + typedef std::vector ArgVector; typedef std::vector RunOptions; @@ -61,11 +66,48 @@ return {BasicArgs}; } -template +template +NodeType importNode(ASTUnit *From, ASTUnit *To, ASTImporter &Importer, + NodeType Node) { + ASTContext &ToCtx = To->getASTContext(); + + // Add 'From' file to virtual file system so importer can 'find' it + // while importing SourceLocations. It is safe to add same file multiple + // times - it just isn't replaced. + vfs::OverlayFileSystem *OFS = static_cast( + ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get()); + vfs::InMemoryFileSystem *MFS = + static_cast(OFS->overlays_begin()->get()); + + StringRef FromFileName = From->getMainFileName(); + MFS->addFile(FromFileName, 0, From->getBufferForFile(FromFileName)); + + auto Imported = Importer.Import(Node); + + // This should dump source locations and assert if some source locations + // were not imported. + SmallString<1024> ImportChecker; + llvm::raw_svector_ostream ToNothing(ImportChecker); + ToCtx.getTranslationUnitDecl()->print(ToNothing); + + // This traverses the AST to catch certain bugs like poorly or not + // implemented subtrees. + Imported->dump(ToNothing); + + return Imported; +} + + +const StringRef DeclToImportID = "declToImport"; +const StringRef DeclToVerifyID = "declToVerify"; + +template testing::AssertionResult testImport(const std::string &FromCode, const ArgVector &FromArgs, const std::string &ToCode, const ArgVector &ToArgs, - MatchVerifier &Verifier, const MatcherType &AMatcher) { + MatchVerifier &Verifier, + const BindableMatcher &SearchMatcher, + const BindableMatcher &VerificationMatcher) { const char *const InputFileName = "input.cc"; const char *const OutputFileName = "output.cc"; @@ -77,50 +119,47 @@ ASTContext &FromCtx = FromAST->getASTContext(), &ToCtx = ToAST->getASTContext(); - // Add input.cc to virtual file system so importer can 'find' it - // while importing SourceLocations. - vfs::OverlayFileSystem *OFS = static_cast( - ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get()); - vfs::InMemoryFileSystem *MFS = static_cast( - OFS->overlays_begin()->get()); - MFS->addFile(InputFileName, 0, llvm::MemoryBuffer::getMemBuffer(FromCode)); - ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx, FromAST->getFileManager(), false); - IdentifierInfo *ImportedII = &FromCtx.Idents.get("declToImport"); - assert(ImportedII && "Declaration with 'declToImport' name" - "should be specified in test!"); - DeclarationName ImportDeclName(ImportedII); - SmallVector FoundDecls; - FromCtx.getTranslationUnitDecl()->localUncachedLookup( - ImportDeclName, FoundDecls); + auto FoundNodes = match(SearchMatcher, FromCtx); + if (FoundNodes.size() != 1) + return testing::AssertionFailure() + << "Multiple potential nodes were found!"; - if (FoundDecls.size() != 1) - return testing::AssertionFailure() << "Multiple declarations were found!"; + auto ToImport = selectFirst(DeclToImportID, FoundNodes); + if (!ToImport) + return testing::AssertionFailure() << "Node type mismatch!"; // Sanity check: the node being imported should match in the same way as // the result node. - EXPECT_TRUE(Verifier.match(FoundDecls.front(), AMatcher)); + BindableMatcher WrapperMatcher(VerificationMatcher); + EXPECT_TRUE(Verifier.match(ToImport, WrapperMatcher)); - auto Imported = Importer.Import(FoundDecls.front()); + auto Imported = importNode(FromAST.get(), ToAST.get(), Importer, ToImport); if (!Imported) return testing::AssertionFailure() << "Import failed, nullptr returned!"; - // This should dump source locations and assert if some source locations - // were not imported. - SmallString<1024> ImportChecker; - llvm::raw_svector_ostream ToNothing(ImportChecker); - ToCtx.getTranslationUnitDecl()->print(ToNothing); - - // This traverses the AST to catch certain bugs like poorly or not - // implemented subtrees. - Imported->dump(ToNothing); - - return Verifier.match(Imported, AMatcher); + return Verifier.match(Imported, WrapperMatcher); } -template +template +testing::AssertionResult +testImport(const std::string &FromCode, const ArgVector &FromArgs, + const std::string &ToCode, const ArgVector &ToArgs, + MatchVerifier &Verifier, + const BindableMatcher &VerificationMatcher) { + return testImport( + FromCode, FromArgs, ToCode, ToArgs, Verifier, + translationUnitDecl( + has(namedDecl(hasName(DeclToImportID)).bind(DeclToImportID))), + VerificationMatcher); +} + +/// Test how AST node named "declToImport" located in the translation unit +/// of "FromCode" virtual file is imported to "ToCode" virtual file. +/// The verification is done by running AMatcher over the imported node. +template void testImport(const std::string &FromCode, Language FromLang, const std::string &ToCode, Language ToLang, MatchVerifier &Verifier, @@ -133,6 +172,124 @@ Verifier, AMatcher)); } +struct ImportAction { + StringRef FromFilename; + StringRef ToFilename; + // FIXME: Generalize this to support other node kinds. + BindableMatcher ImportPredicate; + + ImportAction(StringRef FromFilename, StringRef ToFilename, + DeclarationMatcher ImportPredicate) + : FromFilename(FromFilename), ToFilename(ToFilename), + ImportPredicate(ImportPredicate) {} + + ImportAction(StringRef FromFilename, StringRef ToFilename, + const std::string &DeclName) + : FromFilename(FromFilename), ToFilename(ToFilename), + ImportPredicate(namedDecl(hasName(DeclName))) {} +}; + +using SingleASTUnitForAllOpts = std::vector>; +using AllASTUnitsForAllOpts = StringMap; + +struct CodeEntry { + std::string CodeSample; + Language Lang; + + /// Builds N copies of ASTUnits for each potential compile options set + /// for further import actions. N is equal to size of this option set. + SingleASTUnitForAllOpts createASTUnits(StringRef FileName) const { + auto RunOpts = getRunOptionsForLanguage(Lang); + size_t NumOpts = RunOpts.size(); + SingleASTUnitForAllOpts ResultASTs(NumOpts); + for (size_t CompileOpt = 0; CompileOpt < NumOpts; ++CompileOpt) { + auto AST = tooling::buildASTFromCodeWithArgs( + CodeSample, RunOpts[CompileOpt], FileName); + EXPECT_TRUE(AST.get()); + ResultASTs[CompileOpt] = std::move(AST); + } + return ResultASTs; + } +}; + +using CodeFiles = StringMap; + +/// Test an arbitrary sequence of imports for a set of given in-memory files. +/// The verification is done by running VerificationMatcher against a specified +/// AST node inside of one of given files. +/// \param CodeSamples Map whose key is the file name and the value is the file +/// content. +/// \param ImportActions Sequence of imports. Each import in sequence +/// specifies "from file" and "to file" and a matcher that is used for +/// searching a declaration for import in "from file". +/// \param FileForFinalCheck Name of virtual file for which the final check is +/// applied. +/// \param FinalSelectPredicate Matcher that specifies the AST node in the +/// FileForFinalCheck for which the verification will be done. +/// \param VerificationMatcher Matcher that will be used for verification after +/// all imports in sequence are done. +void testImportSequence(const CodeFiles &CodeSamples, + const std::vector &ImportActions, + StringRef FileForFinalCheck, + BindableMatcher FinalSelectPredicate, + BindableMatcher VerificationMatcher) { + AllASTUnitsForAllOpts AllASTUnits; + StringMap BuiltASTs; + using ImporterKey = std::pair; + llvm::DenseMap> Importers; + + auto GenASTsIfNeeded = [&AllASTUnits, &BuiltASTs, + &CodeSamples](StringRef Filename) { + if (BuiltASTs.find(Filename) == BuiltASTs.end()) { + auto Found = CodeSamples.find(Filename); + assert(Found != CodeSamples.end() && "Wrong file for import!"); + AllASTUnits[Filename] = Found->getValue().createASTUnits(Filename); + BuiltASTs[Filename] = true; + } + }; + + size_t NumCompileOpts = 0; + for (const ImportAction &Action : ImportActions) { + StringRef FromFile = Action.FromFilename, ToFile = Action.ToFilename; + GenASTsIfNeeded(FromFile); + GenASTsIfNeeded(ToFile); + NumCompileOpts = AllASTUnits[FromFile].size(); + + for (size_t CompileOpt = 0; CompileOpt < NumCompileOpts; ++CompileOpt) { + ASTUnit *From = AllASTUnits[FromFile][CompileOpt].get(); + ASTUnit *To = AllASTUnits[ToFile][CompileOpt].get(); + + // Create a new importer if needed. + std::unique_ptr &ImporterRef = Importers[{From, To}]; + if (!ImporterRef) + ImporterRef.reset(new ASTImporter( + To->getASTContext(), To->getFileManager(), From->getASTContext(), + From->getFileManager(), false)); + + // Find the declaration and import it. + auto FoundDecl = match(Action.ImportPredicate.bind(DeclToImportID), + From->getASTContext()); + EXPECT_TRUE(FoundDecl.size() == 1); + const Decl *ToImport = selectFirst(DeclToImportID, FoundDecl); + auto Imported = importNode(From, To, *ImporterRef, ToImport); + EXPECT_TRUE(Imported); + } + } + + // NOTE: We don't do cross-option import check here due to fast growth of + // potential option sets. + for (size_t CompileOpt = 0; CompileOpt < NumCompileOpts; ++CompileOpt) { + // Find the declaration and import it. + auto FoundDecl = + match(FinalSelectPredicate.bind(DeclToVerifyID), + AllASTUnits[FileForFinalCheck][CompileOpt]->getASTContext()); + EXPECT_TRUE(FoundDecl.size() == 1); + const Decl *ToVerify = selectFirst(DeclToVerifyID, FoundDecl); + MatchVerifier Verifier; + EXPECT_TRUE(Verifier.match(ToVerify, + BindableMatcher(VerificationMatcher))); + } +} TEST(ImportExpr, ImportStringLiteral) { MatchVerifier Verifier; @@ -816,5 +973,51 @@ has(fieldDecl(hasType(dependentSizedArrayType()))))))); } +AST_MATCHER_P(TagDecl, hasTypedefForAnonDecl, Matcher, + InnerMatcher) { + if (auto *Typedef = Node.getTypedefNameForAnonDecl()) + return InnerMatcher.matches(*Typedef, Finder, Builder); + return false; +} + +TEST(ImportDecl, ImportEnumSequential) { + CodeFiles Samples{{"main.c", + {"void foo();" + "void moo();" + "int main() { foo(); moo();}", + Lang_C}}, + + {"foo.c", + {"typedef enum { THING_VALUE } thing_t;" + "void conflict(thing_t type);" + "void foo() { (void)THING_VALUE; }" + "void conflict(thing_t type) {}", + Lang_C}}, + + {"moo.c", + {"typedef enum { THING_VALUE } thing_t;" + "void conflict(thing_t type);" + "void moo() { conflict(THING_VALUE); }", + Lang_C}}}; + + auto VerificationMatcher = + enumDecl(has(enumConstantDecl(hasName("THING_VALUE"))), + hasTypedefForAnonDecl(hasName("thing_t"))); + + ImportAction ImportFoo{"foo.c", "main.c", functionDecl(hasName("foo"))}, + ImportMoo{"moo.c", "main.c", functionDecl(hasName("moo"))}; + + testImportSequence( + Samples, {ImportFoo, ImportMoo}, // "foo", them "moo". + // Just check that there is only one enum decl in the result AST. + "main.c", enumDecl(), VerificationMatcher); + + // For different import order, result should be the same. + testImportSequence( + Samples, {ImportMoo, ImportFoo}, // "moo", them "foo". + // Check that there is only one enum decl in the result AST. + "main.c", enumDecl(), VerificationMatcher); +} + } // end namespace ast_matchers } // end namespace clang