[modules] Fix some visibility issues with default template arguments.

There are still problems here, but this is a better starting point.

The main part of the change is: when doing a lookup that would accept visible
or hidden declarations, prefer to produce the latest visible declaration if
there are any visible declarations, rather than always producing the latest
declaration.

Thus, when we inherit default arguments (and other properties) from a previous
declaration, we inherit them from the previous visible declaration; if the
previous declaration is hidden, we already suppress inheritance of default
arguments.

There are a couple of other changes here that fix latent bugs exposed by this
change.

llvm-svn: 239371

9 files changed, 60 insertions, 7 deletions

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 451f9da1b60..e06b58b37be 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -236,7 +236,11 @@ public:
   bool isHidden() const { return Hidden; }
 
   /// \brief Set whether this declaration is hidden from name lookup.
-  void setHidden(bool Hide) { Hidden = Hide; }
+  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 {
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 5c382b0d578..f176e5479e1 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -637,6 +637,8 @@ public:
 
 private:
   Module *getOwningModuleSlow() const;
+protected:
+  bool hasLocalOwningModuleStorage() const;
 
 public:
   /// \brief Get the imported owning module, if this decl is from an imported
@@ -656,7 +658,7 @@ public:
     return reinterpret_cast<Module *const *>(this)[-1];
   }
   void setLocalOwningModule(Module *M) {
-    assert(!isFromASTFile() && Hidden &&
+    assert(!isFromASTFile() && Hidden && hasLocalOwningModuleStorage() &&
            "should not have a cached owning module");
     reinterpret_cast<Module **>(this)[-1] = M;
   }
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index 5bfee8b0d03..97192b53fa4 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -302,10 +302,14 @@ public:
     if (!D->isInIdentifierNamespace(IDNS))
       return nullptr;
 
-    if (isHiddenDeclarationVisible() || isVisible(getSema(), D))
+    if (isVisible(getSema(), D))
       return D;
 
-    return getAcceptableDeclSlow(D);
+    if (auto *Visible = getAcceptableDeclSlow(D))
+      return Visible;
+
+    // Even if hidden declarations are visible, prefer a visible declaration.
+    return isHiddenDeclarationVisible() ? D : nullptr;
   }
 
 private:
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 79cadcfcb16..70bd16ffdd5 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -80,6 +80,10 @@ Module *Decl::getOwningModuleSlow() const {
   return getASTContext().getExternalSource()->getModule(getOwningModuleID());
 }
 
+bool Decl::hasLocalOwningModuleStorage() const {
+  return getASTContext().getLangOpts().ModulesLocalVisibility;
+}
+
 const char *Decl::getDeclKindName() const {
   switch (DeclKind) {
   default: llvm_unreachable("Declaration not in DeclNodes.inc!");
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 2600e8e44d6..d0a55b57c61 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1180,6 +1180,16 @@ Module *Sema::getOwningModule(Decl *Entity) {
   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)
@@ -1218,9 +1228,11 @@ Module *Sema::getOwningModule(Decl *Entity) {
 }
 
 void Sema::makeMergedDefinitionVisible(NamedDecl *ND, SourceLocation Loc) {
-  auto *M = PP.getModuleContainingLocation(Loc);
-  assert(M && "hidden definition not in any module");
-  Context.mergeDefinitionIntoModule(ND, M);
+  if (auto *M = PP.getModuleContainingLocation(Loc))
+    Context.mergeDefinitionIntoModule(ND, M);
+  else
+    // We're not building a module; just make the definition visible.
+    ND->setHidden(false);
 }
 
 /// \brief Find the module in which the given declaration was defined.
diff --git a/clang/test/Modules/Inputs/template-default-args/a.h b/clang/test/Modules/Inputs/template-default-args/a.h
new file mode 100644
index 00000000000..1ef1ea5907b
--- /dev/null
+++ b/clang/test/Modules/Inputs/template-default-args/a.h
@@ -0,0 +1,4 @@
+template<typename T = int> struct A {};
+template<typename T> struct B {};
+template<typename T> struct C;
+template<typename T> struct D;
diff --git a/clang/test/Modules/Inputs/template-default-args/b.h b/clang/test/Modules/Inputs/template-default-args/b.h
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/clang/test/Modules/Inputs/template-default-args/b.h
diff --git a/clang/test/Modules/Inputs/template-default-args/module.modulemap b/clang/test/Modules/Inputs/template-default-args/module.modulemap
new file mode 100644
index 00000000000..6182e6b3eee
--- /dev/null
+++ b/clang/test/Modules/Inputs/template-default-args/module.modulemap
@@ -0,0 +1 @@
+module X { module A { header "a.h" } module B { header "b.h" } }
diff --git a/clang/test/Modules/template-default-args.cpp b/clang/test/Modules/template-default-args.cpp
new file mode 100644
index 00000000000..63187b8dc19
--- /dev/null
+++ b/clang/test/Modules/template-default-args.cpp
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -verify -fmodules-cache-path=%t -I %S/Inputs/template-default-args -std=c++11 %s
+//
+// expected-no-diagnostics
+
+template<typename T> struct A;
+template<typename T> struct B;
+template<typename T> struct C;
+template<typename T = int> struct D;
+
+#include "b.h"
+
+template<typename T = int> struct A {};
+template<typename T> struct B {};
+template<typename T = int> struct B;
+template<typename T = int> struct C;
+template<typename T> struct D {};
+
+A<> a;
+B<> b;
+extern C<> c;
+D<> d;