From 9c31c8ac5cee1292b4537cf2c483ece44a19fcfc Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Sat, 25 Jul 2020 21:26:57 -0700 Subject: [PATCH 1/2] Increase _Pass_fn's size threshold ... to handle member-pointers that are larger than `void*` correctly. Fixes GH-1089. --- stl/inc/xutility | 5 ++- .../test.cpp | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index f80ed07fa86..9161552722b 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -197,9 +197,8 @@ struct _Ref_fn { // pass function object by value as a reference }; template -_INLINE_VAR constexpr bool - _Pass_functor_by_value_v = sizeof(_Fn) <= sizeof(void*) - && conjunction_v, is_trivially_destructible<_Fn>>; +_INLINE_VAR constexpr bool _Pass_functor_by_value_v = conjunction_v, + is_trivially_copy_constructible<_Fn>, is_trivially_destructible<_Fn>>; template , int> = 0> // TRANSITION, if constexpr constexpr _Fn _Pass_fn(_Fn _Val) { // pass functor by value diff --git a/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp b/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp index ff3beb645d5..a61e4c555cb 100644 --- a/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp +++ b/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp @@ -800,3 +800,38 @@ namespace sortable_test { } } } // namespace sortable_test + +namespace gh_1089 { + // Defend against regression of GH-1089: "_Pass_fn/_Ref_fn interferes with the Ranges invoke protocol" + // The _Pass_fn protocol would previously assume that anything larger than a pointer was a function object that it + // could call with `()` and not a pointer-to-member that requires the `invoke` protocol. + + void test() { + struct Base { + virtual int purr() = 0; + }; + + struct Derived1 : virtual Base { + int purr() override { + return 1729; + } + }; + + struct Derived2 : virtual Base {}; + + struct MostDerived : Derived1, Derived2 { + int purr() override { + return 2020; + } + }; + + STATIC_ASSERT(sizeof(&Derived1::purr) == 3 * sizeof(void*)); // NB: relies on non-portable platform properties + + Derived1 a[2]; + MostDerived b[3]; + Derived1* pointers[] = {&b[0], &a[0], &b[1], &a[1], &b[2]}; + + (void) ranges::count(pointers, 1729, &Derived1::purr); + (void) ranges::count(pointers, 2020, &Derived1::purr); + } +} // namespace gh_1089 From 94fe8c695d0288b79da96dbd15d804ea316a7759 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Sun, 26 Jul 2020 10:34:42 -0700 Subject: [PATCH 2/2] STL's review comments --- stl/inc/xutility | 11 +++- .../test.cpp | 59 ++++++++++++------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index 9161552722b..565eaee50d9 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -190,14 +190,21 @@ template struct _Ref_fn { // pass function object by value as a reference template constexpr decltype(auto) operator()(_Args&&... _Vals) { // forward function call operator - return _Fn(_STD forward<_Args>(_Vals)...); +#if _HAS_IF_CONSTEXPR + if constexpr (is_member_pointer_v<_Fx>) { + return _STD invoke(_Fn, _STD forward<_Args>(_Vals)...); + } else +#endif // _HAS_IF_CONSTEXPR + { + return _Fn(_STD forward<_Args>(_Vals)...); + } } _Fx& _Fn; }; template -_INLINE_VAR constexpr bool _Pass_functor_by_value_v = conjunction_v, +_INLINE_VAR constexpr bool _Pass_functor_by_value_v = conjunction_v, is_trivially_copy_constructible<_Fn>, is_trivially_destructible<_Fn>>; template , int> = 0> // TRANSITION, if constexpr diff --git a/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp b/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp index a61e4c555cb..7628e82238b 100644 --- a/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp +++ b/tests/std/tests/P0896R4_ranges_algorithm_machinery/test.cpp @@ -807,31 +807,50 @@ namespace gh_1089 { // could call with `()` and not a pointer-to-member that requires the `invoke` protocol. void test() { - struct Base { - virtual int purr() = 0; - }; + { + struct Base { + virtual int purr() = 0; + }; - struct Derived1 : virtual Base { - int purr() override { - return 1729; - } - }; + struct Derived1 : virtual Base { + int purr() override { + return 1729; + } + }; - struct Derived2 : virtual Base {}; + struct Derived2 : virtual Base {}; - struct MostDerived : Derived1, Derived2 { - int purr() override { - return 2020; - } - }; + struct MostDerived : Derived1, Derived2 { + int purr() override { + return 2020; + } + }; - STATIC_ASSERT(sizeof(&Derived1::purr) == 3 * sizeof(void*)); // NB: relies on non-portable platform properties - Derived1 a[2]; - MostDerived b[3]; - Derived1* pointers[] = {&b[0], &a[0], &b[1], &a[1], &b[2]}; + STATIC_ASSERT(sizeof(&Derived1::purr) > sizeof(void*)); // NB: relies on non-portable platform properties - (void) ranges::count(pointers, 1729, &Derived1::purr); - (void) ranges::count(pointers, 2020, &Derived1::purr); + Derived1 a[2]; + MostDerived b[3]; + Derived1* pointers[] = {&b[0], &a[0], &b[1], &a[1], &b[2]}; + + (void) ranges::count(pointers, 2020, &Derived1::purr); + } + { + struct Cat; + + using PMD_Cat = int Cat::*; + // Quantum effects: we must observe the size before defining Cat or it will become smaller. + STATIC_ASSERT(sizeof(PMD_Cat) > sizeof(void*)); + + struct Cat { + int x = 42; + }; + + STATIC_ASSERT(sizeof(&Cat::x) > sizeof(void*)); // NB: relies on non-portable platform properties + + Cat cats[42]; + + (void) ranges::count(cats, 42, &Cat::x); + } } } // namespace gh_1089