On 21 June 2017 at 16:55, Bruno Cardoso Lopes <bruno.card...@gmail.com> wrote:
> 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. Yes, I'm working on what should (hopefully) be a fairly quick short-term fix. > >> 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
