On Wed, Jun 21, 2017 at 4:44 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > On 21 June 2017 at 14:51, Bruno Cardoso Lopes <bruno.card...@gmail.com> > wrote: >> >> 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. > > > DeclObjC.cpp is making some incorrect assumptions about what the isHidden() > flag on Decls means. Looks like we're going to need to move all of the ObjC > lookup machinery out of DeclObjC into Sema to allow it to perform correct > visibility checks. (For what it's worth, this would already have been broken > for Objective-C++ and local submodule visibility mode prior to this change, > as those modes both have situations where the "Hidden" flag is not the > complete story with regard to whether a declaration is visible in a > particular lookup context.)
Oh, that's bad. Is there any workaround we can do on top of this change for now in order to have the previous behavior for non-LSV and ObjC? This is keeping Swift from building against upstream right now. > >> >> 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 > > -- 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