Add int overrides for (some) size_t params as a convenience for fplus::fwd?

[pthom]

Hello!

I encountered a difficulty when using fwd::drop: it won't accept integers, unless you loudly cast them to size_t:

See the example below:

TEST_CASE("drop - fwd - int")
{
    const std::vector<int> numbers = {1, 2, 3};

    //  This call works in the fplus namespace, without cast
    REQUIRE_EQ(fplus::drop(2, numbers), std::vector<int>{3});

   // This call in the fplus::fwd namespace works, but requires a cumbersome static_cast
    REQUIRE_EQ(fplus::fwd::drop(static_cast<size_t>(2))(numbers), std::vector<int>{3});

    // The version below does not compile, with this message:
    //     In file included from include/fplus/fwd.hpp:119:
    //     fplus/fwd_instances.autogenerated_defines:48:1: error: implicit conversion changes signedness: 'const int' to 'std::size_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
    //     fplus_fwd_define_fn_1(drop)
    //
    // REQUIRE_EQ(fplus::fwd::drop(2)(numbers), std::vector<int>{3});
}

I noticed that adding this definition just below drop's definition inside container_common.hpp can solve this:

template <typename Container,
    typename ContainerOut = internal::remove_const_and_ref_t<Container>>
ContainerOut drop(int amount, Container&& xs)
{
    assert(amount >= 0);
    size_t amount_s = static_cast<size_t>(amount);
    return drop(amount_s, xs);
}

However, I do not know the (possibly numerous) implications of adding overrides for int to fplus::drop/take/drop_last/etc.

What do you think?

[Dobiasd]

Oh, interesting. And good investigation. :+1:

So far, I tried to avoid overloaded functions, e.g., to make it easier to pass them around as arguments (no ambiguity for a certain function name, etc.).

But of course, I see the ugliness of the explicit cast.

Until C++23 will save the day, what do you think about defining your own literal operator in your codebase?

[pthom]

what do you think about defining your own literal operator in your codebase?

I think it is the reasonable approach, since I tested to override more functions, and of course I ran into ambiguities (see below for more details if you are interested).

Until C++23 will save the day

better late than never ;-)

---

More details when trying to override more functions. I took the quick and (not so) dirty macro way:

#define add_int_override_sizet_container(fn) \
template <typename Container, \
    typename ContainerOut = internal::remove_const_and_ref_t<Container>> \
ContainerOut fn(int amount, Container&& xs) \
{ \
    assert(amount >= 0); \
    size_t amount_s = static_cast<size_t>(amount); \
    return fn(amount_s, xs); \
}

add_int_override_sizet_container(drop);
add_int_override_sizet_container(take);
add_int_override_sizet_container(repeat);
add_int_override_sizet_container(stride);
add_int_override_sizet_container(shuffle);
add_int_override_sizet_container(drop_idx);
add_int_override_sizet_container(drop_last);
add_int_override_sizet_container(take_last);
add_int_override_sizet_container(drop_exact);
add_int_override_sizet_container(take_exact);
add_int_override_sizet_container(take_cyclic);
add_int_override_sizet_container(partial_sort);
add_int_override_sizet_container(replicate_elems);

...

And as expected, I got quite a lot of errors when compiling container_common_test.cpp (ambiguous calls and conversion errors).

...

include/fplus/container_common.hpp:2285:1: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
add_int_override_sizet_container(repeat);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fplus/include/fplus/container_common.hpp:2280:15: note: expanded from macro 'add_int_override_sizet_container'
return fn(amount_s, xs); \
           ~~ ^~~~~~~~
fplus/test/container_common_test.cpp:238:16: note: in instantiation of function template specialization 'fplus::repeat<std::vector<int> &, std::vector<int>>' requested here
REQUIRE_EQ(repeat(2, xs), xs2Times);
^
In file included from fplus/test/container_common_test.cpp:8:
In file included from fplus/include/fplus/fplus.hpp:11:
fplus/include/fplus/container_common.hpp:2293:34: error: call to 'take_cyclic' is ambiguous
add_int_override_sizet_container(take_cyclic);
^~~~~~~~~~~
fplus/include/fplus/container_common.hpp:2280:12: note: expanded from macro 'add_int_override_sizet_container'
return fn(amount_s, xs); \
           ^~
fplus/test/container_common_test.cpp:612:16: note: in instantiation of function template specialization 'fplus::take_cyclic<std::vector<int>, std::vector<int>>' requested here
REQUIRE_EQ(take_cyclic(5, IntVector({0,1,2,3})), IntVector({0,1,2,3,0}));
^
fplus/include/fplus/container_common.hpp:766:11: note: candidate function [with Container = std::vector<int>]
Container take_cyclic(std::size_t amount, const Container& xs)
^
fplus/include/fplus/container_common.hpp:2293:34: note: candidate function [with Container = std::vector<int> &, ContainerOut = std::vector<int>]
add_int_override_sizet_container(take_cyclic);
^
fplus/include/fplus/container_common.hpp:2286:1: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
add_int_override_sizet_container(stride);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fplus/include/fplus/container_common.hpp:2280:15: note: expanded from macro 'add_int_override_sizet_container'
return fn(amount_s, xs); \
           ~~ ^~~~~~~~
fplus/test/container_common_test.cpp:731:16: note: in instantiation of function template specialization 'fplus::stride<std::vector<int>, std::vector<int>>' requested here
REQUIRE_EQ(stride(3, IntVector({0,1,2,3,4,5,6,7})), IntVector({0,3,6}));
^
In file included from fplus/test/container_common_test.cpp:8:
In file included from fplus/include/fplus/fplus.hpp:11:
fplus/include/fplus/container_common.hpp:2286:1: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
add_int_override_sizet_container(stride);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fplus/include/fplus/container_common.hpp:2280:15: note: expanded from macro 'add_int_override_sizet_container'
return fn(amount_s, xs); \
           ~~ ^~~~~~~~
fplus/include/fplus/container_common.hpp:2286:34: note: in instantiation of function template specialization 'fplus::stride<std::vector<int> &, std::vector<int>>' requested here
add_int_override_sizet_container(stride);
^
fplus/test/container_common_test.cpp:731:16: note: in instantiation of function template specialization 'fplus::stride<std::vector<int>, std::vector<int>>' requested here
REQUIRE_EQ(stride(3, IntVector({0,1,2,3,4,5,6,7})), IntVector({0,3,6}));
^
4 errors generated.
make[2]: *** [test/CMakeFiles/container_common_test.dir/container_common_test.cpp.o] Error 1
make[1]: *** [test/CMakeFiles/container_common_test.dir/all] Error 2
make: *** [all] Error 2

I think we can close this issue!

[Dobiasd]

Yeah, custom literal operators seem much better compared to going through function-overloading hell. :grin: