From: Sean Callanan Date: Wed, 27 Sep 2017 19:57:58 +0000 (+0000) Subject: Add support for remembering origins to ExternalASTMerger X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=87d985aec94671ac64148e973c6aa3d5090b2ec0;p=clang Add support for remembering origins to ExternalASTMerger ExternalASTMerger has hitherto relied on being able to look up any Decl through its named DeclContext chain. This works for many cases, but causes problems for function-local structs, which cannot be looked up in their containing FunctionDecl. An example case is void f() { { struct S { int a; }; } { struct S { bool b; }; } } It is not possible to lookup either of the two Ses individually (or even to provide enough information to disambiguate) after parsing is over; and there is typically no need to, since they are invisible to the outside world. However, ExternalASTMerger needs to be able to complete either S on demand. This led to an XFAIL on test/Import/local-struct, which this patch removes. The way the patch works is: It defines a new data structure, ExternalASTMerger::OriginMap, which clients are expected to maintain (default-constructing if the origin does not have an ExternalASTMerger servicing it) As DeclContexts are imported, if they cannot be looked up by name they are placed in the OriginMap. This allows ExternalASTMerger to complete them later if necessary. As DeclContexts are imported from an origin that already has its own OriginMap, the origins are forwarded – but only for those DeclContexts that are actually used. This keeps the amount of stored data minimal. The patch also applies several improvements from review: - Thoroughly documents the interface to ExternalASTMerger; - Adds optional logging to help track what's going on; and - Cleans up a bunch of braces and dangling elses. Differential Revision: https://reviews.llvm.org/D38208 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314336 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/ExternalASTMerger.h b/include/clang/AST/ExternalASTMerger.h index 51d0c30ad2..81492aec6e 100644 --- a/include/clang/AST/ExternalASTMerger.h +++ b/include/clang/AST/ExternalASTMerger.h @@ -16,34 +16,159 @@ #include "clang/AST/ASTImporter.h" #include "clang/AST/ExternalASTSource.h" +#include "llvm/Support/raw_ostream.h" namespace clang { +/// ExternalASTSource implementation that merges information from several +/// ASTContexts. +/// +/// ExtermalASTMerger maintains a vector of ASTImporters that it uses to import +/// (potentially incomplete) Decls and DeclContexts from the source ASTContexts +/// in response to ExternalASTSource API calls. +/// +/// When lookup occurs in the resulting imported DeclContexts, the original +/// DeclContexts need to be queried. Roughly, there are three cases here: +/// +/// - The DeclContext of origin can be found by simple name lookup. In this +/// case, no additional state is required. +/// +/// - The DeclContext of origin is different from what would be found by name +/// lookup. In this case, Origins contains an entry overriding lookup and +/// specifying the correct pair of DeclContext/ASTContext. +/// +/// - The DeclContext of origin was determined by another ExterenalASTMerger. +/// (This is possible when the source ASTContext for one of the Importers has +/// its own ExternalASTMerger). The origin must be properly forwarded in this +/// case. +/// +/// ExternalASTMerger's job is to maintain the data structures necessary to +/// allow this. The data structures themselves can be extracted (read-only) and +/// copied for re-use. class ExternalASTMerger : public ExternalASTSource { public: - struct ImporterPair { - std::unique_ptr Forward; - std::unique_ptr Reverse; + /// A single origin for a DeclContext. Unlike Decls, DeclContexts do + /// not allow their containing ASTContext to be determined in all cases. + struct DCOrigin { + DeclContext *DC; + ASTContext *AST; }; + typedef std::map OriginMap; + typedef std::vector> ImporterVector; private: - std::vector Importers; + /// One importer exists for each source. + ImporterVector Importers; + /// Overrides in case name lookup would return nothing or would return + /// the wrong thing. + OriginMap Origins; + /// The installed log stream. + llvm::raw_ostream *LogStream; public: - struct ImporterEndpoint { + /// The target for an ExternalASTMerger. + /// + /// ASTImporters require both ASTContext and FileManager to be able to + /// import SourceLocations properly. + struct ImporterTarget { ASTContext &AST; FileManager &FM; }; - ExternalASTMerger(const ImporterEndpoint &Target, - llvm::ArrayRef Sources); + /// A source for an ExternalASTMerger. + /// + /// ASTImporters require both ASTContext and FileManager to be able to + /// import SourceLocations properly. Additionally, when import occurs for + /// a DeclContext whose origin has been overridden, then this + /// ExternalASTMerger must be able to determine that. + struct ImporterSource { + ASTContext &AST; + FileManager &FM; + const OriginMap &OM; + }; +private: + /// The target for this ExtenralASTMerger. + ImporterTarget Target; + +public: + ExternalASTMerger(const ImporterTarget &Target, + llvm::ArrayRef Sources); + + /// Add a set of ASTContexts as possible origins. + /// + /// Usually the set will be initialized in the constructor, but long-lived + /// ExternalASTMergers may neeed to import from new sources (for example, + /// newly-parsed source files). + /// + /// Ensures that Importers does not gain duplicate entries as a result. + void AddSources(llvm::ArrayRef Sources); + + /// Remove a set of ASTContexts as possible origins. + /// + /// Sometimes an origin goes away (for example, if a source file gets + /// superseded by a newer version). + /// + /// The caller is responsible for ensuring that this doesn't leave + /// DeclContexts that can't be completed. + void RemoveSources(llvm::ArrayRef Sources); + + /// Implementation of the ExternalASTSource API. bool FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) override; + /// Implementation of the ExternalASTSource API. void FindExternalLexicalDecls(const DeclContext *DC, llvm::function_ref IsKindWeWant, SmallVectorImpl &Result) override; + + /// Implementation of the ExternalASTSource API. + void CompleteType(TagDecl *Tag) override; + + /// Implementation of the ExternalASTSource API. + void CompleteType(ObjCInterfaceDecl *Interface) override; + + /// Returns true if DC can be found in any source AST context. + bool CanComplete(DeclContext *DC); + + /// Records an origin in Origins only if name lookup would find + /// something different or nothing at all. + void MaybeRecordOrigin(const DeclContext *ToDC, DCOrigin Origin); + + /// Regardless of any checks, override the Origin for a DeclContext. + void ForceRecordOrigin(const DeclContext *ToDC, DCOrigin Origin); + + /// Get a read-only view of the Origins map, for use in constructing + /// an ImporterSource for another ExternalASTMerger. + const OriginMap &GetOrigins() { return Origins; } + + /// Returns true if Importers contains an ASTImporter whose source is + /// OriginContext. + bool HasImporterForOrigin(ASTContext &OriginContext); + + /// Returns a reference to the ASTRImporter from Importers whose origin + /// is OriginContext. This allows manual import of ASTs while preserving the + /// OriginMap correctly. + ASTImporter &ImporterForOrigin(ASTContext &OriginContext); + + /// Sets the current log stream. + void SetLogStream(llvm::raw_string_ostream &Stream) { LogStream = &Stream; } +private: + /// Records and origin in Origins. + void RecordOriginImpl(const DeclContext *ToDC, DCOrigin Origin, + ASTImporter &importer); + + /// Performs an action for every DeclContext that is identified as + /// corresponding (either by forced origin or by name lookup) to DC. + template + void ForEachMatchingDC(const DeclContext *DC, CallbackType Callback); + +public: + /// Log something if there is a logging callback installed. + llvm::raw_ostream &logs() { return *LogStream; } + + /// True if the log stream is not llvm::nulls(); + bool LoggingEnabled() { return LogStream != &llvm::nulls(); } }; } // end namespace clang diff --git a/lib/AST/ExternalASTMerger.cpp b/lib/AST/ExternalASTMerger.cpp index f69b5d6284..ec2e3034b1 100644 --- a/lib/AST/ExternalASTMerger.cpp +++ b/lib/AST/ExternalASTMerger.cpp @@ -14,6 +14,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ExternalASTMerger.h" @@ -32,29 +33,18 @@ template struct Source { typedef std::pair, ASTImporter *> Candidate; -class LazyASTImporter : public ASTImporter { -public: - LazyASTImporter(ASTContext &ToContext, FileManager &ToFileManager, - ASTContext &FromContext, FileManager &FromFileManager) - : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager, - /*MinimalImport=*/true) {} - Decl *Imported(Decl *From, Decl *To) override { - if (auto ToTag = dyn_cast(To)) { - ToTag->setHasExternalLexicalStorage(); - ToTag->setMustBuildLookupTable(); - } else if (auto ToNamespace = dyn_cast(To)) { - ToNamespace->setHasExternalVisibleStorage(); - } else if (auto ToContainer = dyn_cast(To)) { - ToContainer->setHasExternalLexicalStorage(); - ToContainer->setMustBuildLookupTable(); - } - return ASTImporter::Imported(From, To); - } -}; +/// For the given DC, return the DC that is safe to perform lookups on. This is +/// the DC we actually want to work with most of the time. +const DeclContext *CanonicalizeDC(const DeclContext *DC) { + if (isa(DC)) + return DC->getRedeclContext(); + return DC; +} Source LookupSameContext(Source SourceTU, const DeclContext *DC, ASTImporter &ReverseImporter) { + DC = CanonicalizeDC(DC); if (DC->isTranslationUnit()) { return SourceTU; } @@ -64,101 +54,328 @@ LookupSameContext(Source SourceTU, const DeclContext *DC, // If we couldn't find the parent DC in this TranslationUnit, give up. return nullptr; } - auto ND = cast(DC); + auto *ND = cast(DC); DeclarationName Name = ND->getDeclName(); Source SourceName = ReverseImporter.Import(Name); DeclContext::lookup_result SearchResult = SourceParentDC.get()->lookup(SourceName.get()); size_t SearchResultSize = SearchResult.size(); - // Handle multiple candidates once we have a test for it. - // This may turn up when we import template specializations correctly. - assert(SearchResultSize < 2); - if (SearchResultSize == 0) { - // couldn't find the name, so we have to give up + if (SearchResultSize == 0 || SearchResultSize > 1) { + // There are two cases here. First, we might not find the name. + // We might also find multiple copies, in which case we have no + // guarantee that the one we wanted is the one we pick. (E.g., + // if we have two specializations of the same template it is + // very hard to determine which is the one you want.) + // + // The Origins map fixes this problem by allowing the origin to be + // explicitly recorded, so we trigger that recording by returning + // nothing (rather than a possibly-inaccurate guess) here. return nullptr; } else { NamedDecl *SearchResultDecl = SearchResult[0]; - return dyn_cast(SearchResultDecl); + if (isa(SearchResultDecl) && + SearchResultDecl->getKind() == DC->getDeclKind()) + return cast(SearchResultDecl)->getPrimaryContext(); + return nullptr; // This type of lookup is unsupported } } -bool IsForwardDeclaration(Decl *D) { - if (auto TD = dyn_cast(D)) { - return !TD->isThisDeclarationADefinition(); - } else if (auto FD = dyn_cast(D)) { - return !FD->isThisDeclarationADefinition(); - } else if (auto OID = dyn_cast(D)) { - return OID->isThisDeclarationADefinition(); - } else { - return false; - } -} +/// A custom implementation of ASTImporter, for ExternalASTMerger's purposes. +/// +/// There are several modifications: +/// +/// - It enables lazy lookup (via the HasExternalLexicalStorage flag and a few +/// others), which instructs Clang to refer to ExternalASTMerger. Also, it +/// forces MinimalImport to true, which is necessary to make this work. +/// - It maintains a reverse importer for use with names. This allows lookup of +/// arbitrary names in the source context. +/// - It updates the ExternalASTMerger's origin map as needed whenever a +/// it sees a DeclContext. +class LazyASTImporter : public ASTImporter { +private: + ExternalASTMerger &Parent; + ASTImporter Reverse; + const ExternalASTMerger::OriginMap &FromOrigins; -template -void ForEachMatchingDC( - const DeclContext *DC, - llvm::ArrayRef Importers, - CallbackType Callback) { - for (const ExternalASTMerger::ImporterPair &IP : Importers) { - Source SourceTU = - IP.Forward->getFromContext().getTranslationUnitDecl(); - if (auto SourceDC = LookupSameContext(SourceTU, DC, *IP.Reverse)) - Callback(IP, SourceDC); + llvm::raw_ostream &logs() { return Parent.logs(); } +public: + LazyASTImporter(ExternalASTMerger &_Parent, ASTContext &ToContext, + FileManager &ToFileManager, ASTContext &FromContext, + FileManager &FromFileManager, + const ExternalASTMerger::OriginMap &_FromOrigins) + : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager, + /*MinimalImport=*/true), + Parent(_Parent), Reverse(FromContext, FromFileManager, ToContext, + ToFileManager, /*MinimalImport=*/true), FromOrigins(_FromOrigins) {} + + /// Whenever a DeclContext is imported, ensure that ExternalASTSource's origin + /// map is kept up to date. Also set the appropriate flags. + Decl *Imported(Decl *From, Decl *To) override { + if (auto *ToDC = dyn_cast(To)) { + const bool LoggingEnabled = Parent.LoggingEnabled(); + if (LoggingEnabled) + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " imported (DeclContext*)" << (void*)ToDC + << ", (ASTContext*)" << (void*)&getToContext() + << " from (DeclContext*)" << (void*)llvm::cast(From) + << ", (ASTContext*)" << (void*)&getFromContext() + << "\n"; + Source FromDC( + cast(From)->getPrimaryContext()); + if (FromOrigins.count(FromDC) && + Parent.HasImporterForOrigin(*FromOrigins.at(FromDC).AST)) { + if (LoggingEnabled) + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " forced origin (DeclContext*)" + << (void*)FromOrigins.at(FromDC).DC + << ", (ASTContext*)" + << (void*)FromOrigins.at(FromDC).AST + << "\n"; + Parent.ForceRecordOrigin(ToDC, FromOrigins.at(FromDC)); + } else { + if (LoggingEnabled) + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " maybe recording origin (DeclContext*)" << (void*)FromDC + << ", (ASTContext*)" << (void*)&getFromContext() + << "\n"; + Parent.MaybeRecordOrigin(ToDC, {FromDC, &getFromContext()}); + } + } + if (auto *ToTag = dyn_cast(To)) { + ToTag->setHasExternalLexicalStorage(); + ToTag->setMustBuildLookupTable(); + assert(Parent.CanComplete(ToTag)); + } else if (auto *ToNamespace = dyn_cast(To)) { + ToNamespace->setHasExternalVisibleStorage(); + assert(Parent.CanComplete(ToNamespace)); + } else if (auto *ToContainer = dyn_cast(To)) { + ToContainer->setHasExternalLexicalStorage(); + ToContainer->setMustBuildLookupTable(); + assert(Parent.CanComplete(ToContainer)); + } + return ASTImporter::Imported(From, To); } -} + ASTImporter &GetReverse() { return Reverse; } +}; bool HasDeclOfSameType(llvm::ArrayRef Decls, const Candidate &C) { + if (isa(C.first.get())) + return false; return llvm::any_of(Decls, [&](const Candidate &D) { return C.first.get()->getKind() == D.first.get()->getKind(); }); } + } // end namespace -ExternalASTMerger::ExternalASTMerger(const ImporterEndpoint &Target, - llvm::ArrayRef Sources) { - for (const ImporterEndpoint &S : Sources) { - Importers.push_back( - {llvm::make_unique(Target.AST, Target.FM, S.AST, S.FM), - llvm::make_unique(S.AST, S.FM, Target.AST, Target.FM, - /*MinimalImport=*/true)}); - } +ASTImporter &ExternalASTMerger::ImporterForOrigin(ASTContext &OriginContext) { + for (const std::unique_ptr &I : Importers) + if (&I->getFromContext() == &OriginContext) + return *I; + llvm_unreachable("We should have an importer for this origin!"); } -bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC, - DeclarationName Name) { - llvm::SmallVector Decls; - llvm::SmallVector CompleteDecls; - llvm::SmallVector ForwardDecls; +namespace { +LazyASTImporter &LazyImporterForOrigin(ExternalASTMerger &Merger, + ASTContext &OriginContext) { + return static_cast( + Merger.ImporterForOrigin(OriginContext)); +} +} - auto FilterFoundDecl = [&CompleteDecls, &ForwardDecls](const Candidate &C) { - if (IsForwardDeclaration(C.first.get())) { - if (!HasDeclOfSameType(ForwardDecls, C)) { - ForwardDecls.push_back(C); +bool ExternalASTMerger::HasImporterForOrigin(ASTContext &OriginContext) { + for (const std::unique_ptr &I : Importers) + if (&I->getFromContext() == &OriginContext) + return true; + return false; +} + +template +void ExternalASTMerger::ForEachMatchingDC(const DeclContext *DC, + CallbackType Callback) { + if (Origins.count(DC)) { + ExternalASTMerger::DCOrigin Origin = Origins[DC]; + LazyASTImporter &Importer = LazyImporterForOrigin(*this, *Origin.AST); + Callback(Importer, Importer.GetReverse(), Origin.DC); + } else { + bool DidCallback = false; + for (const std::unique_ptr &Importer : Importers) { + Source SourceTU = + Importer->getFromContext().getTranslationUnitDecl(); + ASTImporter &Reverse = + static_cast(Importer.get())->GetReverse(); + if (auto SourceDC = LookupSameContext(SourceTU, DC, Reverse)) { + DidCallback = true; + if (Callback(*Importer, Reverse, SourceDC)) + break; } - } else { - CompleteDecls.push_back(C); } - }; + if (!DidCallback && LoggingEnabled()) + logs() << "(ExternalASTMerger*)" << (void*)this + << " asserting for (DeclContext*)" << (void*)DC + << ", (ASTContext*)" << (void*)&Target.AST + << "\n"; + assert(DidCallback && "Couldn't find a source context matching our DC"); + } +} + +void ExternalASTMerger::CompleteType(TagDecl *Tag) { + assert(Tag->hasExternalLexicalStorage()); + ForEachMatchingDC(Tag, [&](ASTImporter &Forward, ASTImporter &Reverse, + Source SourceDC) -> bool { + auto *SourceTag = const_cast(cast(SourceDC.get())); + if (SourceTag->hasExternalLexicalStorage()) + SourceTag->getASTContext().getExternalSource()->CompleteType(SourceTag); + if (!SourceTag->getDefinition()) + return false; + Forward.Imported(SourceTag, Tag); + Forward.ImportDefinition(SourceTag); + Tag->setCompleteDefinition(SourceTag->isCompleteDefinition()); + return true; + }); +} +void ExternalASTMerger::CompleteType(ObjCInterfaceDecl *Interface) { + assert(Interface->hasExternalLexicalStorage()); ForEachMatchingDC( - DC, Importers, - [&](const ImporterPair &IP, Source SourceDC) { - DeclarationName FromName = IP.Reverse->Import(Name); - DeclContextLookupResult Result = SourceDC.get()->lookup(FromName); - for (NamedDecl *FromD : Result) { - FilterFoundDecl(std::make_pair(FromD, IP.Forward.get())); - } + Interface, [&](ASTImporter &Forward, ASTImporter &Reverse, + Source SourceDC) -> bool { + auto *SourceInterface = const_cast( + cast(SourceDC.get())); + if (SourceInterface->hasExternalLexicalStorage()) + SourceInterface->getASTContext().getExternalSource()->CompleteType( + SourceInterface); + if (!SourceInterface->getDefinition()) + return false; + Forward.Imported(SourceInterface, Interface); + Forward.ImportDefinition(SourceInterface); + return true; }); +} - llvm::ArrayRef DeclsToReport = - CompleteDecls.empty() ? ForwardDecls : CompleteDecls; +bool ExternalASTMerger::CanComplete(DeclContext *Interface) { + assert(Interface->hasExternalLexicalStorage() || + Interface->hasExternalVisibleStorage()); + bool FoundMatchingDC = false; + ForEachMatchingDC(Interface, + [&](ASTImporter &Forward, ASTImporter &Reverse, + Source SourceDC) -> bool { + FoundMatchingDC = true; + return true; + }); + return FoundMatchingDC; +} - if (DeclsToReport.empty()) { - return false; +namespace { +bool IsSameDC(const DeclContext *D1, const DeclContext *D2) { + if (isa(D1) && isa(D2)) + return true; // There are many cases where Objective-C is ambiguous. + if (auto *T1 = dyn_cast(D1)) + if (auto *T2 = dyn_cast(D2)) + if (T1->getFirstDecl() == T2->getFirstDecl()) + return true; + return D1 == D2 || D1 == CanonicalizeDC(D2); +} +} + +void ExternalASTMerger::MaybeRecordOrigin(const DeclContext *ToDC, + DCOrigin Origin) { + LazyASTImporter &Importer = LazyImporterForOrigin(*this, *Origin.AST); + ASTImporter &Reverse = Importer.GetReverse(); + Source FoundFromDC = + LookupSameContext(Origin.AST->getTranslationUnitDecl(), ToDC, Reverse); + const bool DoRecord = !FoundFromDC || !IsSameDC(FoundFromDC.get(), Origin.DC); + if (DoRecord) + RecordOriginImpl(ToDC, Origin, Importer); + if (LoggingEnabled()) + logs() << "(ExternalASTMerger*)" << (void*)this + << (DoRecord ? " decided " : " decided NOT") + << " to record origin (DeclContext*)" << (void*)Origin.DC + << ", (ASTContext*)" << (void*)&Origin.AST + << "\n"; +} + +void ExternalASTMerger::ForceRecordOrigin(const DeclContext *ToDC, + DCOrigin Origin) { + RecordOriginImpl(ToDC, Origin, ImporterForOrigin(*Origin.AST)); +} + +void ExternalASTMerger::RecordOriginImpl(const DeclContext *ToDC, DCOrigin Origin, + ASTImporter &Importer) { + Origins[ToDC] = Origin; + Importer.ASTImporter::Imported(cast(Origin.DC), const_cast(cast(ToDC))); +} + +ExternalASTMerger::ExternalASTMerger(const ImporterTarget &Target, + llvm::ArrayRef Sources) : LogStream(&llvm::nulls()), Target(Target) { + AddSources(Sources); +} + +void ExternalASTMerger::AddSources(llvm::ArrayRef Sources) { + for (const ImporterSource &S : Sources) { + assert(&S.AST != &Target.AST); + Importers.push_back(llvm::make_unique( + *this, Target.AST, Target.FM, S.AST, S.FM, S.OM)); + } +} + +void ExternalASTMerger::RemoveSources(llvm::ArrayRef Sources) { + if (LoggingEnabled()) + for (const ImporterSource &S : Sources) + logs() << "(ExternalASTMerger*)" << (void*)this + << " removing source (ASTContext*)" << (void*)&S.AST + << "\n"; + Importers.erase( + std::remove_if(Importers.begin(), Importers.end(), + [&Sources](std::unique_ptr &Importer) -> bool { + for (const ImporterSource &S : Sources) { + if (&Importer->getFromContext() == &S.AST) + return true; + } + return false; + }), + Importers.end()); + for (OriginMap::iterator OI = Origins.begin(), OE = Origins.end(); OI != OE; ) { + std::pair Origin = *OI; + bool Erase = false; + for (const ImporterSource &S : Sources) { + if (&S.AST == Origin.second.AST) { + Erase = true; + break; + } + } + if (Erase) + OI = Origins.erase(OI); + else + ++OI; } +} + +bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC, + DeclarationName Name) { + llvm::SmallVector Decls; + llvm::SmallVector Candidates; - Decls.reserve(DeclsToReport.size()); - for (const Candidate &C : DeclsToReport) { + auto FilterFoundDecl = [&Candidates](const Candidate &C) { + if (!HasDeclOfSameType(Candidates, C)) + Candidates.push_back(C); + }; + + ForEachMatchingDC(DC, [&](ASTImporter &Forward, ASTImporter &Reverse, + Source SourceDC) -> bool { + DeclarationName FromName = Reverse.Import(Name); + DeclContextLookupResult Result = SourceDC.get()->lookup(FromName); + for (NamedDecl *FromD : Result) { + FilterFoundDecl(std::make_pair(FromD, &Forward)); + } + return false; + }); + + if (Candidates.empty()) + return false; + + Decls.reserve(Candidates.size()); + for (const Candidate &C : Candidates) { NamedDecl *d = cast(C.second->Import(C.first.get())); assert(d); Decls.push_back(d); @@ -170,17 +387,16 @@ bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC, void ExternalASTMerger::FindExternalLexicalDecls( const DeclContext *DC, llvm::function_ref IsKindWeWant, SmallVectorImpl &Result) { - ForEachMatchingDC( - DC, Importers, - [&](const ImporterPair &IP, Source SourceDC) { - for (const Decl *SourceDecl : SourceDC.get()->decls()) { - if (IsKindWeWant(SourceDecl->getKind())) { - Decl *ImportedDecl = - IP.Forward->Import(const_cast(SourceDecl)); - assert(ImportedDecl->getDeclContext() == DC); - (void)ImportedDecl; - } - } - }); + ForEachMatchingDC(DC, [&](ASTImporter &Forward, ASTImporter &Reverse, + Source SourceDC) -> bool { + for (const Decl *SourceDecl : SourceDC.get()->decls()) { + if (IsKindWeWant(SourceDecl->getKind())) { + Decl *ImportedDecl = Forward.Import(const_cast(SourceDecl)); + assert(!ImportedDecl || IsSameDC(ImportedDecl->getDeclContext(), DC)); + (void)ImportedDecl; + } + } + return false; + }); } diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index f35eef2d4f..d5ec98c8cb 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -7309,11 +7309,15 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, // Give the external AST source a chance to complete the type. if (auto *Source = Context.getExternalSource()) { - if (Tag) - Source->CompleteType(Tag->getDecl()); - else - Source->CompleteType(IFace->getDecl()); - + if (Tag) { + TagDecl *TagD = Tag->getDecl(); + if (TagD->hasExternalLexicalStorage()) + Source->CompleteType(TagD); + } else { + ObjCInterfaceDecl *IFaceD = IFace->getDecl(); + if (IFaceD->hasExternalLexicalStorage()) + Source->CompleteType(IFace->getDecl()); + } // If the external source completed the type, go through the motions // again to ensure we're allowed to use the completed type. if (!T->isIncompleteType()) diff --git a/test/Import/extern-c-function/Inputs/F.cpp b/test/Import/extern-c-function/Inputs/F.cpp new file mode 100644 index 0000000000..c417d56302 --- /dev/null +++ b/test/Import/extern-c-function/Inputs/F.cpp @@ -0,0 +1,3 @@ +extern "C" { + void f(int arg); +} diff --git a/test/Import/extern-c-function/test.cpp b/test/Import/extern-c-function/test.cpp new file mode 100644 index 0000000000..50227cdf9c --- /dev/null +++ b/test/Import/extern-c-function/test.cpp @@ -0,0 +1,4 @@ +// RUN: clang-import-test -import %S/Inputs/F.cpp -expression %s +void expr() { + f(2); +} \ No newline at end of file diff --git a/test/Import/forward-declared-objc-class/Inputs/S1.m b/test/Import/forward-declared-objc-class/Inputs/S1.m new file mode 100644 index 0000000000..b9305d7e89 --- /dev/null +++ b/test/Import/forward-declared-objc-class/Inputs/S1.m @@ -0,0 +1 @@ +@class MyClass; \ No newline at end of file diff --git a/test/Import/forward-declared-objc-class/Inputs/S2.m b/test/Import/forward-declared-objc-class/Inputs/S2.m new file mode 100644 index 0000000000..69d067a3bd --- /dev/null +++ b/test/Import/forward-declared-objc-class/Inputs/S2.m @@ -0,0 +1,6 @@ +@interface MyClass { + int j; +}; ++(MyClass*)fromInteger:(int)_j; +-(int)getInteger; +@end \ No newline at end of file diff --git a/test/Import/forward-declared-objc-class/Inputs/S3.m b/test/Import/forward-declared-objc-class/Inputs/S3.m new file mode 100644 index 0000000000..b9305d7e89 --- /dev/null +++ b/test/Import/forward-declared-objc-class/Inputs/S3.m @@ -0,0 +1 @@ +@class MyClass; \ No newline at end of file diff --git a/test/Import/forward-declared-objc-class/test.m b/test/Import/forward-declared-objc-class/test.m new file mode 100644 index 0000000000..098818be3c --- /dev/null +++ b/test/Import/forward-declared-objc-class/test.m @@ -0,0 +1,6 @@ +// RUN: clang-import-test -x objective-c++ -import %S/Inputs/S1.m --import %S/Inputs/S2.m --import %S/Inputs/S3.m -expression %s +void expr() { + MyClass *c = [MyClass fromInteger:3]; + const int i = [c getInteger]; + const int j = c->j; +} diff --git a/test/Import/forward-declared-struct/Inputs/S3.c b/test/Import/forward-declared-struct/Inputs/S3.c new file mode 100644 index 0000000000..28377c2760 --- /dev/null +++ b/test/Import/forward-declared-struct/Inputs/S3.c @@ -0,0 +1 @@ +struct S; diff --git a/test/Import/forward-declared-struct/test.c b/test/Import/forward-declared-struct/test.c index 7ccdcf9e97..8199aa267d 100644 --- a/test/Import/forward-declared-struct/test.c +++ b/test/Import/forward-declared-struct/test.c @@ -1,4 +1,4 @@ -// RUN: clang-import-test -import %S/Inputs/S1.c --import %S/Inputs/S2.c -expression %s +// RUN: clang-import-test -import %S/Inputs/S1.c --import %S/Inputs/S2.c --import %S/Inputs/S3.c -expression %s void expr() { struct S MyS; MyS.a = 3; diff --git a/test/Import/local-struct-use-origins/Inputs/Callee.cpp b/test/Import/local-struct-use-origins/Inputs/Callee.cpp new file mode 100644 index 0000000000..96cd2f22e4 --- /dev/null +++ b/test/Import/local-struct-use-origins/Inputs/Callee.cpp @@ -0,0 +1,12 @@ +struct Bar { + void bar(int _a, bool _b) { + { + struct S { int a; }; + S s = { _a }; + } + { + struct S { bool b; }; + S t = { _b }; + } + }; +}; diff --git a/test/Import/local-struct-use-origins/test.cpp b/test/Import/local-struct-use-origins/test.cpp new file mode 100644 index 0000000000..abbdbd16d0 --- /dev/null +++ b/test/Import/local-struct-use-origins/test.cpp @@ -0,0 +1,7 @@ +// RUN: clang-import-test -dump-ir -use-origins -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s +// CHECK: %struct.S = type { i +// CHECK: %struct.S.0 = type { i + +void foo() { + return Bar().bar(3, true); +} diff --git a/test/Import/local-struct/test.cpp b/test/Import/local-struct/test.cpp index 8f6e38138f..1e838477a0 100644 --- a/test/Import/local-struct/test.cpp +++ b/test/Import/local-struct/test.cpp @@ -1,7 +1,6 @@ // RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s -// XFAIL: * // CHECK: %struct.S = type { i -// CHECK: %struct.S.0 = type { i1 } +// CHECK: %struct.S.0 = type { i void foo() { return Bar().bar(3, true); diff --git a/test/Import/objc-definitions-in-expression/Inputs/S.m b/test/Import/objc-definitions-in-expression/Inputs/S.m new file mode 100644 index 0000000000..cf8ffaa602 --- /dev/null +++ b/test/Import/objc-definitions-in-expression/Inputs/S.m @@ -0,0 +1,4 @@ +@interface C { +} +-(int)m; +@end diff --git a/test/Import/objc-definitions-in-expression/test.m b/test/Import/objc-definitions-in-expression/test.m new file mode 100644 index 0000000000..0c9984731d --- /dev/null +++ b/test/Import/objc-definitions-in-expression/test.m @@ -0,0 +1,21 @@ +// RUN: clang-import-test -x objective-c++ -import %S/Inputs/S.m -expression %s +@class D; + +@interface B { + int x; + int y; +} +@end + +@interface D : B { + int z; +} +-(int)n; +@end + +void expr() { + C *c; + int i = [c m]; + D *d; + int j = [d n] + d->x; +} diff --git a/test/Import/struct-and-var/Inputs/S1.cpp b/test/Import/struct-and-var/Inputs/S1.cpp new file mode 100644 index 0000000000..9678b94102 --- /dev/null +++ b/test/Import/struct-and-var/Inputs/S1.cpp @@ -0,0 +1 @@ +int F; diff --git a/test/Import/struct-and-var/Inputs/S2.cpp b/test/Import/struct-and-var/Inputs/S2.cpp new file mode 100644 index 0000000000..3f0ad8e160 --- /dev/null +++ b/test/Import/struct-and-var/Inputs/S2.cpp @@ -0,0 +1,3 @@ +struct F { + int a; +}; diff --git a/test/Import/struct-and-var/test.cpp b/test/Import/struct-and-var/test.cpp new file mode 100644 index 0000000000..76f539e41f --- /dev/null +++ b/test/Import/struct-and-var/test.cpp @@ -0,0 +1,5 @@ +// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s +void expr() { + struct F f; + int x = f.a; +} diff --git a/test/Import/template/Inputs/T.cpp b/test/Import/template/Inputs/T.cpp new file mode 100644 index 0000000000..2499936788 --- /dev/null +++ b/test/Import/template/Inputs/T.cpp @@ -0,0 +1,5 @@ +template struct A { + struct B { + T f; + }; +}; diff --git a/test/Import/template/test.cpp b/test/Import/template/test.cpp new file mode 100644 index 0000000000..89cbda8b92 --- /dev/null +++ b/test/Import/template/test.cpp @@ -0,0 +1,4 @@ +// RUN: clang-import-test -import %S/Inputs/T.cpp -expression %s +void expr() { + A::B b; +} diff --git a/tools/clang-import-test/clang-import-test.cpp b/tools/clang-import-test/clang-import-test.cpp index 186a7c82dc..1ea7ee3611 100644 --- a/tools/clang-import-test/clang-import-test.cpp +++ b/tools/clang-import-test/clang-import-test.cpp @@ -48,7 +48,11 @@ static llvm::cl::list static llvm::cl::opt Direct("direct", llvm::cl::Optional, - llvm::cl::desc("Use the parsed declarations without indirection")); + llvm::cl::desc("Use the parsed declarations without indirection")); + +static llvm::cl::opt + UseOrigins("use-origins", llvm::cl::Optional, + llvm::cl::desc("Use DeclContext origin information for more accurate lookups")); static llvm::cl::list ClangArgs("Xcc", llvm::cl::ZeroOrMore, @@ -60,13 +64,11 @@ static llvm::cl::opt llvm::cl::desc("The language to parse (default: c++)"), llvm::cl::init("c++")); -static llvm::cl::opt -DumpAST("dump-ast", llvm::cl::init(false), - llvm::cl::desc("Dump combined AST")); +static llvm::cl::opt DumpAST("dump-ast", llvm::cl::init(false), + llvm::cl::desc("Dump combined AST")); -static llvm::cl::opt -DumpIR("dump-ir", llvm::cl::init(false), - llvm::cl::desc("Dump IR from final parse")); +static llvm::cl::opt DumpIR("dump-ir", llvm::cl::init(false), + llvm::cl::desc("Dump IR from final parse")); namespace init_convenience { class TestDiagnosticConsumer : public DiagnosticConsumer { @@ -154,8 +156,7 @@ private: } }; -std::unique_ptr -BuildCompilerInstance() { +std::unique_ptr BuildCompilerInstance() { auto Ins = llvm::make_unique(); auto DC = llvm::make_unique(); const bool ShouldOwnClient = true; @@ -227,29 +228,54 @@ std::unique_ptr BuildCodeGen(CompilerInstance &CI, } // end namespace namespace { - -void AddExternalSource( - CompilerInstance &CI, - llvm::ArrayRef> Imports) { - ExternalASTMerger::ImporterEndpoint Target({CI.getASTContext(), CI.getFileManager()}); - llvm::SmallVector Sources; - for (const std::unique_ptr &CI : Imports) { - Sources.push_back({CI->getASTContext(), CI->getFileManager()}); + +/// A container for a CompilerInstance (possibly with an ExternalASTMerger +/// attached to its ASTContext). +/// +/// Provides an accessor for the DeclContext origins associated with the +/// ExternalASTMerger (or an empty list of origins if no ExternalASTMerger is +/// attached). +/// +/// This is the main unit of parsed source code maintained by clang-import-test. +struct CIAndOrigins { + using OriginMap = clang::ExternalASTMerger::OriginMap; + std::unique_ptr CI; + + ASTContext &getASTContext() { return CI->getASTContext(); } + FileManager &getFileManager() { return CI->getFileManager(); } + const OriginMap &getOriginMap() { + static const OriginMap EmptyOriginMap; + if (ExternalASTSource *Source = CI->getASTContext().getExternalSource()) + return static_cast(Source)->GetOrigins(); + return EmptyOriginMap; + } + DiagnosticConsumer &getDiagnosticClient() { + return CI->getDiagnosticClient(); } + CompilerInstance &getCompilerInstance() { return *CI; } +}; + +void AddExternalSource(CIAndOrigins &CI, + llvm::MutableArrayRef Imports) { + ExternalASTMerger::ImporterTarget Target( + {CI.getASTContext(), CI.getFileManager()}); + llvm::SmallVector Sources; + for (CIAndOrigins &Import : Imports) + Sources.push_back( + {Import.getASTContext(), Import.getFileManager(), Import.getOriginMap()}); auto ES = llvm::make_unique(Target, Sources); CI.getASTContext().setExternalSource(ES.release()); CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage(); } -std::unique_ptr BuildIndirect(std::unique_ptr &CI) { - std::unique_ptr IndirectCI = - init_convenience::BuildCompilerInstance(); +CIAndOrigins BuildIndirect(CIAndOrigins &CI) { + CIAndOrigins IndirectCI{init_convenience::BuildCompilerInstance()}; auto ST = llvm::make_unique(); auto BC = llvm::make_unique(); - std::unique_ptr AST = - init_convenience::BuildASTContext(*IndirectCI, *ST, *BC); - IndirectCI->setASTContext(AST.release()); - AddExternalSource(*IndirectCI, CI); + std::unique_ptr AST = init_convenience::BuildASTContext( + IndirectCI.getCompilerInstance(), *ST, *BC); + IndirectCI.getCompilerInstance().setASTContext(AST.release()); + AddExternalSource(IndirectCI, CI); return IndirectCI; } @@ -266,46 +292,53 @@ llvm::Error ParseSource(const std::string &Path, CompilerInstance &CI, return llvm::Error::success(); } -llvm::Expected> -Parse(const std::string &Path, - llvm::ArrayRef> Imports, - bool ShouldDumpAST, bool ShouldDumpIR) { - std::unique_ptr CI = - init_convenience::BuildCompilerInstance(); +llvm::Expected Parse(const std::string &Path, + llvm::MutableArrayRef Imports, + bool ShouldDumpAST, bool ShouldDumpIR) { + CIAndOrigins CI{init_convenience::BuildCompilerInstance()}; auto ST = llvm::make_unique(); auto BC = llvm::make_unique(); std::unique_ptr AST = - init_convenience::BuildASTContext(*CI, *ST, *BC); - CI->setASTContext(AST.release()); + init_convenience::BuildASTContext(CI.getCompilerInstance(), *ST, *BC); + CI.getCompilerInstance().setASTContext(AST.release()); if (Imports.size()) - AddExternalSource(*CI, Imports); + AddExternalSource(CI, Imports); std::vector> ASTConsumers; auto LLVMCtx = llvm::make_unique(); - ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx)); - auto &CG = *static_cast(ASTConsumers.back().get()); + ASTConsumers.push_back( + init_convenience::BuildCodeGen(CI.getCompilerInstance(), *LLVMCtx)); + auto &CG = *static_cast(ASTConsumers.back().get()); if (ShouldDumpAST) ASTConsumers.push_back(CreateASTDumper("", true, false, false)); - CI->getDiagnosticClient().BeginSourceFile(CI->getLangOpts(), - &CI->getPreprocessor()); + CI.getDiagnosticClient().BeginSourceFile( + CI.getCompilerInstance().getLangOpts(), + &CI.getCompilerInstance().getPreprocessor()); MultiplexConsumer Consumers(std::move(ASTConsumers)); - Consumers.Initialize(CI->getASTContext()); + Consumers.Initialize(CI.getASTContext()); - if (llvm::Error PE = ParseSource(Path, *CI, Consumers)) { + if (llvm::Error PE = ParseSource(Path, CI.getCompilerInstance(), Consumers)) return std::move(PE); - } - CI->getDiagnosticClient().EndSourceFile(); + CI.getDiagnosticClient().EndSourceFile(); if (ShouldDumpIR) CG.GetModule()->print(llvm::outs(), nullptr); - if (CI->getDiagnosticClient().getNumErrors()) { + if (CI.getDiagnosticClient().getNumErrors()) return llvm::make_error( "Errors occured while parsing the expression.", std::error_code()); - } else { - return std::move(CI); - } + return std::move(CI); +} + +void Forget(CIAndOrigins &CI, llvm::MutableArrayRef Imports) { + llvm::SmallVector Sources; + for (CIAndOrigins &Import : Imports) + Sources.push_back( + {Import.getASTContext(), Import.getFileManager(), Import.getOriginMap()}); + ExternalASTSource *Source = CI.CI->getASTContext().getExternalSource(); + auto *Merger = static_cast(Source); + Merger->RemoveSources(Sources); } } // end namespace @@ -314,31 +347,32 @@ int main(int argc, const char **argv) { const bool DisableCrashReporting = true; llvm::sys::PrintStackTraceOnErrorSignal(argv[0], DisableCrashReporting); llvm::cl::ParseCommandLineOptions(argc, argv); - std::vector> ImportCIs; + std::vector ImportCIs; for (auto I : Imports) { - llvm::Expected> ImportCI = - Parse(I, {}, false, false); + llvm::Expected ImportCI = Parse(I, {}, false, false); if (auto E = ImportCI.takeError()) { llvm::errs() << llvm::toString(std::move(E)); exit(-1); - } else { - ImportCIs.push_back(std::move(*ImportCI)); } + ImportCIs.push_back(std::move(*ImportCI)); } - std::vector> IndirectCIs; - if (!Direct) { + std::vector IndirectCIs; + if (!Direct || UseOrigins) { for (auto &ImportCI : ImportCIs) { - std::unique_ptr IndirectCI = BuildIndirect(ImportCI); + CIAndOrigins IndirectCI = BuildIndirect(ImportCI); IndirectCIs.push_back(std::move(IndirectCI)); } } - llvm::Expected> ExpressionCI = - Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST, DumpIR); + if (UseOrigins) + for (auto &ImportCI : ImportCIs) + IndirectCIs.push_back(std::move(ImportCI)); + llvm::Expected ExpressionCI = + Parse(Expression, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs, + DumpAST, DumpIR); if (auto E = ExpressionCI.takeError()) { llvm::errs() << llvm::toString(std::move(E)); exit(-1); - } else { - return 0; } + Forget(*ExpressionCI, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs); + return 0; } -