Author: djasper Date: Mon May 15 02:51:10 2017 New Revision: 303037 URL: http://llvm.org/viewvc/llvm-project?rev=303037&view=rev Log: 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. Modified: cfe/trunk/include/clang/AST/Decl.h cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/include/clang/Basic/LangOptions.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=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Decl.h (original) +++ cfe/trunk/include/clang/AST/Decl.h Mon May 15 02:51:10 2017 @@ -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(); Modified: cfe/trunk/include/clang/AST/DeclBase.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/DeclBase.h (original) +++ cfe/trunk/include/clang/AST/DeclBase.h Mon May 15 02:51:10 2017 @@ -706,20 +706,6 @@ 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/Basic/LangOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/LangOptions.h (original) +++ cfe/trunk/include/clang/Basic/LangOptions.h Mon May 15 02:51:10 2017 @@ -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; } Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Mon May 15 02:51:10 2017 @@ -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. Modified: cfe/trunk/lib/AST/ASTDumper.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTDumper.cpp (original) +++ cfe/trunk/lib/AST/ASTDumper.cpp Mon May 15 02:51:10 2017 @@ -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<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=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/lib/AST/Decl.cpp (original) +++ cfe/trunk/lib/AST/Decl.cpp Mon May 15 02:51:10 2017 @@ -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 Modified: cfe/trunk/lib/AST/DeclBase.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/lib/AST/DeclBase.cpp (original) +++ cfe/trunk/lib/AST/DeclBase.cpp Mon May 15 02:51:10 2017 @@ -75,7 +75,7 @@ void *Decl::operator new(std::size_t Siz 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 Siz char *Buffer = reinterpret_cast<char *>( ::operator new(ExtraAlign + sizeof(Module *) + Size + Extra, Ctx)); Buffer += ExtraAlign; - auto *ParentModule = - Parent ? cast<Decl>(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() cons } bool Decl::hasLocalOwningModuleStorage() const { - return getASTContext().getLangOpts().trackLocalOwningModule(); + return getASTContext().getLangOpts().ModulesLocalVisibility; } const char *Decl::getDeclKindName() const { @@ -275,11 +273,6 @@ 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=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/lib/Sema/Sema.cpp (original) +++ cfe/trunk/lib/Sema/Sema.cpp Mon May 15 02:51:10 2017 @@ -93,10 +93,11 @@ Sema::Sema(Preprocessor &pp, ASTContext 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; Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon May 15 02:51:10 2017 @@ -16034,14 +16034,6 @@ 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().trackLocalOwningModule()) { - cast<Decl>(CurContext)->setHidden(true); - cast<Decl>(CurContext)->setLocalOwningModule(Mod); - } } void Sema::ActOnModuleEnd(SourceLocation EomLoc, Module *Mod) { @@ -16070,13 +16062,6 @@ void Sema::ActOnModuleEnd(SourceLocation DirectiveLoc = EomLoc; } BuildModuleInclude(DirectiveLoc, Mod); - - // Any further declarations are in whatever module we returned to. - if (getLangOpts().trackLocalOwningModule()) { - 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=303037&r1=303036&r2=303037&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon May 15 02:51:10 2017 @@ -1326,6 +1326,62 @@ 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); @@ -1464,6 +1520,7 @@ 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