Open nextsilicon-itay-bookstein opened 1 year ago
I went and discussed this issue with a colleague, and these are the points I'd like to argue :)
There's no robust general container-agnostic way of rebinding containers, even when trying to look beyond https://en.cppreference.com/w/cpp/named_req/Container + C::allocator_type
-if-present. The current implementation basically makes the following two assumptions, if I understand it correctly:
C<T>
or C<T, A>
.C::value_type
and C::allocator_type
(i.e. they don't just 'happen to be' equal, but they are always directly set from the template type parameters).If you consider std::unordered_map
as a counter-example (maybe you want to rebind std::unordered_map<int, cti::coninuable<int>>
to std::unordered_map<int, int>
; I know it's not within the current design to do that, I'm just using it to illustrate the point):
value_type = std::pair<const K, V>
.NewType
, but for the new value_type
.Barring any container-provided rebinding machinery (a-la allocators), perhaps it's 'safer' to provide a per-type rebind logic and leave it as an extension point for users wanting to provide containers that don't fit the mold, while falling back to rebinding to e.g. an std::vector
when none of these options pan out?
Having the SFINAE machinery match against a container and decide to rebind it, but then bail out to the propagate-plain-value case is quite dangerous (especially when it's for continuables returning void and you have no type-safety machinery in the .then()
to save you). Perhaps it should cause a compiler error in these cases?
What do you think?
Hi @ibookstein , I'm glad that you like my library.
I fully agree with you reproduction as well as your second argumentation.
std::vector
or similar is the ideal input:
Arbitrary arguments which are connected. Every type is allowed as arguments, continuables may be contained inside tuple like types (std::tuple) or in homogeneous containers such as std::vector. Non continuable arguments are preserved and passed to the final result as shown below:
Probably the best way to solve this is to provide a trait that performs the remapping. The trait could only be used if the potentially remapped type provides a begin()
and end()
method e.g. satisfies the concept of an iterable. Then we could guard the remapping with a hard static_assert
in case no remapper is provided for the iterable.
Sadly I'm currently considering this of scope feature-wise. For most use-cases std::vector
should be enough. I'm open for a PR that adds this feature but I won't add it by myself in the near future.
Hi, @Naios !
Thanks for making
continuable
, it's a neat library!I passed an
absl::InlinedVector<T, N, A>
tocti::when_all()
, and to my surprise the continuables in it did not get connected to the hierarchy. Investigating further revealed that because of a substitution failure deep inside the container-remapping code, the code degraded from using the container logic to using the plain-value logic (for values "that aren't accepted by the mapper"). Specifically, it failed substitution forrebind_container()
. If I understand correctly, it failed because the template type signature ofabsl::InlinedVector
did not match the ones thatrebind_container()
tries to accept. I'm wondering whether it should somehow match against traits rather than template type parameter shape?Commit Hash
Used
continuable/4.2.0
andabseil/20210324.2
from conan, but I doubt it matters much.Expected Behavior
Print "Connection timed out\n"
Actual Behavior
Print "hi\n" + trap on exceptional unhandled continuable (SIGILL).
If the code is changed to use
std::vector
instead, expected behavior is observed.Steps to Reproduce
Example code:
Your Environment