arximboldi/lager

lager::with() and lager::reader<tuple<...>> behave in a non-symmetrical way

dimula73 opened this issue · 8 comments

I have a feeling like there is some bug in mapping readers holding a tuple. It seems to be impossible to map-unzip them into a normal function. Here is a code example:

#include <zug/tuplify.hpp>
#include <zug/transducer/zip.hpp>
#include <zug/transducer/unzip.hpp>

TEST_CASE("tuple unfolding")
{
    auto callback = [] ([[maybe_unused]] int x, [[maybe_unused]] std::string y) {
        return std::to_string(x) + "-" + y;
    };

    auto callback_tuple = [] ([[maybe_unused]] std::tuple<int, std::string> x) {
        return std::to_string(std::get<0>(x)) + "-" + std::get<1>(x);
    };


    state<int, automatic_tag> value1{7};
    state<std::string, automatic_tag> value2{"value"};


    reader<std::string> converted_value{with(value1, value2).map(callback)};
    reader<std::string> converted_value2{with(value1, value2).xform(zug::zip).map(callback_tuple)};

    CHECK(converted_value.get() == "7-value");
    CHECK(converted_value2.get() == "7-value");

    reader<std::tuple<int, std::string>> tuplified{with(value1, value2)};


    // BUG: Fails to compile!
    // reader<std::string> converted_from_tuple{tuplified.xform(zug::unzip).map(callback)};

    reader<std::string> converted_from_tuple2{tuplified.map(callback_tuple)};

    //CHECK(converted_from_tuple.get() == "7-value");
    CHECK(converted_from_tuple2.get() == "7-value");
}

It is possible to map a with-expression to both, tuple-style and unfolded callback, but it is impossible to map a tuple-containing reader to it.

Full unittest is in this commit: ba8fe64

They are not meant to be symmetric. lager::with(...) does not return a reader, but a combined expression of the two things... it zips automatically at the end of the chain when converting to a reader. A reader of tuples does not auto-unzip. We could add that but it could be surprising. We could have that lager::with() zip automatically at the beginning of the "chain" but that would make many common use-cases more inconvenient. I think this probably will be a won't-fix, unless you have a good argument to change it?

Hi, @arximboldi!

The problem is not that lager::with(...) is too permissive, but that lager::reader<tuple<...>> is too restrictive and (seems to be) non-unpackable. I haven't managed to find any way to unpack it, it packs itself back on any my attempt, even when requesting it explicitly like that:

reader<std::string> converted_from_tuple{tuplified.xform(zug::unzip).map(callback)};

In the real code here we use lager::with(...) and lager::reader<tuple<...>> somewhat interchangeably. Basically, when one has a lens that does some "heavy" computations (e.g. this lens), one wants to cache the result somewhere and put it into a reader.

It is also impossible to bind to a lager::with(...) expression (which is understandable), so one needs to put it into a reader first. So, when the same expression is a public "signal" and a source of a subordinate value at the same time, it becomes troublesome to use it.

PS:
Btw, to overcome the unpacking issue on watching/binding I implemented a (hackish-looking?) wrapper:

namespace kismpl {
/**
 * Convert a given functor f accepting multiple arguments into
 * a function that accepts a tuple with the same number of
 * elements
 */
constexpr auto unzip_wrapper = [] (auto f) {
    return
        [=] (auto &&x) {
            return std::apply(f, std::forward<decltype(x)>(x));
        };
};

}

TEST_CASE("tuple unfolding when binding")
{
    auto callback = [] ([[maybe_unused]] int x, [[maybe_unused]] std::string y) {
        std::cout << "unpacked callback" << std::endl;
    };

    auto callback_tuple = [] ([[maybe_unused]] std::tuple<int, std::string> x) {
        std::cout << "packed callback" << std::endl;
    };


    state<int, automatic_tag> value1{7};
    state<std::string, automatic_tag> value2{"value"};

    reader<std::tuple<int, std::string>> converted_value{with(value1, value2)};

    converted_value.bind(kismpl::unzip_wrapper(callback));
    converted_value.bind(callback_tuple);
}

Is it possible to implement the same with the standard transducers? Or, perhaps, we could include this wrapper into lager?

Hmmm, interesting, something like this should work:

auto callback = [](int x, string y) { return y; };
reader<tuple<int, string>> tuplified = ...;
reader<std::string> converted_from_tuple{tuplified.xform(zug::unzip).map(callback)};

In what way does this fail?

Here is the full error message:

https://invent.kde.org/-/snippets/2458

The line at C:/dev/env-4-lager/lager-tree/lager/test/cursor.cpp:259:46 is:

reader<std::string> converted_from_tuple{tuplified.xform(zug::unzip).map(callback)};

(from the branch linked above)

Hmm interesting, that looks indeed like a bug!

Maybe the bug is in zug itself... It would be interesting to try isolate that in an unit test...

There is a unittest in this branch: ba8fe64

Or you mean a unittest for zug?

Hmmm, after some thought, maybe the bug is not in zug... Probably the problem is that .map() works both via transducer composition, but also as-if it had realized the intermediate cursor. No need to try reproduce in zug until we further investigate here.