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 14:51:58 PDT 2017
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.
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
More information about the cfe-commits
mailing list