r303322 - [modules] Switch from inferring owning modules based on source location to
Michael Gottesman via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 22 16:18:24 PDT 2017
> On Jun 21, 2017, at 4:56 PM, Richard Smith via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> On 21 June 2017 at 16:55, Bruno Cardoso Lopes <bruno.cardoso at gmail.com <mailto:bruno.cardoso at gmail.com>> wrote:
> On Wed, Jun 21, 2017 at 4:44 PM, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
> > On 21 June 2017 at 14:51, Bruno Cardoso Lopes <bruno.cardoso at gmail.com <mailto:bruno.cardoso at 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 <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!
Do you have an eta on this?
This is blocking swift from compiling against ToT LLVM. As you know these changes come in fast so the longer we wait, the more likely other breakage sneaks in. I imagine we are going to probably build against the newer llvm/clang in the next bit, so this is the worst time to have a long period of time of breakage.
+CC Jordan Rose since I think he ran into this.
Thanks in advance = ),
Michael
>
> >> Thanks,
> >>
> >> On Wed, May 17, 2017 at 7:29 PM, Richard Smith via cfe-commits
> >> <cfe-commits at lists.llvm.org <mailto:cfe-commits at 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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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-note at rewrite.ii:1 {{here}}
> >> > +#else
> >> > +// expected-note at 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 <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 at -2 {{redefinition of '}}
> >> > // expected-note at -3 {{unguarded header; consider using #ifdef guards or
> >> > #pragma once}}
> >> > -// expected-note-re at 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-re at 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 at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
> >>
> >>
> >>
> >> --
> >> Bruno Cardoso Lopes
> >> http://www.brunocardoso.cc <http://www.brunocardoso.cc/>
> >
> >
>
>
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc <http://www.brunocardoso.cc/>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170622/59a538d1/attachment-0001.html>
More information about the cfe-commits
mailing list