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 67f704119a622348731800b65b4d7c49e7f86669..bba40e56b003bc4460985d2d4e8020438eb5a5af 100644 (file)
--- 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 08879b36cce54a9b31b6985b62f669bfbeb62203..15ac11a5a77701f2d232997c03d37e4083bba91a 100644 (file)
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -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;
   }

diff --git a/include/clang/Basic/LangOptions.h b/include/clang/Basic/LangOptions.h
index ceaedf58574fddf8d7cdf2e13179d9852cd730c9..20a0e584560206b60060b59189f8807d024fdfc3 100644 (file)
--- 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 feaf93692458a2d0d97a93e3dcc29c2e8b43b855..86c995c54fdae5b85b1a3ba23371e953dbf1aa52 100644 (file)
--- 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 d89be0d9e6fa49b4b6f333afde22325717720c96..ef491ab06f8c3d6547976e756952dcbac48e33b7 100644 (file)
--- 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<NamedDecl>(D))
       for (Module *M : D->getASTContext().getModulesWithMergedDefinition(
                const_cast<NamedDecl *>(ND)))

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index a1342f477b681d42aaf48a9d353722d5115ca513..0f2558e24ba5685a017b6b7a74569fc074ac7a14 100644 (file)
--- 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 e0c626c585b3fdbacde5f003aa808c3d13bf110f..5c2c9cbd0180233d6ffc18b1e5ef081adfacb63f 100644 (file)
--- 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<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() 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<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,

diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index e7b0914641ff53314a77a381c983635a8becef9c..ca1d27e9505f03fa0edf0f3d822bcc0c84e1c069 100644 (file)
--- 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 fb3922d57213f2c7782c124c1eec819bca3c0102..fb5f56e0523f4a9f5b772f9a394cc1afa795d6c7 100644 (file)
--- 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<Decl>(CurContext)->setHidden(true);
-    cast<Decl>(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<Decl>(CurContext)->setLocalOwningModule(getCurrentModule());
-    if (!getCurrentModule())
-      cast<Decl>(CurContext)->setHidden(false);
-  }
 }
 
 void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc,

diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 04350831c68143731516b77232a0e9b572d8255a..c5b579a4b2e9bce25b976387d412cbb63e6386e9 100644 (file)
--- 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<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 &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;
     }