
Boost : 
Subject: Re: [boost] [yap] Review
From: P F (pfultz2_at_[hidden])
Date: 20180214 02:24:22
> On Feb 12, 2018, at 7:57 PM, Zach Laine via Boost <boost_at_[hidden]> wrote:
>
> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <boost_at_[hidden]>
> wrote:
>
> Thanks for reviewing, Paul.
>
> [snip]
>
>
>> I think this library is very useful. I would love to use this for some
>> projects at work, but the dependency on Hana prevents that(I need to
>> support gcc 5 and MSVC). Looking at the codebase, it could easily be
>> replaced with Boost.Fit(once its included in the release) in the future.
>
>
>
>>
>
> Of course, I dont expect a reimplementation without Hana as a condition for
>> acceptance. However, I think Hana at least can be removed from the
>> "interface". The expression elements could be expressed as a `std::tuple`
>> and the placeholders could be a `std::integral_constant`. This should make
>> it easier to use another backend without breaking API compatibility.
>>
>
> I actually experimented with trying to remove the Hana bits for this exact
> reason. The conclusion I came to was that writing the library was quite a
> bit harder in a couple of places, but that's not too bad. The bigger
> problem is that the lack of algorithms over std::tuple for users of the
> library when they write transforms. Maybe we should talk offline about how
> to do things that are done now with Hana, with Fit instead.
Actually, the main algorithms you use are transform, for_each, and unpack, which is trivial to do with Fit. Doing a `zip` or `filter` is a little more complicated, but grepping the source code, I did not see that. I discuss how thats done here:
http://pfultz2.com/blog/2015/09/12/powerofunpack/ <http://pfultz2.com/blog/2015/09/12/powerofunpack/>
Which we can discuss more offline.
> C++14includingMSVC or even C++11 would be great. It will have to be a
> pretty clear win for users for me to make that change, though, given how
> great Hana is for working with tuples.
But Hana should be able to work std::tuple, just the same. So at least changing to use something else, whether Fit or not, you wonâ€™t break API compatibility.
>
> In either case, I really want Fit for some things in Yap. When can we
> expect it in a release? Nudge! :)
I am hoping to finish the changes this week to get it into next release.
>
> Some other notes:
>>
>>  Couldn't the operator macros be provided as base classes? So instead of
>> writing:
>>
>> template <boost::yap::expr_kind Kind, typename Tuple>
>> struct user_expr
>> {
>> static const boost::yap::expr_kind kind = Kind;
>>
>> Tuple elements;
>>
>> // Member operator overloads for operator!().
>> BOOST_YAP_USER_UNARY_OPERATOR_MEMBER(logical_not, ::user_expr)
>> };
>>
>> The user could write:
>>
>> template <boost::yap::expr_kind Kind, typename Tuple>
>> struct user_expr : logical_not<::user_expr>
>> {
>> static const boost::yap::expr_kind kind = Kind;
>>
>> Tuple elements;
>> };
>>
>
> Visibly using inheritance for code reuse goes against all I hold dear. One
> pragmatic problem with this is that types in the debugger that have been
> assembled from a dozen or so operatorproviding base classes shows up as
> multiple pages of garbage (at least in my preferred debugger, gdb).
I guess its just a matter of deciding which you hate more inheritance or macros.
>
> You could also get rid of the need for `::user_expr`
>
>
> We actually want that. It's often necessary to be able to return an
> expression created from some template different than the template defining
> he operator. I could have doubled the number of functions and macros in
> Yap that take an expression template template parameter, providing an
> overload that does not take this parameter. Of course, the macros would be
> differently named, not overloaded (another small problem). This would make
> it easier to write the commoncase code (in which the expression template
> used for the return type is that same as the expression template defining
> the operation). I deemed that to be a bad tradeoff.
The point is to use `mp_assign` to change the parameters. So instead of writing:
expr_template<::boost::yap::expr_kind::op_name, tuple_type>
You would write:
mp_assign<expr_type, mp_list<::boost::yap::expr_kind::op_name, tuple_type>
Of course, to make that work, you will need to make `boost::yap::expr_kind::op_name` a type(which can be done by simply wrapping it in an integral constant).
>
>
>> by just taking it as a type and using something similiar to `mp_transform`
>> to change the parameters. Or even change `Kind` to be a type of
>> `std::integral_constant<boost::yap::expr_kind, Kind>` so `mp_transform`
>> could be used directly:
>>
>> template <typename Kind, typename Tuple>
>> struct user_expr : logical_not<user_expr>
>> {
>> static const boost::yap::expr_kind kind = Kind{};
>>
>> Tuple elements;
>> };
>>
>
> You certainly could. What does this buy, though?
Being able to use `mp_assign` to change the parameters to `user_expr`. Although, you could write your own version which allows the first parameter to be an integral.
> I'm not getting that
> part yet. One thing this give up is the inability to write functions in
> terms of chained
>
> constexpr if (kind == expr_kind::foo)
>
> statements, which I quite like. You can do this with is_same<> of course,
> but at the cost of 45 extra template instantiations and a lot more noise.
That should still work. I dont see why it wouldnâ€™t. In fact, you should be able to do all of these:
template<boost::yap::expr_kind Kind>
using expr_kind_c = std::integral_constant<boost::yap::expr_kind, Kind>;
constexpr if (kind == expr_kind_c<foo>{})
constexpr if (Kind{} == expr_kind_c<foo>{})
constexpr if (kind{} == expr_kind::foo)
What may not work is doing something like `fit::if_(Kind{} == expr_kind_c<foo>{})`(due to lack of operator overloading for integral_constant), but using `fit::if_c<Kind{} == expr_kind_c<foo>{}>` still does work.
>
>>  The `make_expr_from_tuple` does a move here:
>>
>> auto make_expr_from_tuple (ExprTemplate<Kind, OldTuple> const & expr,
>> NewTuple && tuple)
>> { return ExprTemplate<Kind, NewTuple>{std::move(tuple)}; }
>>
>> But maybe this should be using `std::forward`?
>>
>
> No, its in detail::, and its only used in this one line:
>
> return make_expr_from_tuple(expr, std::move(transformed_tuple));
>
> So a move is always what we want.
I see, I just the forwarding reference, but I see that it will always be an rvalue anyways.
>
> Zach
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk