Re: [boost] [Hana] Formal review for Hana

Subject: Re: [boost] [Hana] Formal review for Hana
From: Louis Dionne (ldionne.2_at_[hidden])
Date: 2015-06-16 12:11:11

Zach Laine <whatwasthataddress <at> gmail.com> writes:

>
> On Tue, Jun 16, 2015 at 9:35 AM, Louis Dionne <ldionne.2 <at> gmail.com> wrote:
>
> > Zach Laine <whatwasthataddress <at> gmail.com> writes:
> >
> > [...]
> >
> > which is closer to your initial implementation. Now, it is obvious that
> >
> >     auto ncolumns = hana::size(head_row);
> >
> > should work, right?
>
> I also understand why this works.
>
> I do *not* understand why this did not work for me, since it seems to be
> exactly what the code in your PR does:
>
> static const std::size_t columns = hana::size(std::declval<HeadRow>());

You are using std::declval in an evaluated context, which is illegal.
Remember that std::declval is declared (but never defined) as

    template <class _Tp>
    typename add_rvalue_reference<_Tp>::type declval() noexcept;
    // no definition

I'm using std::declval inside decltype(...), which is an unevaluated context.

> [...]
>
> Right.  I should have been more clear.  The use of type<...> was a
> suggestion for a workaround interface for non-literal types that I was
> suggesting.  I shouldn't have conflated the two threads of discussion.  I
> made that suggestion because I tried both of these:
>
> // Does not work; not a literal type
> static const std::size_t columns = hana::size(HeadRow{});
>
> // Does not work for as-yet-mysterious-to-me reasons
> static const std::size_t columns = hana::size(std::declval<HeadRow>());
>
> ... and was looking for a way to get a "fake" object of type HeadRow to
> pass to hana::size().

I understand. Like I said, from my point of view, the proper workaround is to
use a lambda as I suggested.

> > [...]
> >
> >
> > No problem. Now I understand better. So basically you want to write
> >
> >     tuple_1 foo;
> >     auto bar = boost::hana::filter(foo, my_predicate);
> >
> > I don't understand why that's O(N^2) copies. That should really be N
> > copies,
> > where `N = hana::size(bar)`. As a bonus, if you don't need `foo` around
> > anymore, you can just write
> >
> >     tuple_1 foo;
> >     auto bar = boost::hana::filter(std::move(foo), my_predicate);
> >
> > and now you get N moves, not even N copies.
> >
>
> That's good to know.  I was concerned that the pure functional
> implementation would internally produce intermediate values of size 1, 2,
> 3, ... N.  This is often the case in pure functional implementations.  Even
> so, it returns a temporary that must then be copied/moved again into bar.
> That means I'm doing 2*N copies/moves instead of N. That implies that I
> still cannot use filter() in hot code.

The current implementation of filter for Tuple will be as good as I described.
The generic implementation for other sequence types (say an adapted std::tuple)
will be slower. So there's room for improvement, of course.

> (I know that above bar is
> initialized with the result of filter(), but in many cases, the result will
> be assigned to an existing value, and the final copy is not guaranteed to
> be elided.  In much of my code, I need that guarantee, or a way to fall
> back to direct assignment where the elision does not occur.)

The result of `filter` is an rvalue temporary tuple. If the input sequence to
filter was a movable-from tuple, it turns out that this rvalue result will
have been move-constructed. The rest is up to the thing that receives the
result of filter(). If you assign the result of filter() to something that
has a move-assignment operator, then no copy occurs. I might be
misunderstanding your requirement.

> > [...]
> >
> > First, assignment to tuples will be fixed and I consider it a bug right
> > now.
> > However,
> >
> > [...]
> >
> > Does that solve your problem, or am I misunderstanding it?
>
> That all works fine, but I actually need assignment across tuple types that
> are different, but have compatible elements:
>
> hana::_tuple<A, B> x = ...;
> hana::_tuple<C, D> y = ...;
>
> // some stuff happens ...
>
> // This should compile iff std::is_same<A, C>::value && std::is_same<B,
> D>::value
> x = y;
>
> // But this should work as long as a C is assignable to an A and a D is
> assignable to a B:
> hana::copy(x, y);

I guess I will need to decide upon this when I resolve the issue about tuple
assignment. It is not yet clear to me why `x = y` should not work when the
tuple types are different but have compatible elements. I must think about it.

Regards,
Louis