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

2018-08-09 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339375: [Sema] P0961R1: Relaxing the structured bindings 
customization point finding… (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50418?vs=159629=159989#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50418

Files:
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
  cfe/trunk/www/cxx_status.html

Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -755,7 +755,7 @@
   
 
 http://wg21.link/p0961r1;>P0961R1 (DR)
-No
+SVN
   
   
 
Index: cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
   auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
 }
 
-template float (A);
+template float (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}}
@@ -57,7 +57,7 @@
 template<> struct std::tuple_element<1, A> { typedef float  };
 template<> struct std::tuple_element<2, A> { typedef const float  };
 
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
 template struct std::tuple_element { typedef int type[N +1 ]; };
 
 template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
   return c;
 }
 
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+  // FIXME: Emit a note here explaining why this was ignored.
+  template struct get {};
+};
 template<> struct std::tuple_size { 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 { 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 {
@@ -230,3 +236,62 @@
   }
   static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
 }
+
+// P0961R1
+struct InvalidMemberGet {
+  int get();
+  template  int get();
+  struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template  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  int get() { return 0; }
+  template  float get() { return 0; }
+};
+template <> struct std::tuple_size { 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  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 get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Derived : Base1, Base2 {};
+
+template <> struct std::tuple_size { 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}}
+
+struct Base {
+  template int get();
+};
+struct UsingGet : Base {
+  using Base::get;
+};
+
+template <> struct std::tuple_size {
+  static constexpr size_t value = 1;
+};
+template <> struct std::tuple_element<0, UsingGet> { typedef int type; };
+
+auto [y] = UsingGet();
Index: 

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

2018-08-09 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM with a small bugfix.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1122
+for (Decl *D : MemberGet) {
+  if (FunctionTemplateDecl *FTD = dyn_cast(D)) {
+TemplateParameterList *TPL = FTD->getTemplateParameters();

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

```
struct A {
  template void get();
};
struct B : A {
  using A::get;
};
```

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


https://reviews.llvm.org/D50418



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-08-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1118-1130
+//   ... and if that finds at least one declaration that is a function
+//   template whose first template parameter is a non-type parameter ...
+LookupResult::Filter Filter = MemberGet.makeFilter();
+while (Filter.hasNext()) {
+  NamedDecl *ND = Filter.next();
+  if (auto *FTD = dyn_cast(ND)) {
+TemplateParameterList *TPL = FTD->getTemplateParameters();

rsmith wrote:
> This should be done by walking the lookup results, not by filtering them. 
> Testcase:
> 
> ```
> struct A {
>   int get();
> };
> struct B {
>   template int get();
> };
> struct C : A, B {};
> // plus specializations of tuple_size and tuple_element
> 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`.
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!


https://reviews.llvm.org/D50418



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-08-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington updated this revision to Diff 159629.
erik.pilkington added a comment.

Address review comments.


https://reviews.llvm.org/D50418

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -755,7 +755,7 @@
   
 
 http://wg21.link/p0961r1;>P0961R1 (DR)
-No
+SVN
   
   
 
Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
   auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
 }
 
-template float (A);
+template float (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}}
@@ -57,7 +57,7 @@
 template<> struct std::tuple_element<1, A> { typedef float  };
 template<> struct std::tuple_element<2, A> { typedef const float  };
 
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
 template struct std::tuple_element { typedef int type[N +1 ]; };
 
 template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
   return c;
 }
 
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+  // FIXME: Emit a note here explaining why this was ignored.
+  template struct get {};
+};
 template<> struct std::tuple_size { 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 { 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 {
@@ -230,3 +236,48 @@
   }
   static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
 }
+
+// P0961R1
+struct InvalidMemberGet {
+  int get();
+  template  int get();
+  struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template  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  int get() { return 0; }
+  template  float get() { return 0; }
+};
+template <> struct std::tuple_size { 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  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 get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Derived : Base1, Base2 {};
+
+template <> struct std::tuple_size { 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}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1108,15 +1108,26 @@
 
   // [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())
   

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

2018-08-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1118-1130
+//   ... and if that finds at least one declaration that is a function
+//   template whose first template parameter is a non-type parameter ...
+LookupResult::Filter Filter = MemberGet.makeFilter();
+while (Filter.hasNext()) {
+  NamedDecl *ND = Filter.next();
+  if (auto *FTD = dyn_cast(ND)) {
+TemplateParameterList *TPL = FTD->getTemplateParameters();

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

```
struct A {
  int get();
};
struct B {
  template int get();
};
struct C : A, B {};
// plus specializations of tuple_size and tuple_element
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`.


Repository:
  rC Clang

https://reviews.llvm.org/D50418



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-08-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.

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


Repository:
  rC Clang

https://reviews.llvm.org/D50418

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -755,7 +755,7 @@
   
 
 http://wg21.link/p0961r1;>P0961R1 (DR)
-No
+SVN
   
   
 
Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
   auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
 }
 
-template float (A);
+template float (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}}
@@ -57,7 +57,7 @@
 template<> struct std::tuple_element<1, A> { typedef float  };
 template<> struct std::tuple_element<2, A> { typedef const float  };
 
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
 template struct std::tuple_element { typedef int type[N +1 ]; };
 
 template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
   return c;
 }
 
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+  // FIXME: Emit a note here explaining why this was ignored.
+  template struct get {};
+};
 template<> struct std::tuple_size { 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 { 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 {
@@ -230,3 +236,35 @@
   }
   static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
 }
+
+// P0961R1
+struct InvalidMemberGet {
+  int get();
+  template  int get();
+  struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template  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  int get() { return 0; }
+  template  float get() { return 0; }
+};
+template <> struct std::tuple_size { 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  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;
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1108,14 +1108,29 @@
 
   // [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);
+
+//   ... and if that finds at least one declaration that is a function
+//   template whose first