r303322 - [modules] Switch from inferring owning modules based on source location to
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 21 16:56:36 PDT 2017
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,
> >>
> >> 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/
> 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-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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170621/cfa238b7/attachment-0001.html>
More information about the cfe-commits
mailing list