r303322 - [modules] Switch from inferring owning modules based on source location to
Bruno Cardoso Lopes via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 21 16:55:31 PDT 2017
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.
>
>>
>> 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
More information about the cfe-commits
mailing list