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 @@ -1046,6 +1046,17 @@ Importer.Import(From); } +static void setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To, + ASTImporter &Importer) { + if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) { + auto *ToTypedef = + cast_or_null(Importer.Import(FromTypedef)); + assert (ToTypedef && "Failed to import typedef of an anonymous structure"); + + To->setTypedefNameForAnonDecl(ToTypedef); + } +} + bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To, ImportDefinitionKind Kind) { if (To->getDefinition() || To->isBeingDefined()) { @@ -1056,6 +1067,8 @@ } To->startDefinition(); + + setTypedefNameForAnonDecl(From, To, Importer); // Add base classes. if (CXXRecordDecl *ToCXX = dyn_cast(To)) { @@ -1187,6 +1200,8 @@ To->startDefinition(); + setTypedefNameForAnonDecl(From, To, Importer); + QualType T = Importer.Import(Importer.getFromContext().getTypeDeclType(From)); if (T.isNull()) return true; @@ -1668,6 +1683,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,29 +6594,7 @@ // Record the imported declaration. ImportedDecls[FromD] = ToD; - - 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; - } - } - } - } - + return ToD; } Index: test/Analysis/Inputs/ctu-other.cpp =================================================================== --- test/Analysis/Inputs/ctu-other.cpp +++ test/Analysis/Inputs/ctu-other.cpp @@ -65,3 +65,6 @@ return chf2(x); } } + +typedef struct { int n; } Anonymous; +int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; } Index: test/Analysis/Inputs/externalFnMap.txt =================================================================== --- test/Analysis/Inputs/externalFnMap.txt +++ test/Analysis/Inputs/externalFnMap.txt @@ -11,3 +11,4 @@ c:@F@h_chain#I# ctu-chain.cpp.ast c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast c:@N@chns@F@chf2#I# ctu-chain.cpp.ast +c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast Index: test/Analysis/ctu-main.cpp =================================================================== --- test/Analysis/ctu-main.cpp +++ test/Analysis/ctu-main.cpp @@ -40,6 +40,8 @@ int chf1(int x); } +int fun_using_anon_struct(int); + int main() { clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}} clang_analyzer_eval(f(4) == 3); // expected-warning{{TRUE}} @@ -55,4 +57,5 @@ clang_analyzer_eval(mycls::embed_cls2().fecl2(0) == -11); // expected-warning{{TRUE}} clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}} + clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}} } Index: unittests/AST/ASTImporterTest.cpp =================================================================== --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -20,10 +20,15 @@ #include "DeclMatcher.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; @@ -70,22 +75,61 @@ // Creates a virtual file and assigns that to the context of given AST. If the // file already exists then the file will not be created again as a duplicate. -static void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName, - const std::string &Code) { +static void +createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName, + std::unique_ptr &&Buffer) { assert(ToAST); ASTContext &ToCtx = ToAST->getASTContext(); auto *OFS = static_cast( ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get()); auto *MFS = static_cast(OFS->overlays_begin()->get()); - MFS->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str())); + MFS->addFile(FileName, 0, std::move(Buffer)); +} + +static void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName, + const std::string &Code) { + return createVirtualFileIfNeeded( + ToAST, FileName, llvm::MemoryBuffer::getMemBuffer(Code.c_str())); } -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. + StringRef FromFileName = From->getMainFileName(); + createVirtualFileIfNeeded(To, FromFileName, + 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"; @@ -97,46 +141,47 @@ ASTContext &FromCtx = FromAST->getASTContext(), &ToCtx = ToAST->getASTContext(); - // Add input.cc to virtual file system so importer can 'find' it - // while importing SourceLocations. - createVirtualFileIfNeeded(ToAST.get(), InputFileName, 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, @@ -149,8 +194,6 @@ Verifier, AMatcher)); } -const StringRef DeclToImportID = "declToImport"; - // This class provides generic methods to write tests which can check internal // attributes of AST nodes like getPreviousDecl(), isVirtual(), etc. Also, // this fixture makes it possible to import from several "From" contexts. @@ -297,16 +340,124 @@ } }; -AST_MATCHER_P(RecordDecl, hasFieldOrder, std::vector, Order) { - size_t Index = 0; - for (FieldDecl *Field : Node.fields()) { - if (Index == Order.size()) - return false; - if (Field->getName() != Order[Index]) - return false; - ++Index; + +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))); } - return Index == Order.size(); } TEST(ImportExpr, ImportStringLiteral) { @@ -1127,6 +1278,18 @@ MatchVerifier{}.match(To->getTranslationUnitDecl(), Pattern)); } +AST_MATCHER_P(RecordDecl, hasFieldOrder, std::vector, Order) { + size_t Index = 0; + for (FieldDecl *Field : Node.fields()) { + if (Index == Order.size()) + return false; + if (Field->getName() != Order[Index]) + return false; + ++Index; + } + return Index == Order.size(); +} + TEST_P(ASTImporterTestBase, TUshouldContainClassTemplateSpecializationOfExplicitInstantiation) { Decl *From, *To; @@ -1503,5 +1666,51 @@ ParameterizedTests, ImportFunctions, ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); +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