r303322 - [modules] Switch from inferring owning modules based on source location to
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 22 18:05:44 PDT 2017
On 22 June 2017 at 16:18, Michael Gottesman <mgottesman at apple.com> wrote:
>
> 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>
> wrote:
>
>> On Wed, Jun 21, 2017 at 4:44 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On 21 June 2017 at 14:51, Bruno Cardoso Lopes <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, 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?
>
Let me know if you're still having problems after r306075.
> 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> 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/clan
>> g/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/clan
>> g/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/clan
>> g/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/clan
>> g/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/clan
>> g/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/Sem
>> aDecl.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_differe
>> nt_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_differe
>> nt_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_red
>> eclaration)
>> >> > << 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()->get
>> Location(),
>> >> > + 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_red
>> eclaration)
>> >> > << 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(FOldDecLo
>> c.first);
>> >> > SourceLocation NewIncLoc = SrcMgr.getIncludeLoc(FNewDecLo
>> c.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_par
>> am_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/Sem
>> aLookup.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.getModulesWithMerged
>> Definition(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<Na
>> medDecl>(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/Sem
>> aTemplate.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->getLexicalDeclCont
>> ext());
>> >> > // 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/Serializ
>> ation/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->getSour
>> ceManager()));
>> >> > - 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/Serializ
>> ation/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-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
>> >> >
>> >> > ============================================================
>> ==================
>> >> > --- 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
>> >> > 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 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/57f3630d/attachment-0001.html>
More information about the cfe-commits
mailing list