blackbird icon indicating copy to clipboard operation
blackbird copied to clipboard

added perfect forwarding to the existing combinators

Open boris-kuz opened this issue 2 years ago • 1 comments

Cool idea for a library! Was playing around with it a little bit after watching your The Twin Algorithms talk and found this small "issue", though I'm not sure if you want to call it that. By always passing the argument by value in the resulting combined function, things like these don't work

int a{5};
auto modifying_function = [](auto& x){x++; return x;};
auto identity = [](auto x){return std::forward<decltype(x)>(x);};
auto combined = combinators::_b(identity, modifying_function);
combined(x);
assert(a == 6); //this will fail, a will be 5 since combined is capturing by value.

Again, this behavior be intended, so feel free to reject!

Best wishes, Boris

boris-kuz avatar Jul 26 '22 21:07 boris-kuz

This is a language quirk of pointers/references that causes a sharp corner in this otherwise very nice "C++ combinator DSL".

If you were to replace your int with a pointer to an int then this would work.

So I feel like I can argue that one way to fix this bug is to always have C++ mutate the underlying data (using vectors, spans, pointers to refer to data so when we copy the handles instead of copying and making stack local mutations by default). You can enforce this with a type checker or a linter that throws an error whenever you try to use a "raw" data type.

But that's kinda a silly argument 😁

A better argument is to always mutate the underlying data with C++'s simple feature: the owning reference AKA rvalue references (no matter what is passed in, a l-value, an l-value reference, or an r-value) AKA as I think of them std::moveing references AKA forwarding references AKA just use perfect forwarding.

Well, we just have to use perfect forwarding because C++ has imperfect forwarding.

But this library is a love letter to combinator languages, so admitting the ugly underlying stuff to make correct code in a language that doesn't prioritize that feels like fighting the language of C++ instead of putting love on the style of thinking necessary in APL or BQN.

I wonder if a version that uses overloads for const lvalues and rvalues would look nicer (as mentioned in Item 25 of Meyers Effective Modern C++). Yeah, having these overloads can thus cause copying to happen, but.. well, one way to avoid having copies happen is to just use a pointer type. Or making it a compiler error to pass a l-value reference (would be nice if we could use something like std::enable_if here)

AriSweedler avatar Sep 05 '23 07:09 AriSweedler