From: Daniel Jasper Date: Mon, 15 May 2017 07:51:10 +0000 (+0000) Subject: Revert r302965 - [modules] When creating a declaration, cache its owning X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ffd03c81d3b17bd25a9f3c6b22fc1b650f65a505;p=clang Revert r302965 - [modules] When creating a declaration, cache its owning module immediately Also revert dependent r302969. This is leading to crashes. Will provide more details reproduction instructions to Richard. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@303037 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 67f704119a..bba40e56b0 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -301,6 +301,16 @@ public: using Decl::isModulePrivate; using Decl::setModulePrivate; + /// \brief Determine whether this declaration is hidden from name lookup. + bool isHidden() const { return Hidden; } + + /// \brief Set whether this declaration is hidden from name lookup. + void setHidden(bool Hide) { + assert((!Hide || isFromASTFile() || hasLocalOwningModuleStorage()) && + "declaration with no owning module can't be hidden"); + Hidden = Hide; + } + /// \brief Determine whether this declaration is a C++ class member. bool isCXXClassMember() const { const DeclContext *DC = getDeclContext(); diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h index 08879b36cc..15ac11a5a7 100644 --- a/include/clang/AST/DeclBase.h +++ b/include/clang/AST/DeclBase.h @@ -706,20 +706,6 @@ public: reinterpret_cast(this)[-1] = M; } - Module *getOwningModule() const { - return isFromASTFile() ? getImportedOwningModule() : getLocalOwningModule(); - } - - /// \brief Determine whether this declaration is hidden from name lookup. - bool isHidden() const { return Hidden; } - - /// \brief Set whether this declaration is hidden from name lookup. - void setHidden(bool Hide) { - assert((!Hide || isFromASTFile() || hasLocalOwningModuleStorage()) && - "declaration with no owning module can't be hidden"); - Hidden = Hide; - } - unsigned getIdentifierNamespace() const { return IdentifierNamespace; } diff --git a/include/clang/Basic/LangOptions.h b/include/clang/Basic/LangOptions.h index ceaedf5857..20a0e58456 100644 --- a/include/clang/Basic/LangOptions.h +++ b/include/clang/Basic/LangOptions.h @@ -166,11 +166,6 @@ public: return getCompilingModule() != CMK_None; } - /// Do we need to track the owning module for a local declaration? - bool trackLocalOwningModule() const { - return ModulesLocalVisibility; - } - bool isSignedOverflowDefined() const { return getSignedOverflowBehavior() == SOB_Defined; } diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index feaf936924..86c995c54f 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1463,9 +1463,11 @@ private: VisibleModuleSet VisibleModules; + Module *CachedFakeTopLevelModule; + public: /// \brief Get the module owning an entity. - Module *getOwningModule(Decl *Entity) { return Entity->getOwningModule(); } + Module *getOwningModule(Decl *Entity); /// \brief Make a merged definition of an existing hidden definition \p ND /// visible at the specified location. diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index d89be0d9e6..ef491ab06f 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -1038,10 +1038,10 @@ void ASTDumper::dumpDecl(const Decl *D) { dumpSourceRange(D->getSourceRange()); OS << ' '; dumpLocation(D->getLocation()); - if (D->isFromASTFile()) - OS << " imported"; - if (Module *M = D->getOwningModule()) + if (Module *M = D->getImportedOwningModule()) OS << " in " << M->getFullModuleName(); + else if (Module *M = D->getLocalOwningModule()) + OS << " in (local) " << M->getFullModuleName(); if (auto *ND = dyn_cast(D)) for (Module *M : D->getASTContext().getModulesWithMergedDefinition( const_cast(ND))) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index a1342f477b..0f2558e24b 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -47,7 +47,9 @@ bool Decl::isOutOfLine() const { TranslationUnitDecl::TranslationUnitDecl(ASTContext &ctx) : Decl(TranslationUnit, nullptr, SourceLocation()), - DeclContext(TranslationUnit), Ctx(ctx), AnonymousNamespace(nullptr) {} + DeclContext(TranslationUnit), Ctx(ctx), AnonymousNamespace(nullptr) { + Hidden = Ctx.getLangOpts().ModulesLocalVisibility; +} //===----------------------------------------------------------------------===// // NamedDecl Implementation diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index e0c626c585..5c2c9cbd01 100644 --- a/lib/AST/DeclBase.cpp +++ b/lib/AST/DeclBase.cpp @@ -75,7 +75,7 @@ void *Decl::operator new(std::size_t Size, const ASTContext &Ctx, assert(!Parent || &Parent->getParentASTContext() == &Ctx); // With local visibility enabled, we track the owning module even for local // declarations. - if (Ctx.getLangOpts().trackLocalOwningModule()) { + if (Ctx.getLangOpts().ModulesLocalVisibility) { // Ensure required alignment of the resulting object by adding extra // padding at the start if required. size_t ExtraAlign = @@ -83,9 +83,7 @@ void *Decl::operator new(std::size_t Size, const ASTContext &Ctx, char *Buffer = reinterpret_cast( ::operator new(ExtraAlign + sizeof(Module *) + Size + Extra, Ctx)); Buffer += ExtraAlign; - auto *ParentModule = - Parent ? cast(Parent)->getOwningModule() : nullptr; - return new (Buffer) Module*(ParentModule) + 1; + return new (Buffer) Module*(nullptr) + 1; } return ::operator new(Size + Extra, Ctx); } @@ -96,7 +94,7 @@ Module *Decl::getOwningModuleSlow() const { } bool Decl::hasLocalOwningModuleStorage() const { - return getASTContext().getLangOpts().trackLocalOwningModule(); + return getASTContext().getLangOpts().ModulesLocalVisibility; } const char *Decl::getDeclKindName() const { @@ -275,11 +273,6 @@ void Decl::setLexicalDeclContext(DeclContext *DC) { getMultipleDC()->LexicalDC = DC; } Hidden = cast(DC)->Hidden; - if (Hidden && !isFromASTFile()) { - assert(hasLocalOwningModuleStorage() && - "hidden local declaration without local submodule visibility?"); - setLocalOwningModule(cast(DC)->getOwningModule()); - } } void Decl::setDeclContextsImpl(DeclContext *SemaDC, DeclContext *LexicalDC, diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index e7b0914641..ca1d27e950 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -93,10 +93,11 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, ValueWithBytesObjCTypeMethod(nullptr), NSArrayDecl(nullptr), ArrayWithObjectsMethod(nullptr), NSDictionaryDecl(nullptr), DictionaryWithObjectsMethod(nullptr), GlobalNewDeleteDeclared(false), - TUKind(TUKind), NumSFINAEErrors(0), AccessCheckingSFINAE(false), - InNonInstantiationSFINAEContext(false), NonInstantiationEntries(0), - ArgumentPackSubstitutionIndex(-1), CurrentInstantiationScope(nullptr), - DisableTypoCorrection(false), TyposCorrected(0), AnalysisWarnings(*this), + TUKind(TUKind), NumSFINAEErrors(0), CachedFakeTopLevelModule(nullptr), + AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false), + NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1), + CurrentInstantiationScope(nullptr), DisableTypoCorrection(false), + TyposCorrected(0), AnalysisWarnings(*this), ThreadSafetyDeclCache(nullptr), VarDataSharingAttributesStack(nullptr), CurScope(nullptr), Ident_super(nullptr), Ident___float128(nullptr) { TUScope = nullptr; diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index fb3922d572..fb5f56e052 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -16034,14 +16034,6 @@ void Sema::ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod) { ModuleScopes.back().OuterVisibleModules = std::move(VisibleModules); VisibleModules.setVisible(Mod, DirectiveLoc); - - // The enclosing context is now part of this module. - // FIXME: Consider creating a child DeclContext to hold the entities - // lexically within the module. - if (getLangOpts().trackLocalOwningModule()) { - cast(CurContext)->setHidden(true); - cast(CurContext)->setLocalOwningModule(Mod); - } } void Sema::ActOnModuleEnd(SourceLocation EomLoc, Module *Mod) { @@ -16070,13 +16062,6 @@ void Sema::ActOnModuleEnd(SourceLocation EomLoc, Module *Mod) { DirectiveLoc = EomLoc; } BuildModuleInclude(DirectiveLoc, Mod); - - // Any further declarations are in whatever module we returned to. - if (getLangOpts().trackLocalOwningModule()) { - cast(CurContext)->setLocalOwningModule(getCurrentModule()); - if (!getCurrentModule()) - cast(CurContext)->setHidden(false); - } } void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc, diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 04350831c6..c5b579a4b2 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -1326,6 +1326,62 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) { return !R.empty(); } +Module *Sema::getOwningModule(Decl *Entity) { + // If it's imported, grab its owning module. + Module *M = Entity->getImportedOwningModule(); + if (M || !isa(Entity) || !cast(Entity)->isHidden()) + return M; + assert(!Entity->isFromASTFile() && + "hidden entity from AST file has no owning module"); + + if (!getLangOpts().ModulesLocalVisibility) { + // If we're not tracking visibility locally, the only way a declaration + // can be hidden and local is if it's hidden because it's parent is (for + // instance, maybe this is a lazily-declared special member of an imported + // class). + auto *Parent = cast(Entity->getDeclContext()); + assert(Parent->isHidden() && "unexpectedly hidden decl"); + return getOwningModule(Parent); + } + + // It's local and hidden; grab or compute its owning module. + M = Entity->getLocalOwningModule(); + if (M) + return M; + + if (auto *Containing = + PP.getModuleContainingLocation(Entity->getLocation())) { + M = Containing; + } else if (Entity->isInvalidDecl() || Entity->getLocation().isInvalid()) { + // Don't bother tracking visibility for invalid declarations with broken + // locations. + cast(Entity)->setHidden(false); + } else { + // We need to assign a module to an entity that exists outside of any + // module, so that we can hide it from modules that we textually enter. + // Invent a fake module for all such entities. + if (!CachedFakeTopLevelModule) { + CachedFakeTopLevelModule = + PP.getHeaderSearchInfo().getModuleMap().findOrCreateModule( + "", nullptr, false, false).first; + + auto &SrcMgr = PP.getSourceManager(); + SourceLocation StartLoc = + SrcMgr.getLocForStartOfFile(SrcMgr.getMainFileID()); + auto &TopLevel = ModuleScopes.empty() + ? VisibleModules + : ModuleScopes[0].OuterVisibleModules; + TopLevel.setVisible(CachedFakeTopLevelModule, StartLoc); + } + + M = CachedFakeTopLevelModule; + } + + if (M) + Entity->setLocalOwningModule(M); + return M; +} + void Sema::makeMergedDefinitionVisible(NamedDecl *ND) { if (auto *M = getCurrentModule()) Context.mergeDefinitionIntoModule(ND, M); @@ -1464,6 +1520,7 @@ bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) { if (SemaRef.getLangOpts().ModulesLocalVisibility) { DeclModule = SemaRef.getOwningModule(D); if (!DeclModule) { + // getOwningModule() may have decided the declaration should not be hidden. assert(!D->isHidden() && "hidden decl not from a module"); return true; }