On Wed, Jun 21, 2017 at 4:56 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > 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 Richard! > >> >> >> 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 > > -- 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