[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules
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
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
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
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
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
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