Hi Richard, Somehow this commit caused some methods in ObjC to do not become visible in an interface when compiling with modules on. I filed https://bugs.llvm.org/show_bug.cgi?id=33552, any idea what could have gone wrong here? `hasVisibleDeclarationImpl` doesn't seem to have changed the logic.
Thanks, On Wed, May 17, 2017 at 7:29 PM, Richard Smith via cfe-commits <cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Wed May 17 21:29:20 2017 > New Revision: 303322 > > URL: http://llvm.org/viewvc/llvm-project?rev=303322&view=rev > Log: > [modules] Switch from inferring owning modules based on source location to > inferring based on the current module at the point of creation. > > This should result in no functional change except when building a preprocessed > module (or more generally when using #pragma clang module begin/end to switch > module in the middle of a file), in which case it allows us to correctly track > the owning module for declarations. We can't map from FileID to module in the > preprocessed module case, since all modules would have the same FileID. > > There are still a couple of remaining places that try to infer a module from a > source location; I'll clean those up in follow-up changes. > > Modified: > cfe/trunk/include/clang/AST/ASTContext.h > cfe/trunk/include/clang/AST/DeclBase.h > cfe/trunk/include/clang/Basic/LangOptions.h > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/include/clang/Serialization/ASTWriter.h > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/lib/Sema/SemaLookup.cpp > cfe/trunk/lib/Sema/SemaTemplate.cpp > cfe/trunk/lib/Serialization/ASTWriter.cpp > cfe/trunk/lib/Serialization/ASTWriterDecl.cpp > cfe/trunk/test/Modules/preprocess-module.cpp > cfe/trunk/test/SemaCXX/modules-ts.cppm > > Modified: cfe/trunk/include/clang/AST/ASTContext.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/ASTContext.h (original) > +++ cfe/trunk/include/clang/AST/ASTContext.h Wed May 17 21:29:20 2017 > @@ -935,7 +935,7 @@ public: > > /// \brief Get the additional modules in which the definition \p Def has > /// been merged. > - ArrayRef<Module*> getModulesWithMergedDefinition(NamedDecl *Def) { > + ArrayRef<Module*> getModulesWithMergedDefinition(const NamedDecl *Def) { > auto MergedIt = MergedDefModules.find(Def); > if (MergedIt == MergedDefModules.end()) > return None; > > Modified: cfe/trunk/include/clang/AST/DeclBase.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/DeclBase.h (original) > +++ cfe/trunk/include/clang/AST/DeclBase.h Wed May 17 21:29:20 2017 > @@ -332,15 +332,15 @@ private: > bool AccessDeclContextSanity() const; > > protected: > - > Decl(Kind DK, DeclContext *DC, SourceLocation L) > - : NextInContextAndBits(), DeclCtx(DC), > - Loc(L), DeclKind(DK), InvalidDecl(0), > - HasAttrs(false), Implicit(false), Used(false), Referenced(false), > - Access(AS_none), FromASTFile(0), Hidden(DC && cast<Decl>(DC)->Hidden), > - IdentifierNamespace(getIdentifierNamespaceForKind(DK)), > - CacheValidAndLinkage(0) > - { > + : NextInContextAndBits(), DeclCtx(DC), Loc(L), DeclKind(DK), > + InvalidDecl(0), HasAttrs(false), Implicit(false), Used(false), > + Referenced(false), Access(AS_none), FromASTFile(0), > + Hidden(DC && cast<Decl>(DC)->Hidden && > + (!cast<Decl>(DC)->isFromASTFile() || > + hasLocalOwningModuleStorage())), > + IdentifierNamespace(getIdentifierNamespaceForKind(DK)), > + CacheValidAndLinkage(0) { > if (StatisticsEnabled) add(DK); > } > > @@ -698,6 +698,9 @@ public: > Module *getLocalOwningModule() const { > if (isFromASTFile() || !Hidden) > return nullptr; > + > + assert(hasLocalOwningModuleStorage() && > + "hidden local decl but no local module storage"); > return reinterpret_cast<Module *const *>(this)[-1]; > } > void setLocalOwningModule(Module *M) { > > Modified: cfe/trunk/include/clang/Basic/LangOptions.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/LangOptions.h (original) > +++ cfe/trunk/include/clang/Basic/LangOptions.h Wed May 17 21:29:20 2017 > @@ -168,7 +168,7 @@ public: > > /// Do we need to track the owning module for a local declaration? > bool trackLocalOwningModule() const { > - return ModulesLocalVisibility; > + return isCompilingModule() || ModulesLocalVisibility || ModulesTS; > } > > bool isSignedOverflowDefined() const { > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Wed May 17 21:29:20 2017 > @@ -1507,6 +1507,12 @@ public: > hasVisibleDefaultArgument(const NamedDecl *D, > llvm::SmallVectorImpl<Module *> *Modules = > nullptr); > > + /// Determine if there is a visible declaration of \p D that is an explicit > + /// specialization declaration for a specialization of a template. (For a > + /// member specialization, use hasVisibleMemberSpecialization.) > + bool hasVisibleExplicitSpecialization( > + const NamedDecl *D, llvm::SmallVectorImpl<Module *> *Modules = > nullptr); > + > /// Determine if there is a visible declaration of \p D that is a member > /// specialization declaration (as opposed to an instantiated declaration). > bool hasVisibleMemberSpecialization( > @@ -2360,7 +2366,7 @@ public: > void MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool MergeTypeWithOld); > void MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old); > bool checkVarDeclRedefinition(VarDecl *OldDefn, VarDecl *NewDefn); > - void notePreviousDefinition(SourceLocation Old, SourceLocation New); > + void notePreviousDefinition(const NamedDecl *Old, SourceLocation New); > bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, Scope *S); > > // AssignmentAction - This is used by all the assignment diagnostic > functions > > Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original) > +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed May 17 21:29:20 2017 > @@ -627,10 +627,6 @@ public: > /// \brief Add a version tuple to the given record > void AddVersionTuple(const VersionTuple &Version, RecordDataImpl &Record); > > - /// \brief Infer the submodule ID that contains an entity at the given > - /// source location. > - serialization::SubmoduleID inferSubmoduleIDFromLocation(SourceLocation > Loc); > - > /// \brief Retrieve or create a submodule ID for this module, or return 0 > if > /// the submodule is neither local (a submodle of the currently-written > module) > /// nor from an imported module. > > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed May 17 21:29:20 2017 > @@ -2613,7 +2613,7 @@ llvm::DIModule *CGDebugInfo::getParentMo > // best to make this behavior a command line or debugger tuning > // option. > FullSourceLoc Loc(D->getLocation(), CGM.getContext().getSourceManager()); > - if (Module *M = ClangModuleMap->inferModuleFromLocation(Loc)) { > + if (Module *M = D->getOwningModule()) { > // This is a (sub-)module. > auto Info = ExternalASTSource::ASTSourceDescriptor(*M); > return getOrCreateModuleRef(Info, /*SkeletonCU=*/false); > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed May 17 21:29:20 2017 > @@ -2021,7 +2021,7 @@ bool Sema::isIncompatibleTypedef(TypeDec > Diag(New->getLocation(), > diag::err_redefinition_variably_modified_typedef) > << Kind << NewType; > if (Old->getLocation().isValid()) > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > New->setInvalidDecl(); > return true; > } > @@ -2034,7 +2034,7 @@ bool Sema::isIncompatibleTypedef(TypeDec > Diag(New->getLocation(), diag::err_redefinition_different_typedef) > << Kind << NewType << OldType; > if (Old->getLocation().isValid()) > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > New->setInvalidDecl(); > return true; > } > @@ -2101,7 +2101,7 @@ void Sema::MergeTypedefNameDecl(Scope *S > > NamedDecl *OldD = OldDecls.getRepresentativeDecl(); > if (OldD->getLocation().isValid()) > - notePreviousDefinition(OldD->getLocation(), New->getLocation()); > + notePreviousDefinition(OldD, New->getLocation()); > > return New->setInvalidDecl(); > } > @@ -2193,7 +2193,7 @@ void Sema::MergeTypedefNameDecl(Scope *S > > Diag(New->getLocation(), diag::err_redefinition) > << New->getDeclName(); > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > return New->setInvalidDecl(); > } > > @@ -2214,7 +2214,7 @@ void Sema::MergeTypedefNameDecl(Scope *S > > Diag(New->getLocation(), diag::ext_redefinition_of_typedef) > << New->getDeclName(); > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > } > > /// DeclhasAttr - returns true if decl Declaration already has the target > @@ -2448,7 +2448,7 @@ static bool mergeDeclAttribute(Sema &S, > return false; > } > > -static const Decl *getDefinition(const Decl *D) { > +static const NamedDecl *getDefinition(const Decl *D) { > if (const TagDecl *TD = dyn_cast<TagDecl>(D)) > return TD->getDefinition(); > if (const VarDecl *VD = dyn_cast<VarDecl>(D)) { > @@ -2475,7 +2475,7 @@ static void checkNewAttributesAfterDef(S > if (!New->hasAttrs()) > return; > > - const Decl *Def = getDefinition(Old); > + const NamedDecl *Def = getDefinition(Old); > if (!Def || Def == New) > return; > > @@ -2502,7 +2502,7 @@ static void checkNewAttributesAfterDef(S > : diag::err_redefinition; > S.Diag(VD->getLocation(), Diag) << VD->getDeclName(); > if (Diag == diag::err_redefinition) > - S.notePreviousDefinition(Def->getLocation(), VD->getLocation()); > + S.notePreviousDefinition(Def, VD->getLocation()); > else > S.Diag(Def->getLocation(), diag::note_previous_definition); > VD->setInvalidDecl(); > @@ -2891,7 +2891,7 @@ bool Sema::MergeFunctionDecl(FunctionDec > } else { > Diag(New->getLocation(), diag::err_redefinition_different_kind) > << New->getDeclName(); > - notePreviousDefinition(OldD->getLocation(), New->getLocation()); > + notePreviousDefinition(OldD, New->getLocation()); > return true; > } > } > @@ -2928,7 +2928,7 @@ bool Sema::MergeFunctionDecl(FunctionDec > !Old->hasAttr<InternalLinkageAttr>()) { > Diag(New->getLocation(), diag::err_internal_linkage_redeclaration) > << New->getDeclName(); > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > New->dropAttr<InternalLinkageAttr>(); > } > > @@ -3657,7 +3657,7 @@ void Sema::MergeVarDecl(VarDecl *New, Lo > if (!Old) { > Diag(New->getLocation(), diag::err_redefinition_different_kind) > << New->getDeclName(); > - notePreviousDefinition(Previous.getRepresentativeDecl()->getLocation(), > + notePreviousDefinition(Previous.getRepresentativeDecl(), > New->getLocation()); > return New->setInvalidDecl(); > } > @@ -3687,7 +3687,7 @@ void Sema::MergeVarDecl(VarDecl *New, Lo > Old->getStorageClass() == SC_None && > !Old->hasAttr<WeakImportAttr>()) { > Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName(); > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > // Remove weak_import attribute on new declaration. > New->dropAttr<WeakImportAttr>(); > } > @@ -3696,7 +3696,7 @@ void Sema::MergeVarDecl(VarDecl *New, Lo > !Old->hasAttr<InternalLinkageAttr>()) { > Diag(New->getLocation(), diag::err_internal_linkage_redeclaration) > << New->getDeclName(); > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > New->dropAttr<InternalLinkageAttr>(); > } > > @@ -3853,29 +3853,22 @@ void Sema::MergeVarDecl(VarDecl *New, Lo > New->setImplicitlyInline(); > } > > -void Sema::notePreviousDefinition(SourceLocation Old, SourceLocation New) { > +void Sema::notePreviousDefinition(const NamedDecl *Old, SourceLocation New) { > SourceManager &SrcMgr = getSourceManager(); > auto FNewDecLoc = SrcMgr.getDecomposedLoc(New); > - auto FOldDecLoc = SrcMgr.getDecomposedLoc(Old); > + auto FOldDecLoc = SrcMgr.getDecomposedLoc(Old->getLocation()); > auto *FNew = SrcMgr.getFileEntryForID(FNewDecLoc.first); > auto *FOld = SrcMgr.getFileEntryForID(FOldDecLoc.first); > auto &HSI = PP.getHeaderSearchInfo(); > - StringRef HdrFilename = SrcMgr.getFilename(SrcMgr.getSpellingLoc(Old)); > + StringRef HdrFilename = > + SrcMgr.getFilename(SrcMgr.getSpellingLoc(Old->getLocation())); > > - auto noteFromModuleOrInclude = [&](SourceLocation &Loc, > - SourceLocation &IncLoc) -> bool { > - Module *Mod = nullptr; > + auto noteFromModuleOrInclude = [&](Module *Mod, > + SourceLocation IncLoc) -> bool { > // Redefinition errors with modules are common with non modular mapped > // headers, example: a non-modular header H in module A that also gets > // included directly in a TU. Pointing twice to the same > header/definition > // is confusing, try to get better diagnostics when modules is on. > - if (getLangOpts().Modules) { > - auto ModLoc = SrcMgr.getModuleImportLoc(Old); > - if (!ModLoc.first.isInvalid()) > - Mod = HSI.getModuleMap().inferModuleFromLocation( > - FullSourceLoc(Loc, SrcMgr)); > - } > - > if (IncLoc.isValid()) { > if (Mod) { > Diag(IncLoc, diag::note_redefinition_modules_same_file) > @@ -3899,19 +3892,19 @@ void Sema::notePreviousDefinition(Source > if (FNew == FOld && FNewDecLoc.second == FOldDecLoc.second) { > SourceLocation OldIncLoc = SrcMgr.getIncludeLoc(FOldDecLoc.first); > SourceLocation NewIncLoc = SrcMgr.getIncludeLoc(FNewDecLoc.first); > - EmittedDiag = noteFromModuleOrInclude(Old, OldIncLoc); > - EmittedDiag |= noteFromModuleOrInclude(New, NewIncLoc); > + EmittedDiag = noteFromModuleOrInclude(Old->getOwningModule(), OldIncLoc); > + EmittedDiag |= noteFromModuleOrInclude(getCurrentModule(), NewIncLoc); > > // If the header has no guards, emit a note suggesting one. > if (FOld && !HSI.isFileMultipleIncludeGuarded(FOld)) > - Diag(Old, diag::note_use_ifdef_guards); > + Diag(Old->getLocation(), diag::note_use_ifdef_guards); > > if (EmittedDiag) > return; > } > > // Redefinition coming from different files or couldn't do better above. > - Diag(Old, diag::note_previous_definition); > + Diag(Old->getLocation(), diag::note_previous_definition); > } > > /// We've just determined that \p Old and \p New both appear to be > definitions > @@ -3934,7 +3927,7 @@ bool Sema::checkVarDeclRedefinition(VarD > return false; > } else { > Diag(New->getLocation(), diag::err_redefinition) << New; > - notePreviousDefinition(Old->getLocation(), New->getLocation()); > + notePreviousDefinition(Old, New->getLocation()); > New->setInvalidDecl(); > return true; > } > @@ -13503,9 +13496,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned > } else if (TUK == TUK_Reference && > (PrevTagDecl->getFriendObjectKind() == > Decl::FOK_Undeclared || > - PP.getModuleContainingLocation( > - PrevDecl->getLocation()) != > - PP.getModuleContainingLocation(KWLoc)) && > + PrevDecl->getOwningModule() != getCurrentModule()) && > SS.isEmpty()) { > // This declaration is a reference to an existing entity, but > // has different visibility from that entity: it either makes > @@ -13561,7 +13552,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned > Diag(NameLoc, diag::warn_redefinition_in_param_list) << > Name; > else > Diag(NameLoc, diag::err_redefinition) << Name; > - notePreviousDefinition(Def->getLocation(), > + notePreviousDefinition(Def, > NameLoc.isValid() ? NameLoc : KWLoc); > // If this is a redefinition, recover by making this > // struct be anonymous, which will make any later > @@ -13652,7 +13643,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned > // The tag name clashes with something else in the target scope, > // issue an error and recover by making this tag be anonymous. > Diag(NameLoc, diag::err_redefinition_different_kind) << Name; > - notePreviousDefinition(PrevDecl->getLocation(), NameLoc); > + notePreviousDefinition(PrevDecl, NameLoc); > Name = nullptr; > Invalid = true; > } > @@ -15356,7 +15347,7 @@ Decl *Sema::ActOnEnumConstant(Scope *S, > Diag(IdLoc, diag::err_redefinition_of_enumerator) << Id; > else > Diag(IdLoc, diag::err_redefinition) << Id; > - notePreviousDefinition(PrevDecl->getLocation(), IdLoc); > + notePreviousDefinition(PrevDecl, IdLoc); > return nullptr; > } > } > > Modified: cfe/trunk/lib/Sema/SemaLookup.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) > +++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed May 17 21:29:20 2017 > @@ -1420,11 +1420,46 @@ bool Sema::hasVisibleDefaultArgument(con > Modules); > } > > +template<typename Filter> > +static bool hasVisibleDeclarationImpl(Sema &S, const NamedDecl *D, > + llvm::SmallVectorImpl<Module *> > *Modules, > + Filter F) { > + for (auto *Redecl : D->redecls()) { > + auto *R = cast<NamedDecl>(Redecl); > + if (!F(R)) > + continue; > + > + if (S.isVisible(R)) > + return true; > + > + if (Modules) { > + Modules->push_back(R->getOwningModule()); > + const auto &Merged = S.Context.getModulesWithMergedDefinition(R); > + Modules->insert(Modules->end(), Merged.begin(), Merged.end()); > + } > + } > + > + return false; > +} > + > +bool Sema::hasVisibleExplicitSpecialization( > + const NamedDecl *D, llvm::SmallVectorImpl<Module *> *Modules) { > + return hasVisibleDeclarationImpl(*this, D, Modules, [](const NamedDecl *D) > { > + if (auto *RD = dyn_cast<CXXRecordDecl>(D)) > + return RD->getTemplateSpecializationKind() == > TSK_ExplicitSpecialization; > + if (auto *FD = dyn_cast<FunctionDecl>(D)) > + return FD->getTemplateSpecializationKind() == > TSK_ExplicitSpecialization; > + if (auto *VD = dyn_cast<VarDecl>(D)) > + return VD->getTemplateSpecializationKind() == > TSK_ExplicitSpecialization; > + llvm_unreachable("unknown explicit specialization kind"); > + }); > +} > + > bool Sema::hasVisibleMemberSpecialization( > const NamedDecl *D, llvm::SmallVectorImpl<Module *> *Modules) { > assert(isa<CXXRecordDecl>(D->getDeclContext()) && > "not a member specialization"); > - for (auto *Redecl : D->redecls()) { > + return hasVisibleDeclarationImpl(*this, D, Modules, [](const NamedDecl *D) > { > // If the specialization is declared at namespace scope, then it's a > member > // specialization declaration. If it's lexically inside the class > // definition then it was instantiated. > @@ -1432,19 +1467,8 @@ bool Sema::hasVisibleMemberSpecializatio > // FIXME: This is a hack. There should be a better way to determine this. > // FIXME: What about MS-style explicit specializations declared within a > // class definition? > - if (Redecl->getLexicalDeclContext()->isFileContext()) { > - auto *NonConstR = const_cast<NamedDecl*>(cast<NamedDecl>(Redecl)); > - > - if (isVisible(NonConstR)) > - return true; > - > - if (Modules) { > - Modules->push_back(getOwningModule(NonConstR)); > - const auto &Merged = > Context.getModulesWithMergedDefinition(NonConstR); > - Modules->insert(Modules->end(), Merged.begin(), Merged.end()); > - } > - } > - } > + return D->getLexicalDeclContext()->isFileContext(); > + }); > > return false; > } > @@ -1459,23 +1483,19 @@ bool Sema::hasVisibleMemberSpecializatio > /// your module can see, including those later on in your module). > bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) { > assert(D->isHidden() && "should not call this: not in slow case"); > - Module *DeclModule = nullptr; > - > - if (SemaRef.getLangOpts().ModulesLocalVisibility) { > - DeclModule = SemaRef.getOwningModule(D); > - if (!DeclModule) { > - assert(!D->isHidden() && "hidden decl not from a module"); > - return true; > - } > > - // If the owning module is visible, and the decl is not module private, > - // then the decl is visible too. (Module private is ignored within the > same > - // top-level module.) > - if ((!D->isFromASTFile() || !D->isModulePrivate()) && > - (SemaRef.isModuleVisible(DeclModule) || > - SemaRef.hasVisibleMergedDefinition(D))) > - return true; > - } > + Module *DeclModule = SemaRef.getOwningModule(D); > + assert(DeclModule && "hidden decl not from a module"); > + > + // If the owning module is visible, and the decl is not module private, > + // then the decl is visible too. (Module private is ignored within the same > + // top-level module.) > + // FIXME: Check the owning module for module-private declarations rather > than > + // assuming "same AST file" is the same thing as "same module". > + if ((!D->isFromASTFile() || !D->isModulePrivate()) && > + (SemaRef.isModuleVisible(DeclModule) || > + SemaRef.hasVisibleMergedDefinition(D))) > + return true; > > // If this declaration is not at namespace scope nor module-private, > // then it is visible if its lexical parent has a visible definition. > @@ -1571,20 +1591,8 @@ static NamedDecl *findAcceptableDecl(Sem > bool Sema::hasVisibleDeclarationSlow(const NamedDecl *D, > llvm::SmallVectorImpl<Module *> > *Modules) { > assert(!isVisible(D) && "not in slow case"); > - > - for (auto *Redecl : D->redecls()) { > - auto *NonConstR = const_cast<NamedDecl*>(cast<NamedDecl>(Redecl)); > - if (isVisible(NonConstR)) > - return true; > - > - if (Modules) { > - Modules->push_back(getOwningModule(NonConstR)); > - const auto &Merged = Context.getModulesWithMergedDefinition(NonConstR); > - Modules->insert(Modules->end(), Merged.begin(), Merged.end()); > - } > - } > - > - return false; > + return hasVisibleDeclarationImpl(*this, D, Modules, > + [](const NamedDecl *) { return true; }); > } > > NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const { > @@ -4957,6 +4965,14 @@ void Sema::diagnoseMissingImport(SourceL > MissingImportKind MIK, bool Recover) { > assert(!Modules.empty()); > > + // Weed out duplicates from module list. > + llvm::SmallVector<Module*, 8> UniqueModules; > + llvm::SmallDenseSet<Module*, 8> UniqueModuleSet; > + for (auto *M : Modules) > + if (UniqueModuleSet.insert(M).second) > + UniqueModules.push_back(M); > + Modules = UniqueModules; > + > if (Modules.size() > 1) { > std::string ModuleList; > unsigned N = 0; > > Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed May 17 21:29:20 2017 > @@ -7901,6 +7901,7 @@ bool Sema::CheckFunctionTemplateSpeciali > TemplateSpecializationKind TSK = SpecInfo->getTemplateSpecializationKind(); > if (TSK == TSK_Undeclared || TSK == TSK_ImplicitInstantiation) { > Specialization->setLocation(FD->getLocation()); > + Specialization->setLexicalDeclContext(FD->getLexicalDeclContext()); > // C++11 [dcl.constexpr]p1: An explicit specialization of a constexpr > // function can differ from the template declaration with respect to > // the constexpr specifier. > @@ -7961,6 +7962,7 @@ bool Sema::CheckFunctionTemplateSpeciali > // FIXME: We need an update record for this AST mutation. > Specialization->setDeletedAsWritten(false); > } > + // FIXME: We need an update record for this AST mutation. > SpecInfo->setTemplateSpecializationKind(TSK_ExplicitSpecialization); > MarkUnusedFileScopedDecl(Specialization); > } > @@ -9745,7 +9747,7 @@ private: > IsHiddenExplicitSpecialization = > Spec->getMemberSpecializationInfo() > ? !S.hasVisibleMemberSpecialization(Spec, &Modules) > - : !S.hasVisibleDeclaration(Spec); > + : !S.hasVisibleExplicitSpecialization(Spec, &Modules); > } else { > checkInstantiated(Spec); > } > > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed May 17 21:29:20 2017 > @@ -2841,25 +2841,6 @@ void ASTWriter::WriteSubmodules(Module * > "non-imported submodule?"); > } > > -serialization::SubmoduleID > -ASTWriter::inferSubmoduleIDFromLocation(SourceLocation Loc) { > - if (Loc.isInvalid() || !WritingModule) > - return 0; // No submodule > - > - // Find the module that owns this location. > - ModuleMap &ModMap = PP->getHeaderSearchInfo().getModuleMap(); > - Module *OwningMod > - = > ModMap.inferModuleFromLocation(FullSourceLoc(Loc,PP->getSourceManager())); > - if (!OwningMod) > - return 0; > - > - // Check whether this submodule is part of our own module. > - if (WritingModule != OwningMod && !OwningMod->isSubModuleOf(WritingModule)) > - return 0; > - > - return getSubmoduleID(OwningMod); > -} > - > void ASTWriter::WritePragmaDiagnosticMappings(const DiagnosticsEngine &Diag, > bool isModule) { > llvm::SmallDenseMap<const DiagnosticsEngine::DiagState *, unsigned, 64> > > Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original) > +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed May 17 21:29:20 2017 > @@ -299,7 +299,7 @@ void ASTDeclWriter::VisitDecl(Decl *D) { > Record.push_back(D->isTopLevelDeclInObjCContainer()); > Record.push_back(D->getAccess()); > Record.push_back(D->isModulePrivate()); > - Record.push_back(Writer.inferSubmoduleIDFromLocation(D->getLocation())); > + Record.push_back(Writer.getSubmoduleID(D->getOwningModule())); > > // If this declaration injected a name into a context different from its > // lexical context, and that context is an imported namespace, we need to > > Modified: cfe/trunk/test/Modules/preprocess-module.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/preprocess-module.cpp?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/test/Modules/preprocess-module.cpp (original) > +++ cfe/trunk/test/Modules/preprocess-module.cpp Wed May 17 21:29:20 2017 > @@ -25,8 +25,8 @@ > // RUN: %clang_cc1 -fmodules -fmodule-name=file -fmodule-file=%t/fwd.pcm > -fmodule-map-file=%S/Inputs/preprocess/module.modulemap -x > c++-module-map-cpp-output %t/rewrite.ii -emit-module -o /dev/null > > // Check the module we built works. > -// RUN: %clang_cc1 -fmodules -fmodule-file=%t/no-rewrite.pcm %s -verify > -// RUN: %clang_cc1 -fmodules -fmodule-file=%t/rewrite.pcm %s -verify > +// RUN: %clang_cc1 -fmodules -fmodule-file=%t/no-rewrite.pcm %s -I%t -verify > -fno-modules-error-recovery > +// RUN: %clang_cc1 -fmodules -fmodule-file=%t/rewrite.pcm %s -I%t -verify > -fno-modules-error-recovery -DREWRITE > > > // == module map > @@ -95,10 +95,12 @@ > // NO-REWRITE: #pragma clang module end > > > -// expected-no-diagnostics > - > -// FIXME: This should be rejected: we have not imported the submodule > defining it yet. > -__FILE *a; > +__FILE *a; // expected-error {{declaration of '__FILE' must be imported}} > +#ifdef REWRITE > +// expected-n...@rewrite.ii:1 {{here}} > +#else > +// expected-n...@no-rewrite.ii:1 {{here}} > +#endif > > #pragma clang module import file > > > Modified: cfe/trunk/test/SemaCXX/modules-ts.cppm > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/modules-ts.cppm?rev=303322&r1=303321&r2=303322&view=diff > ============================================================================== > --- cfe/trunk/test/SemaCXX/modules-ts.cppm (original) > +++ cfe/trunk/test/SemaCXX/modules-ts.cppm Wed May 17 21:29:20 2017 > @@ -18,7 +18,8 @@ int n; > #if TEST >= 2 > // expected-error@-2 {{redefinition of '}} > // expected-note@-3 {{unguarded header; consider using #ifdef guards or > #pragma once}} > -// expected-note...@modules-ts.cppm:1 {{'{{.*}}modules-ts.cppm' included > multiple times, additional include site here}} > +// FIXME: We should drop the "header from" in this diagnostic. > +// expected-note...@modules-ts.cppm:1 {{'{{.*}}modules-ts.cppm' included > multiple times, additional include site in header from module 'foo'}} > #endif > > #if TEST == 0 > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Bruno Cardoso Lopes http://www.brunocardoso.cc _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
