Consistently create a new declaration when merging a pre-existing but

hidden definition with a would-be-parsed redefinition.

This permits a bunch of cleanups. In particular, we no longer need to
take merged definitions into account when checking declaration
visibility, only when checking definition visibility, which makes
certain visibility checks take linear instead of quadratic time.

We could also now remove the UPD_DECL_EXPORTED update record and track
on each declaration whether it was demoted from a definition (as we
already do for variables), but I'm not doing that in this patch to keep
the changes here simpler.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@342018 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 45 insertions, 45 deletions

diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 8cf8dae886..f7f5957f96 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -1392,8 +1392,17 @@ llvm::DenseSet<Module*> &Sema::getLookupModules() {
   return LookupModulesCache;
 }
 
+/// Determine whether the module M is part of the current module from the
+/// perspective of a module-private visibility check.
+static bool isInCurrentModule(const Module *M, const LangOptions &LangOpts) {
+  // If M is the global module fragment of a module that we've not yet finished
+  // parsing, then it must be part of the current module.
+  return M->getTopLevelModuleName() == LangOpts.CurrentModule ||
+         (M->Kind == Module::GlobalModuleFragment && !M->Parent);
+}
+
 bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
-  for (Module *Merged : Context.getModulesWithMergedDefinition(Def))
+  for (const Module *Merged : Context.getModulesWithMergedDefinition(Def))
     if (isModuleVisible(Merged))
       return true;
   return false;
@@ -1407,8 +1416,8 @@ bool Sema::hasMergedDefinitionInCurrentModule(NamedDecl *Def) {
   if (Def->getModuleOwnershipKind() == Decl::ModuleOwnershipKind::Visible &&
       getLangOpts().ModulesLocalVisibility)
     return true;
-  for (Module *Merged : Context.getModulesWithMergedDefinition(Def))
-    if (Merged->getTopLevelModuleName() == getLangOpts().CurrentModule)
+  for (const Module *Merged : Context.getModulesWithMergedDefinition(Def))
+    if (isInCurrentModule(Merged, getLangOpts()))
       return true;
   return false;
 }
@@ -1428,8 +1437,6 @@ hasVisibleDefaultArgument(Sema &S, const ParmDecl *D,
     if (!DefaultArg.isInherited() && Modules) {
       auto *NonConstD = const_cast<ParmDecl*>(D);
       Modules->push_back(S.getOwningModule(NonConstD));
-      const auto &Merged = S.Context.getModulesWithMergedDefinition(NonConstD);
-      Modules->insert(Modules->end(), Merged.begin(), Merged.end());
     }
 
     // If there was a previous default argument, maybe its parameter is visible.
@@ -1464,11 +1471,8 @@ static bool hasVisibleDeclarationImpl(Sema &S, const NamedDecl *D,
 
     HasFilteredRedecls = true;
 
-    if (Modules) {
+    if (Modules)
       Modules->push_back(R->getOwningModule());
-      const auto &Merged = S.Context.getModulesWithMergedDefinition(R);
-      Modules->insert(Modules->end(), Merged.begin(), Merged.end());
-    }
   }
 
   // Only return false if there is at least one redecl that is not filtered out.
@@ -1519,27 +1523,11 @@ bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) {
   assert(D->isHidden() && "should not call this: not in slow case");
 
   Module *DeclModule = SemaRef.getOwningModule(D);
-  if (!DeclModule) {
-    // A module-private declaration with no owning module means this is in the
-    // global module in the C++ Modules TS. This is visible within the same
-    // translation unit only.
-    // FIXME: Don't assume that "same translation unit" means the same thing
-    // as "not from an AST file".
-    assert(D->isModulePrivate() && "hidden decl has no module");
-    if (!D->isFromASTFile() || SemaRef.hasMergedDefinitionInCurrentModule(D))
-      return true;
-  } else {
-    // 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->isModulePrivate()
-          ? DeclModule->getTopLevelModuleName() ==
-                    SemaRef.getLangOpts().CurrentModule ||
-            SemaRef.hasMergedDefinitionInCurrentModule(D)
-          : SemaRef.isModuleVisible(DeclModule) ||
-            SemaRef.hasVisibleMergedDefinition(D))
-      return true;
-  }
+  assert(DeclModule && "hidden decl has no owning module");
+
+  // If the owning module is visible, the decl is visible.
+  if (SemaRef.isModuleVisible(DeclModule, D->isModulePrivate()))
+    return true;
 
   // Determine whether a decl context is a file context for the purpose of
   // visibility. This looks through some (export and linkage spec) transparent
@@ -1589,29 +1577,41 @@ bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) {
     return VisibleWithinParent;
   }
 
-  // FIXME: All uses of DeclModule below this point should also check merged
-  // modules.
-  if (!DeclModule)
-    return false;
+  return false;
+}
+
+bool Sema::isModuleVisible(const Module *M, bool ModulePrivate) {
+  // The module might be ordinarily visible. For a module-private query, that
+  // means it is part of the current module. For any other query, that means it
+  // is in our visible module set.
+  if (ModulePrivate) {
+    if (isInCurrentModule(M, getLangOpts()))
+      return true;
+  } else {
+    if (VisibleModules.isVisible(M))
+      return true;
+  }
+
+  // Otherwise, it might be visible by virtue of the query being within a
+  // template instantiation or similar that is permitted to look inside M.
 
   // Find the extra places where we need to look.
-  const auto &LookupModules = SemaRef.getLookupModules();
+  const auto &LookupModules = getLookupModules();
   if (LookupModules.empty())
     return false;
 
-  // If our lookup set contains the decl's module, it's visible.
-  if (LookupModules.count(DeclModule))
+  // If our lookup set contains the module, it's visible.
+  if (LookupModules.count(M))
     return true;
 
-  // If the declaration isn't exported, it's not visible in any other module.
-  if (D->isModulePrivate())
+  // For a module-private query, that's everywhere we get to look.
+  if (ModulePrivate)
     return false;
 
-  // Check whether DeclModule is transitively exported to an import of
-  // the lookup set.
+  // Check whether M is transitively exported to an import of the lookup set.
   return std::any_of(LookupModules.begin(), LookupModules.end(),
-                     [&](const Module *M) {
-                       return M->isModuleVisible(DeclModule); });
+                     [&](const Module *LookupM) {
+                       return LookupM->isModuleVisible(M); });
 }
 
 bool Sema::isVisibleSlow(const NamedDecl *D) {
@@ -5061,12 +5061,12 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
   if (!Def)
     Def = Decl;
 
-  Module *Owner = getOwningModule(Decl);
+  Module *Owner = getOwningModule(Def);
   assert(Owner && "definition of hidden declaration is not in a module");
 
   llvm::SmallVector<Module*, 8> OwningModules;
   OwningModules.push_back(Owner);
-  auto Merged = Context.getModulesWithMergedDefinition(Decl);
+  auto Merged = Context.getModulesWithMergedDefinition(Def);
   OwningModules.insert(OwningModules.end(), Merged.begin(), Merged.end());
 
   diagnoseMissingImport(Loc, Decl, Decl->getLocation(), OwningModules, MIK,