This was leading to many crashers. Reverted in r303037. On Sat, May 13, 2017 at 1:27 AM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Author: rsmith > Date: Fri May 12 18:27:00 2017 > New Revision: 302965 > > URL: http://llvm.org/viewvc/llvm-project?rev=302965&view=rev > Log: > [modules] When creating a declaration, cache its owning module immediately > rather than waiting until it's queried. > > Currently this is only applied to local submodule visibility mode, as we > don't > yet allocate storage for the owning module in non-local-visibility modules > compilations. > > Modified: > cfe/trunk/include/clang/AST/Decl.h > cfe/trunk/include/clang/AST/DeclBase.h > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/lib/AST/ASTDumper.cpp > cfe/trunk/lib/AST/Decl.cpp > cfe/trunk/lib/AST/DeclBase.cpp > cfe/trunk/lib/Sema/Sema.cpp > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/lib/Sema/SemaLookup.cpp > > Modified: cfe/trunk/include/clang/AST/Decl.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/AST/Decl.h?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/include/clang/AST/Decl.h (original) > +++ cfe/trunk/include/clang/AST/Decl.h Fri May 12 18:27:00 2017 > @@ -301,16 +301,6 @@ 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(); > > Modified: cfe/trunk/include/clang/AST/DeclBase.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/AST/DeclBase.h?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/include/clang/AST/DeclBase.h (original) > +++ cfe/trunk/include/clang/AST/DeclBase.h Fri May 12 18:27:00 2017 > @@ -706,6 +706,20 @@ public: > reinterpret_cast<Module **>(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; > } > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/Sema/Sema.h?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Fri May 12 18:27:00 2017 > @@ -1463,11 +1463,9 @@ private: > > VisibleModuleSet VisibleModules; > > - Module *CachedFakeTopLevelModule; > - > public: > /// \brief Get the module owning an entity. > - Module *getOwningModule(Decl *Entity); > + Module *getOwningModule(Decl *Entity) { return > Entity->getOwningModule(); } > > /// \brief Make a merged definition of an existing hidden definition \p > ND > /// visible at the specified location. > > Modified: cfe/trunk/lib/AST/ASTDumper.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ > ASTDumper.cpp?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/AST/ASTDumper.cpp (original) > +++ cfe/trunk/lib/AST/ASTDumper.cpp Fri May 12 18:27:00 2017 > @@ -1038,10 +1038,10 @@ void ASTDumper::dumpDecl(const Decl *D) > dumpSourceRange(D->getSourceRange()); > OS << ' '; > dumpLocation(D->getLocation()); > - if (Module *M = D->getImportedOwningModule()) > + if (D->isFromASTFile()) > + OS << " imported"; > + if (Module *M = D->getOwningModule()) > OS << " in " << M->getFullModuleName(); > - else if (Module *M = D->getLocalOwningModule()) > - OS << " in (local) " << M->getFullModuleName(); > if (auto *ND = dyn_cast<NamedDecl>(D)) > for (Module *M : D->getASTContext().getModulesWithMergedDefinition( > const_cast<NamedDecl *>(ND))) > > Modified: cfe/trunk/lib/AST/Decl.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ > Decl.cpp?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/AST/Decl.cpp (original) > +++ cfe/trunk/lib/AST/Decl.cpp Fri May 12 18:27:00 2017 > @@ -47,9 +47,7 @@ bool Decl::isOutOfLine() const { > > TranslationUnitDecl::TranslationUnitDecl(ASTContext &ctx) > : Decl(TranslationUnit, nullptr, SourceLocation()), > - DeclContext(TranslationUnit), Ctx(ctx), AnonymousNamespace(nullptr) > { > - Hidden = Ctx.getLangOpts().ModulesLocalVisibility; > -} > + DeclContext(TranslationUnit), Ctx(ctx), AnonymousNamespace(nullptr) > {} > > //===------------------------------------------------------- > ---------------===// > // NamedDecl Implementation > > Modified: cfe/trunk/lib/AST/DeclBase.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ > DeclBase.cpp?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/AST/DeclBase.cpp (original) > +++ cfe/trunk/lib/AST/DeclBase.cpp Fri May 12 18:27:00 2017 > @@ -83,7 +83,9 @@ void *Decl::operator new(std::size_t Siz > char *Buffer = reinterpret_cast<char *>( > ::operator new(ExtraAlign + sizeof(Module *) + Size + Extra, > Ctx)); > Buffer += ExtraAlign; > - return new (Buffer) Module*(nullptr) + 1; > + auto *ParentModule = > + Parent ? cast<Decl>(Parent)->getOwningModule() : nullptr; > + return new (Buffer) Module*(ParentModule) + 1; > } > return ::operator new(Size + Extra, Ctx); > } > @@ -273,6 +275,11 @@ void Decl::setLexicalDeclContext(DeclCon > getMultipleDC()->LexicalDC = DC; > } > Hidden = cast<Decl>(DC)->Hidden; > + if (Hidden && !isFromASTFile()) { > + assert(hasLocalOwningModuleStorage() && > + "hidden local declaration without local submodule > visibility?"); > + setLocalOwningModule(cast<Decl>(DC)->getOwningModule()); > + } > } > > void Decl::setDeclContextsImpl(DeclContext *SemaDC, DeclContext > *LexicalDC, > > Modified: cfe/trunk/lib/Sema/Sema.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > Sema.cpp?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Sema/Sema.cpp (original) > +++ cfe/trunk/lib/Sema/Sema.cpp Fri May 12 18:27:00 2017 > @@ -93,11 +93,10 @@ Sema::Sema(Preprocessor &pp, ASTContext > ValueWithBytesObjCTypeMethod(nullptr), NSArrayDecl(nullptr), > ArrayWithObjectsMethod(nullptr), NSDictionaryDecl(nullptr), > DictionaryWithObjectsMethod(nullptr), > GlobalNewDeleteDeclared(false), > - TUKind(TUKind), NumSFINAEErrors(0), CachedFakeTopLevelModule( > nullptr), > - AccessCheckingSFINAE(false), InNonInstantiationSFINAEContex > t(false), > - NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1), > - CurrentInstantiationScope(nullptr), DisableTypoCorrection(false), > - TyposCorrected(0), AnalysisWarnings(*this), > + TUKind(TUKind), NumSFINAEErrors(0), 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; > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaDecl.cpp?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri May 12 18:27:00 2017 > @@ -16034,6 +16034,14 @@ void Sema::ActOnModuleBegin(SourceLocati > 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().ModulesLocalVisibility) { > + cast<Decl>(CurContext)->setHidden(true); > + cast<Decl>(CurContext)->setLocalOwningModule(Mod); > + } > } > > void Sema::ActOnModuleEnd(SourceLocation EomLoc, Module *Mod) { > @@ -16062,6 +16070,13 @@ void Sema::ActOnModuleEnd(SourceLocation > DirectiveLoc = EomLoc; > } > BuildModuleInclude(DirectiveLoc, Mod); > + > + // Any further declarations are in whatever module we returned to. > + if (getLangOpts().ModulesLocalVisibility) { > + cast<Decl>(CurContext)->setLocalOwningModule(getCurrentModule()); > + if (!getCurrentModule()) > + cast<Decl>(CurContext)->setHidden(false); > + } > } > > void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc, > > Modified: cfe/trunk/lib/Sema/SemaLookup.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaLookup.cpp?rev=302965&r1=302964&r2=302965&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) > +++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri May 12 18:27:00 2017 > @@ -1326,62 +1326,6 @@ bool Sema::CppLookupName(LookupResult &R > return !R.empty(); > } > > -Module *Sema::getOwningModule(Decl *Entity) { > - // If it's imported, grab its owning module. > - Module *M = Entity->getImportedOwningModule(); > - if (M || !isa<NamedDecl>(Entity) || !cast<NamedDecl>(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<NamedDecl>(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<NamedDecl>(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( > - "<top-level>", 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); > @@ -1520,7 +1464,6 @@ bool LookupResult::isVisibleSlow(Sema &S > 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; > } > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits