[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.

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

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 451f9da1b60acec311c5c02c304d7b1655c40f1e..e06b58b37be26fbea8041c8e99c3bd579d7b49df 100644 (file)
--- a/include/clang/AST/Decl.h
+++ b/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/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 5c382b0d57807419483fe345260468515c44d0da..f176e5479e1e64e273331183a5a3e162267c9c95 100644 (file)
--- a/include/clang/AST/DeclBase.h
+++ b/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/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h
index 5bfee8b0d037cb52ca1dbaa9721666556510eeb1..97192b53fa441c6bd8b7d780ab6dbc5869f8a3b6 100644 (file)
--- a/include/clang/Sema/Lookup.h
+++ b/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/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 79cadcfcb167bccfd7f730a8fb8c41494bdda1f7..70bd16ffdd5db1180f894496ff4d4800f6ac9990 100644 (file)
--- a/lib/AST/DeclBase.cpp
+++ b/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/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 2600e8e44d6c9d6bf06f42d2239a264a2ca8fb21..d0a55b57c61fffb71eb2c9c9cca8a1008747180c 100644 (file)
--- a/lib/Sema/SemaLookup.cpp
+++ b/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/test/Modules/Inputs/template-default-args/a.h b/test/Modules/Inputs/template-default-args/a.h
new file mode 100644 (file)
index 0000000..1ef1ea5
--- /dev/null
+++ b/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/test/Modules/Inputs/template-default-args/b.h b/test/Modules/Inputs/template-default-args/b.h
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/test/Modules/Inputs/template-default-args/module.modulemap b/test/Modules/Inputs/template-default-args/module.modulemap
new file mode 100644 (file)
index 0000000..6182e6b
--- /dev/null
+++ b/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/test/Modules/template-default-args.cpp b/test/Modules/template-default-args.cpp
new file mode 100644 (file)
index 0000000..63187b8
--- /dev/null
+++ b/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;