Subject: Re: [boost] [Hana] Formal review for Hana
From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2015-06-16 14:42:35
On Tue, Jun 16, 2015 at 11:11 AM, Louis Dionne <ldionne.2_at_[hidden]> wrote:
> 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.
Gah! Thanks.
> > > [...]
> > >
> > >
> > > 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.
>
When you say "slower" do you mean 2*N or N^2?
> > (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.
>
Sometimes extraneous moves are not ok either. I really, really, need to
use mutating operations and side effects at least some of the time.
> > > [...]
> > >
> > > 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.
Well, sometimes C is only explicitly convertible to A. I perhaps
overstated things above. What I should have said is, in the general case,
"x = y" is not defined for some values, if the assignment relies on
implicit conversion. Moreover, I still want to do other mutating
operations from one tuple to another, aside from just assignment.
Zach