From 50c3b31af6e71162f7c93e042341ad10c640259f Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Wed, 1 May 2019 16:03:04 -0400 Subject: [PATCH 1/2] Should `int(*)()` be convertible to `inplace_function`? Before this patch, we reported that `is_convertible>`, but if you actually tried to do it, you got a compiler error down in the guts of `inplace_function`. After this patch, we let you do it. However, see issue #150: I think anyone who relies on this behavior is probably doing so unintentionally (and their code is probably wrong). Therefore, maybe we should change our SFINAE so that `is_convertible>` would be `false`. --- SG14/inplace_function.h | 8 +++++--- SG14_test/inplace_function_test.cpp | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/SG14/inplace_function.h b/SG14/inplace_function.h index 59453966..00306de3 100644 --- a/SG14/inplace_function.h +++ b/SG14/inplace_function.h @@ -105,9 +105,11 @@ template struct vtable template explicit constexpr vtable(wrapper) noexcept : invoke_ptr{ [](storage_ptr_t storage_ptr, Args&&... args) -> R - { return (*static_cast(storage_ptr))( - static_cast(args)... - ); } + { + return static_cast((*static_cast(storage_ptr))( + static_cast(args)... + )); + } }, copy_ptr{ [](storage_ptr_t dst_ptr, storage_ptr_t src_ptr) -> void { ::new (dst_ptr) C{ (*static_cast(src_ptr)) }; } diff --git a/SG14_test/inplace_function_test.cpp b/SG14_test/inplace_function_test.cpp index 6aaffff3..2ca870a0 100644 --- a/SG14_test/inplace_function_test.cpp +++ b/SG14_test/inplace_function_test.cpp @@ -510,6 +510,21 @@ static void test_convertibility_with_lambdas() static_assert(!std::is_convertible>::value, ""); } +static void test_void_returning_function_runtime() +{ + auto lambda = []() { return 42; }; + static_assert(std::is_convertible>::value, ""); + static_assert(std::is_convertible>::value, ""); + + stdext::inplace_function f = lambda; + f = lambda; + f(); + + stdext::inplace_function g = lambda; + g = lambda; + g(); +} + namespace { struct InstrumentedCopyConstructor { static int copies; @@ -703,6 +718,7 @@ void sg14_test::inplace_function_test() test_is_convertible(); test_convertibility_with_qualified_call_operators(); test_convertibility_with_lambdas(); + test_void_returning_function_runtime(); test_return_by_move(); test_is_invocable(); test_overloading_on_arity(); From 191f893be2f5d946433a02e675f459b5d5493c90 Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Thu, 23 May 2019 14:08:46 -0400 Subject: [PATCH 2/2] inplace_function: is_convertible>. Put the "standard" behavior under a macro flag, and default to the "safer" behavior. That is, disallow the conversion by default. --- SG14/inplace_function.h | 11 +++++++ SG14_test/inplace_function_test.cpp | 50 +++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/SG14/inplace_function.h b/SG14/inplace_function.h index 00306de3..facce9e8 100644 --- a/SG14/inplace_function.h +++ b/SG14/inplace_function.h @@ -153,20 +153,31 @@ struct is_valid_inplace_dst : std::true_type }; // C++11 MSVC compatible implementation of std::is_invocable_r. +// By default, our is_invocable_r is FALSE, +// even though std::is_invocable_r is TRUE. +// Set SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID if you want that behavior. template void accept(R); template struct is_invocable_r_impl : std::false_type {}; template struct is_invocable_r_impl< +#if SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID decltype(std::declval()(std::declval()...), void()), +#else + decltype(std::declval()(std::declval()...)), +#endif void, F, Args... > : std::true_type {}; template struct is_invocable_r_impl< +#if SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID decltype(std::declval()(std::declval()...), void()), +#else + decltype(std::declval()(std::declval()...)), +#endif const void, F, Args... diff --git a/SG14_test/inplace_function_test.cpp b/SG14_test/inplace_function_test.cpp index 2ca870a0..52702cec 100644 --- a/SG14_test/inplace_function_test.cpp +++ b/SG14_test/inplace_function_test.cpp @@ -476,6 +476,7 @@ static void test_convertibility_with_lambdas() const auto a = []() -> int { return 3; }; static_assert(std::is_convertible>::value, ""); + static_assert(std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); @@ -485,7 +486,13 @@ static void test_convertibility_with_lambdas() static_assert(!std::is_convertible>::value, ""); const auto c = [](int, NoDefaultCtor) -> int { return 3; }; + static_assert(std::is_convertible>::value, ""); + static_assert(std::is_convertible>::value, ""); +#if SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID static_assert(std::is_convertible>::value, ""); +#else + static_assert(!std::is_convertible>::value, ""); +#endif static_assert(!std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); @@ -500,12 +507,14 @@ static void test_convertibility_with_lambdas() // Same as a, but not const. auto e = []() -> int { return 3; }; static_assert(std::is_convertible>::value, ""); + static_assert(std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); // Same as a, but not const and mutable. auto f = []() mutable -> int { return 3; }; static_assert(std::is_convertible>::value, ""); + static_assert(std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); static_assert(!std::is_convertible>::value, ""); } @@ -513,16 +522,15 @@ static void test_convertibility_with_lambdas() static void test_void_returning_function_runtime() { auto lambda = []() { return 42; }; +#if SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID static_assert(std::is_convertible>::value, ""); - static_assert(std::is_convertible>::value, ""); stdext::inplace_function f = lambda; f = lambda; f(); - - stdext::inplace_function g = lambda; - g = lambda; - g(); +#else + static_assert(!std::is_convertible>::value, ""); +#endif } namespace { @@ -583,11 +591,22 @@ static void test_is_invocable() static_assert(! is_invocable_r::value, ""); static_assert(! is_invocable_r::value, ""); + static_assert(is_invocable_r::value, ""); + static_assert(! is_invocable_r::value, ""); + static_assert(! is_invocable_r::value, ""); + static_assert(is_invocable_r::value, ""); static_assert(! is_invocable_r::value, ""); + static_assert(is_invocable_r::value, ""); + static_assert(! is_invocable_r::value, ""); + // Testing widening and narrowing conversions, and the "conversion" to void. +#if SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID static_assert(is_invocable_r::value, ""); +#else + static_assert(! is_invocable_r::value, ""); +#endif static_assert(is_invocable_r::value, ""); static_assert(is_invocable_r::value, ""); @@ -598,9 +617,14 @@ static void test_is_invocable() // > Determines whether Fn can be invoked with the arguments ArgTypes... // > to yield a result that is convertible to R. // - // void is treated specially because a functions return value can be ignored. + // void is treated specially because a function's return value can be ignored. +#if SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID static_assert(is_invocable_r::value, ""); static_assert(is_invocable_r::value, ""); +#else + static_assert(! is_invocable_r::value, ""); + static_assert(! is_invocable_r::value, ""); +#endif // Regression tests for both is_invocable and is_convertible. static_assert(is_invocable_r::value, ""); @@ -631,6 +655,17 @@ static void test_overloading_on_return_type() EXPECT_EQ(overloaded_function3([](int) { return nullptr; }), 2); } +#if !(SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID) +static int overloaded_function4(stdext::inplace_function) { return 1; } +static int overloaded_function4(stdext::inplace_function) { return 2; } + +static void test_overloading_on_return_type_void() +{ + EXPECT_EQ(overloaded_function4([]() -> void {}), 1); + EXPECT_EQ(overloaded_function4([]() -> int { return 0; }), 2); +} +#endif + void sg14_test::inplace_function_test() { // first set of tests (from Optiver) @@ -724,6 +759,9 @@ void sg14_test::inplace_function_test() test_overloading_on_arity(); test_overloading_on_parameter_type(); test_overloading_on_return_type(); +#if !(SG14_INPLACE_FUNCTION_INVOCABLE_AS_VOID) + test_overloading_on_return_type_void(); +#endif } #ifdef TEST_MAIN