[Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules

[erik.pilkington]

Summary

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0961r1.html

I don't believe an actual defect report was filed for this, but the paper (in the "wording" section) claims that we should back-port this to C++17 as if it were a defect report. Besides that, this is pretty straightforward.

Thanks for taking a look!
Erik

[rsmith]

clang/lib/Sema/SemaDeclCXX.cpp

1118–1130

This should be done by walking the lookup results, not by filtering them. Testcase:

struct A {
  int get();
};
struct B {
  template<int> int get();
};
struct C : A, B {};
// plus specializations of tuple_size<C> and tuple_element<N, C>
auto [x] = C();

This should be ill-formed due to ambiguity when looking up C::get. We should not resolve the ambiguity by selecting B::get.

[erik.pilkington]

Address review comments.

clang/lib/Sema/SemaDeclCXX.cpp

1118–1130

Ah, I see. In the new patch we finish the class member access lookup, then we check this condition separately. I added this test to the patch. Thanks!

[rsmith]

LGTM with a small bugfix.

clang/lib/Sema/SemaDeclCXX.cpp

1122

You should use D->getUnderlyingDecl() rather than just D here, otherwise you'll reject this:

struct A {
  template<int> void get();
};
struct B : A {
  using A::get;
};

... because D will be a UsingShadowDecl not a FunctionTemplateDecl. (Please also add a corresponding testcase.)

clang/lib/Sema/SemaDeclCXX.cpp

 
   if (Bindings.empty())
     return false;
 
   DeclarationName GetDN = S.PP.getIdentifierInfo("get");
 
   // [dcl.decomp]p3:
   //   The unqualified-id get is looked up in the scope of E by class member
-  //   access lookup
+  //   access lookup ...
   LookupResult MemberGet(S, GetDN, Src->getLocation(), Sema::LookupMemberName);
   bool UseMemberGet = false;
   if (S.isCompleteType(Src->getLocation(), DecompType)) {
     if (auto *RD = DecompType->getAsCXXRecordDecl())
       S.LookupQualifiedName(MemberGet, RD);
     if (MemberGet.isAmbiguous())
       return true;
-    UseMemberGet = !MemberGet.empty();
+    //   ... and if that finds at least one declaration that is a function
+    //   template whose first template parameter is a non-type parameter ...
+    for (Decl *D : MemberGet) {
+      if (FunctionTemplateDecl *FTD = dyn_cast<FunctionTemplateDecl>(D)) {
+        TemplateParameterList *TPL = FTD->getTemplateParameters();
+        if (TPL->size() != 0 && isa<NonTypeTemplateParmDecl>(TPL->getParam(0))) {
+          //   ... the initializer is e.get<i>().
+          UseMemberGet = true;
+          break;
+        }
+      }
+    }
     S.FilterAcceptableTemplateNames(MemberGet);
   }
 
   unsigned I = 0;
   for (auto *B : Bindings) {
     BindingDiagnosticTrap Trap(S, B);
     SourceLocation Loc = B->getLocation();
 

clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp

 
 int get(A);
 
 void no_get_2() {
   // FIXME: This diagnostic is not great.
   auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
 }
 
-template<int> float &get(A);
+template<int> float &get(A); // expected-note 2 {{no known conversion}}
 
 void no_tuple_element_1() {
   auto [a0, a1, a2] = A(); // expected-error-re {{'std::tuple_element<0U{{L*}}, A>::type' does not name a type}} expected-note {{in implicit}}
 }
 
 namespace std { template<size_t, typename> struct tuple_element; } // expected-note 2{{here}}
 
 void no_tuple_element_2() {
   auto [a0, a1, a2] = A(); // expected-error {{implicit instantiation of undefined template 'std::tuple_element<0, A>'}} expected-note {{in implicit}}
 }
 
 template<> struct std::tuple_element<0, A> { typedef float type; };
 
 void no_tuple_element_3() {
   auto [a0, a1, a2] = A(); // expected-error {{implicit instantiation of undefined template 'std::tuple_element<1, A>'}} expected-note {{in implicit}}
 }
 
 template<> struct std::tuple_element<1, A> { typedef float &type; };
 template<> struct std::tuple_element<2, A> { typedef const float &type; };
 
-template<int N> auto get(B) -> int (&)[N + 1];
+template<int N> auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
 template<int N> struct std::tuple_element<N, B> { typedef int type[N +1 ]; };
 
 template<typename T> struct std::tuple_size<const T> : std::tuple_size<T> {};
 template<size_t N, typename T> struct std::tuple_element<N, const T> {
   typedef const typename std::tuple_element<N, T>::type type;
 };
 
 void referenced_type() {

 
 int member_get() {
   auto [c] = C();
   using T = int;
   using T = decltype(c);
   return c;
 }
 
-struct D { template<int> struct get {}; }; // expected-note {{declared here}}
+struct D {
+  // FIXME: Emit a note here explaining why this was ignored.
+  template<int> struct get {};
+};
 template<> struct std::tuple_size<D> { static const int value = 1; };
 template<> struct std::tuple_element<0, D> { typedef D::get<0> type; };
 void member_get_class_template() {
-  auto [d] = D(); // expected-error {{cannot refer to member 'get' in 'D' with '.'}} expected-note {{in implicit init}}
+  auto [d] = D(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
 }
 
-struct E { int get(); };
+struct E {
+  // FIXME: Emit a note here explaining why this was ignored.
+  int get();
+};
 template<> struct std::tuple_size<E> { static const int value = 1; };
 template<> struct std::tuple_element<0, E> { typedef int type; };
 void member_get_non_template() {
   // FIXME: This diagnostic is not very good.
-  auto [e] = E(); // expected-error {{no member named 'get'}} expected-note {{in implicit init}}
+  auto [e] = E(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
 }
 
 namespace ADL {
   struct X {};
 };
 template<int> int get(ADL::X);
 template<> struct std::tuple_size<ADL::X> { static const int value = 1; };
 template<> struct std::tuple_element<0, ADL::X> { typedef int type; };
constexpr int g() {
     {
       auto [a, b, c] = q;
       p = &c;
     }
     return *p; // expected-note {{read of object outside its lifetime}}
   }
   static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
 }
+
+// P0961R1
+struct InvalidMemberGet {
+  int get();
+  template <class T> int get();
+  struct get {};
+};
+template <> struct std::tuple_size<InvalidMemberGet> { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template <size_t> float get(InvalidMemberGet) { return 0; }
+int f() {
+  InvalidMemberGet img;
+  auto [x] = img;
+  typedef decltype(x) same_as_float;
+  typedef float same_as_float;
+}
+
+struct ValidMemberGet {
+  int get();
+  template <class T> int get() { return 0; }
+  template <size_t N> float get() { return 0; }
+};
+template <> struct std::tuple_size<ValidMemberGet> { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, ValidMemberGet> { typedef float type; };
+// Don't use this one; we should use the member get.
+template <size_t N> int get(ValidMemberGet) { static_assert(N && false, ""); }
+int f2() {
+  ValidMemberGet img;
+  auto [x] = img;
+  typedef decltype(x) same_as_float;
+  typedef float same_as_float;
+}
+
+struct Base1 {
+  int get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Base2 {
+  template<int> int get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Derived : Base1, Base2 {};
+
+template <> struct std::tuple_size<Derived> { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, Derived> { typedef int type; };
+
+auto [x] = Derived(); // expected-error{{member 'get' found in multiple base classes of different types}}

clang/www/cxx_status.html

     <tr>
       <td rowspan="3">Structured bindings</td>
       <td><a href="http://wg21.link/p0217r3">P0217R3</a></td>
       <td class="full" align="center">Clang 4</td>
     </tr>
       <tr>
         <!-- from Jacksonville 2018 -->
         <td><a href="http://wg21.link/p0961r1">P0961R1</a> (<a href="#dr">DR</a>)</td>
-        <td class="none" align="center">No</td>
+        <td class="none" align="center">SVN</td>
       </tr>
       <tr>
         <!-- from Jacksonville 2018 -->
         <td><a href="http://wg21.link/p0969r0">P0969R0</a> (<a href="#dr">DR</a>)</td>
         <td class="none" align="center">No</td>
       </tr>
     <tr>
       <td>Separate variable and condition for <tt>if</tt> and <tt>switch</tt></td>