Re: [boost] [yap] Review

Subject: Re: [boost] [yap] Review
From: P F (pfultz2_at_[hidden])
Date: 2018-02-14 20:44:21

> On Feb 14, 2018, at 1:38 PM, Zach Laine <whatwasthataddress_at_[hidden]> wrote:
> 
> On Tue, Feb 13, 2018 at 8:24 PM, P F <pfultz2_at_[hidden] <mailto:pfultz2_at_[hidden]>> wrote:
> 
>> On Feb 12, 2018, at 7:57 PM, Zach Laine via Boost <boost_at_[hidden] <mailto:boost_at_[hidden]>> wrote:
>> 
>> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <boost_at_[hidden] <mailto:boost_at_[hidden]>>
>> wrote:
>> 
>> 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/power-of-unpack/ <http://pfultz2.com/blog/2015/09/12/power-of-unpack/>
> 
> Which we can discuss more offline.
> 
>  I also use Hana constants all over the place.

Yes, but it should be simple to move from `0_c` to `c<0>{}`. Ultimately, it really should be moved to a separate library, because even Boost.TypeTraits could use these “enhanced” integral constants(but that is another discussion).

>>> 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.
> 
> Ok, but all of the above is still the "what".  I want the "why". Why is it better to be able to use mp_assign?
>>  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.
> 
> Right, but all of that is noisier than what I currently have. 

Not really because `constexpr if (kind == expr_kind::foo)` will still work.

> If there are advantages to using mp_assign, maybe this extra noise in a few places is worth it.  I still have no idea what the mp_assign advantages are though.

The point is to be able to get rid of `::` at the beginning in `BOOST_YAP_USER_UNARY_OPERATOR_MEMBER(logical_not, ::user_expr)`. If the user forgets those, you need to have a clear way to tell the user they need to add them, because it may not be obvious from the compiler error without understanding the implementation details. 

One way to solve this is to avoid it altogether by not passing a template, which you can try to utilize something like mp_assign. 

Another possible solution may be to add a static_assert with a message like “you are passing a type instead of template, you may need to prepend :: to the parameter to this macro". I think you can use function overloading to distinguish between a type and a template, but I am not sure. 

Paul