[modules] When picking one of two template declarations as a lookup result,

it's not sufficient to prefer the declaration with more default arguments, or
the one that's visible; they might both be visible, but one of them might have
a visible default argument where the other has a hidden default argument.

llvm-svn: 247486

clang/lib/Sema/SemaLookup.cpp

@@ -387,6 +387,8 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind,
     // If D has more default arguments, it is preferred.
     if (DMin != EMin)
       return DMin < EMin;
+    // FIXME: When we track visibility for default function arguments, check
+    // that we pick the declaration with more visible default arguments.
   }
 
   // Pick the template with more default template arguments.
@@ -394,9 +396,22 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind,
     auto *ETD = cast<TemplateDecl>(EUnderlying);
     unsigned DMin = DTD->getTemplateParameters()->getMinRequiredArguments();
     unsigned EMin = ETD->getTemplateParameters()->getMinRequiredArguments();
-    // If D has more default arguments, it is preferred.
+    // If D has more default arguments, it is preferred. Note that default
+    // arguments (and their visibility) is monotonically increasing across the
+    // redeclaration chain, so this is a quick proxy for "is more recent".
     if (DMin != EMin)
       return DMin < EMin;
+    // If D has more *visible* default arguments, it is preferred. Note, an
+    // earlier default argument being visible does not imply that a later
+    // default argument is visible, so we can't just check the first one.
+    for (unsigned I = DMin, N = DTD->getTemplateParameters()->size();
+        I != N; ++I) {
+      if (!S.hasVisibleDefaultArgument(
+              ETD->getTemplateParameters()->getParam(I)) &&
+          S.hasVisibleDefaultArgument(
+              DTD->getTemplateParameters()->getParam(I)))
+        return true;
+    }
   }
 
   // For most kinds of declaration, it doesn't really matter which one we pick.

clang/test/Modules/Inputs/template-default-args/a.h

@@ -9,4 +9,8 @@ template<typename T = int> struct H;
 template<typename T> struct J {};
 template<typename T = int> struct J;
 struct K : J<> {};
+template<typename T = void> struct L;
+struct FriendL {
+  template<typename T> friend struct L;
+};
 END

clang/test/Modules/Inputs/template-default-args/module.modulemap

@@ -3,3 +3,6 @@ module X {
   module B { header "b.h" }
   module C { header "c.h" }
 }
+module Y {
+  module D { header "d.h" }
+}

clang/test/Modules/template-default-args.cpp

@@ -13,6 +13,7 @@ template<typename T = int, typename U = int> struct I {};
 END
 
 #include "b.h"
+#include "d.h"
 
 BEGIN
 template<typename T = int> struct A {};
@@ -41,4 +42,5 @@ G<> g; // expected-error {{default argument of 'G' must be imported from module
 H<> h; // expected-error {{default argument of 'H' must be imported from module 'X.A' before it is required}}
 // expected-note@a.h:8 {{default argument declared here}}
 I<> i;
+L<> *l;
 END