diff --git a/include/clang/AST/DeclContextInternals.h b/include/clang/AST/DeclContextInternals.h index 84f3698d6b585facb82d152dbaf7337ab2c5d380..9c626c80aaee1c7edc501f417c49934e18b340fb 100644 --- a/include/clang/AST/DeclContextInternals.h +++ b/include/clang/AST/DeclContextInternals.h @@ -18,6 +18,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclarationName.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/SmallVector.h" #include <algorithm> @@ -26,23 +27,29 @@ namespace clang { class DependentDiagnostic; -/// StoredDeclsList - This is an array of decls optimized a common case of only -/// containing one entry. +/// \brief An array of decls optimized for the common case of only containing +/// one entry. struct StoredDeclsList { - /// DeclsTy - When in vector form, this is what the Data pointer points to. + /// \brief When in vector form, this is what the Data pointer points to. typedef SmallVector<NamedDecl *, 4> DeclsTy; + /// \brief A collection of declarations, with a flag to indicate if we have + /// further external declarations. + typedef llvm::PointerIntPair<DeclsTy *, 1, bool> DeclsAndHasExternalTy; + /// \brief The stored data, which will be either a pointer to a NamedDecl, - /// or a pointer to a vector. - llvm::PointerUnion<NamedDecl *, DeclsTy *> Data; + /// or a pointer to a vector with a flag to indicate if there are further + /// external declarations. + llvm::PointerUnion<NamedDecl*, DeclsAndHasExternalTy> Data; public: StoredDeclsList() {} StoredDeclsList(const StoredDeclsList &RHS) : Data(RHS.Data) { if (DeclsTy *RHSVec = RHS.getAsVector()) - Data = new DeclsTy(*RHSVec); + Data = DeclsAndHasExternalTy(new DeclsTy(*RHSVec), + RHS.hasExternalDecls()); } ~StoredDeclsList() { @@ -56,7 +63,7 @@ public: delete Vector; Data = RHS.Data; if (DeclsTy *RHSVec = RHS.getAsVector()) - Data = new DeclsTy(*RHSVec); + Data = DeclsAndHasExternalTy(new DeclsTy(*RHSVec), hasExternalDecls()); return *this; } @@ -66,8 +73,27 @@ public: return Data.dyn_cast<NamedDecl *>(); } + DeclsAndHasExternalTy getAsVectorAndHasExternal() const { + return Data.dyn_cast<DeclsAndHasExternalTy>(); + } + DeclsTy *getAsVector() const { - return Data.dyn_cast<DeclsTy *>(); + return getAsVectorAndHasExternal().getPointer(); + } + + bool hasExternalDecls() const { + return getAsVectorAndHasExternal().getInt(); + } + + void setHasExternalDecls() { + if (DeclsTy *Vec = getAsVector()) + Data = DeclsAndHasExternalTy(Vec, true); + else { + DeclsTy *VT = new DeclsTy(); + if (NamedDecl *OldD = getAsDecl()) + VT->push_back(OldD); + Data = DeclsAndHasExternalTy(VT, true); + } } void setOnlyValue(NamedDecl *ND) { @@ -110,6 +136,8 @@ public: Vec.erase(std::remove_if(Vec.begin(), Vec.end(), std::mem_fun(&Decl::isFromASTFile)), Vec.end()); + // Don't have any external decls any more. + Data = DeclsAndHasExternalTy(&Vec, false); } } @@ -165,12 +193,14 @@ public: /// not a redeclaration to merge it into the appropriate place in our list. /// void AddSubsequentDecl(NamedDecl *D) { + assert(!isNull() && "don't AddSubsequentDecl when we have no decls"); + // If this is the second decl added to the list, convert this to vector // form. if (NamedDecl *OldD = getAsDecl()) { DeclsTy *VT = new DeclsTy(); VT->push_back(OldD); - Data = VT; + Data = DeclsAndHasExternalTy(VT, false); } DeclsTy &Vec = *getAsVector(); diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index e4d4e77942d43428d73dabc995193c6dadf342f9..be1992a4d545e6bebb4e78ad3addb1c6fee99b77 100644 --- a/lib/AST/DeclBase.cpp +++ b/lib/AST/DeclBase.cpp @@ -926,11 +926,8 @@ void DeclContext::reconcileExternalVisibleStorage() { NeedToReconcileExternalVisibleStorage = false; StoredDeclsMap &Map = *LookupPtr.getPointer(); - ExternalASTSource *Source = getParentASTContext().getExternalSource(); - for (StoredDeclsMap::iterator I = Map.begin(); I != Map.end(); ++I) { - I->second.removeExternalDecls(); - Source->FindExternalVisibleDeclsByName(this, I->first); - } + for (StoredDeclsMap::iterator I = Map.begin(); I != Map.end(); ++I) + I->second.setHasExternalDecls(); } /// \brief Load the declarations within this lexical storage from an @@ -983,8 +980,7 @@ ExternalASTSource::SetNoExternalVisibleDeclsForName(const DeclContext *DC, if (!(Map = DC->LookupPtr.getPointer())) Map = DC->CreateStoredDeclsMap(Context); - // Add an entry to the map for this name, if it's not already present. - (*Map)[Name]; + (*Map)[Name].removeExternalDecls(); return DeclContext::lookup_result(); } @@ -1246,16 +1242,14 @@ DeclContext::lookup(DeclarationName Name) { if (!Map) Map = CreateStoredDeclsMap(getParentASTContext()); - // If a PCH/module has a result for this name, and we have a local - // declaration, we will have imported the PCH/module result when adding the - // local declaration or when reconciling the module. + // If we have a lookup result with no external decls, we are done. std::pair<StoredDeclsMap::iterator, bool> R = Map->insert(std::make_pair(Name, StoredDeclsList())); - if (!R.second) + if (!R.second && !R.first->second.hasExternalDecls()) return R.first->second.getLookupResult(); ExternalASTSource *Source = getParentASTContext().getExternalSource(); - if (Source->FindExternalVisibleDeclsByName(this, Name)) { + if (Source->FindExternalVisibleDeclsByName(this, Name) || R.second) { if (StoredDeclsMap *Map = LookupPtr.getPointer()) { StoredDeclsMap::iterator I = Map->find(Name); if (I != Map->end()) @@ -1304,7 +1298,8 @@ DeclContext::noload_lookup(DeclarationName Name) { LookupPtr.setInt(false); // There may now be names for which we have local decls but are - // missing the external decls. + // missing the external decls. FIXME: Just set the hasExternalDecls + // flag on those names that have external decls. NeedToReconcileExternalVisibleStorage = true; Map = LookupPtr.getPointer(); @@ -1461,7 +1456,18 @@ void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D, bool Internal) { // Insert this declaration into the map. StoredDeclsList &DeclNameEntries = (*Map)[D->getDeclName()]; - if (DeclNameEntries.isNull()) { + + if (Internal) { + // If this is being added as part of loading an external declaration, + // this may not be the only external declaration with this name. + // In this case, we never try to replace an existing declaration; we'll + // handle that when we finalize the list of declarations for this name. + DeclNameEntries.setHasExternalDecls(); + DeclNameEntries.AddSubsequentDecl(D); + return; + } + + else if (DeclNameEntries.isNull()) { DeclNameEntries.setOnlyValue(D); return; } diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index c8fd32ff2f71b545c9bacd95d9349854c2de960a..f42d7568895707a662f6bf5396c8e39ce9112525 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -5070,6 +5070,14 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, return false; } + // FIXME: If there's an unimported definition of this type in a module (for + // instance, because we forward declared it, then imported the definition), + // import that definition now. + // FIXME: What about other cases where an import extends a redeclaration + // chain for a declaration that can be accessed through a mechanism other + // than name lookup (eg, referenced in a template, or a variable whose type + // could be completed by the module)? + const TagType *Tag = T->getAs<TagType>(); const ObjCInterfaceType *IFace = 0; diff --git a/test/Modules/Inputs/def.h b/test/Modules/Inputs/def.h index eb7eb7e59dc177c2f1a4ae09e82afef89d6915e4..6fa83d3eec75a6ede084fecfb5b6629edec79fb8 100644 --- a/test/Modules/Inputs/def.h +++ b/test/Modules/Inputs/def.h @@ -17,4 +17,11 @@ class Def2 { public: void func(); }; + +namespace Def3NS { + class Def3 { + public: + void func(); + }; +} #endif diff --git a/test/Modules/Inputs/namespaces-top.h b/test/Modules/Inputs/namespaces-top.h index 0c607f5285160eeb79dbd3e7458bf3c6e9de3397..7aa8490eb7e18c8bb721811401bd8c857c374313 100644 --- a/test/Modules/Inputs/namespaces-top.h +++ b/test/Modules/Inputs/namespaces-top.h @@ -12,3 +12,8 @@ namespace N3 { namespace N12 { } +namespace N13 { + void f(); + int f(int); + void (*p)() = &f; +} diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp index 0949436b5d80300020f4a6bd0d7c520441c2dc44..9e6cd17828e260f4aaf034fed5914184cae5dc02 100644 --- a/test/Modules/cxx-templates.cpp +++ b/test/Modules/cxx-templates.cpp @@ -82,11 +82,8 @@ typedef SomeTemplate<int&> SomeTemplateIntRef; SomeTemplate<char*> some_template_char_ptr; SomeTemplate<char&> some_template_char_ref; -// FIXME: There should only be two 'f's here. // CHECK-GLOBAL: DeclarationName 'f' // CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f' -// CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f' -// CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f' // CHECK-GLOBAL-NEXT: `-FunctionTemplate {{.*}} 'f' // CHECK-NAMESPACE-N: DeclarationName 'f' diff --git a/test/Modules/decldef.mm b/test/Modules/decldef.mm index c693c7f2ba55ca1fa819e53822e49d98372bb586..35694935dfbb9370b2e61d02dd9ab8f3b8152ab1 100644 --- a/test/Modules/decldef.mm +++ b/test/Modules/decldef.mm @@ -6,8 +6,10 @@ @class Def; Def *def; -class Def2; +class Def2; // expected-note {{forward decl}} Def2 *def2; +namespace Def3NS { class Def3; } // expected-note {{forward decl}} +Def3NS::Def3 *def3; @interface Unrelated - defMethod; @@ -39,5 +41,9 @@ void testDef() { } void testDef2() { - def2->func(); + // FIXME: These should both work, since we've (implicitly) imported + // decldef.Def here, but they don't, because nothing has triggered the lazy + // loading of the definitions of these classes. + def2->func(); // expected-error {{incomplete}} + def3->func(); // expected-error {{incomplete}} } diff --git a/test/Modules/namespaces.cpp b/test/Modules/namespaces.cpp index 426e0025f9f84e0ea54ed566a69045823fca07ef..8c225e051bdead033611bdfda8805268b0f1d0c9 100644 --- a/test/Modules/namespaces.cpp +++ b/test/Modules/namespaces.cpp @@ -75,3 +75,10 @@ void testAnonymousNotMerged() { // expected-note@Inputs/namespaces-right.h:60 {{passing argument to parameter here}} // expected-note@Inputs/namespaces-right.h:67 {{passing argument to parameter here}} + +// Test that bringing in one name from an overload set does not hide the rest. +void testPartialImportOfOverloadSet() { + void (*p)() = N13::p; + p(); + N13::f(0); +}