From 535ff8035458d859e3d309067a9ce45623daf269 Mon Sep 17 00:00:00 2001
From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Fri, 11 Sep 2015 22:39:35 +0000
Subject: [PATCH] [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                           | 17 ++++++++++++++++-
 clang/test/Modules/Inputs/template-default-args/a.h     |  4 ++++
 .../Inputs/template-default-args/module.modulemap       |  3 +++
 clang/test/Modules/template-default-args.cpp            |  2 ++
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 923da4e..bda535b 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/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.
diff --git a/clang/test/Modules/Inputs/template-default-args/a.h b/clang/test/Modules/Inputs/template-default-args/a.h
index a8a01a9..532cbc8 100644
--- a/clang/test/Modules/Inputs/template-default-args/a.h
+++ b/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
diff --git a/clang/test/Modules/Inputs/template-default-args/module.modulemap b/clang/test/Modules/Inputs/template-default-args/module.modulemap
index d54dfc3..21bf40c 100644
--- a/clang/test/Modules/Inputs/template-default-args/module.modulemap
+++ b/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" }
+}
diff --git a/clang/test/Modules/template-default-args.cpp b/clang/test/Modules/template-default-args.cpp
index 3a519f2..9d16cbf 100644
--- a/clang/test/Modules/template-default-args.cpp
+++ b/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
-- 
2.7.4