[yap] Review

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[yap] Review

Boost - Dev mailing list
- Whether you believe the library should be accepted into Boost

I think it should be Accepted.

- Your name

Paul Fultz II

- What is your evaluation of the library's:

The design is very nice and simple to follow. I have found Proto to be a little hard for me to grep, but this seems fairly simple to follow.

I think the transform -> evalute workflow is great. Also, with transforms being just function objects, it makes transformations very composable.

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.

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;
};

You could also get rid of the need for `::user_expr` 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;
};

- The customization points seem unnecessary. Why wouldn't the user just use `transform`? Using `transform` also keeps the customizations local. Also, implicit transformations seems like it would lead to suprises(and not good suprises). Are those on the cutting block?

- It would be interesting to provide common transformations, like common subexpression elimination, or dead code elimination, but I guess that is beyond the scope of this library.

- The `op_string` function is missing the case for `expr_ref` and `terminal`. Also, the tests only check for plus.

- 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`?

- Did you attempt to use the library? If so:

Yes, I ran it on clang 3.8 and gcc 7.

- How much effort did you put into your evaluation of the review?

Several hours


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email]>
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.
C++14-including-MSVC 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.

In either case, I really want Fit for some things in Yap.  When can we
expect it in a release?  Nudge! :)

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 operator-providing base classes shows up as
multiple pages of garbage (at least in my preferred debugger, gdb).

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 common-case 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 trade-off.


> 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?  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.


> - The customization points seem unnecessary. Why wouldn't the user just
> use `transform`? Using `transform` also keeps the customizations local.
> Also, implicit transformations seems like it would lead to suprises(and not
> good suprises). Are those on the cutting block?
>

Absolutely.  As I said in Steven's review, they've only made it this long
to make sure people were not more interested in them than I am (which is
not at all).


> - It would be interesting to provide common transformations, like common
> subexpression elimination, or dead code elimination, but I guess that is
> beyond the scope of this library.
>

I think it is.


> - The `op_string` function is missing the case for `expr_ref` and
> `terminal`. Also, the tests only check for plus.
>

That function only applies to operators, not those special expr_kinds, so
those were left out.  I added them for completeness, and adjusted the docs
for that function accordingly.


> - 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.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
AMDG

On 02/12/2018 06:57 PM, Zach Laine via Boost wrote:

> <snip>
>> 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).

You don't need a separate macro:
#define BOOST_YAP_USER_UNARY_OPERATOR_MEMBER(op_name, ...) \
  /* implementation */

>  This would make
> it easier to write the common-case 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 trade-off.
>
In Christ,
Steven Watanabe

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
AMDG

On 02/12/2018 06:57 PM, Zach Laine via Boost wrote:

> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email]>
> wrote:
>
>> 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.
>

  If that's really important then why doesn't
BOOST_YAP_USER_FREE_BINARY_OPERATOR take two
expr_template parameters?

In Christ,
Steven Watanabe

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Tue, Feb 13, 2018 at 11:28 AM, Steven Watanabe via Boost <
[hidden email]> wrote:

> AMDG
>
> On 02/12/2018 06:57 PM, Zach Laine via Boost wrote:
> > <snip>
> >> 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).
>
> You don't need a separate macro:
> #define BOOST_YAP_USER_UNARY_OPERATOR_MEMBER(op_name, ...) \
>   /* implementation */


Ah, that's right.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Tue, Feb 13, 2018 at 11:30 AM, Steven Watanabe via Boost <
[hidden email]> wrote:

> AMDG
>
> On 02/12/2018 06:57 PM, Zach Laine via Boost wrote:
> > On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email]>
> > wrote:
> >
> >> 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.
> >
>
>   If that's really important then why doesn't
> BOOST_YAP_USER_FREE_BINARY_OPERATOR take two
> expr_template parameters?


Because there's only one result.   The expr_template parameter governs how
the result is constructed, not which inputs are accepted.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
AMDG

On 02/13/2018 09:47 AM, Zach Laine via Boost wrote:

> On Tue, Feb 13, 2018 at 11:30 AM, Steven Watanabe via Boost <
> [hidden email]> wrote:
>
>> On 02/12/2018 06:57 PM, Zach Laine via Boost wrote:
>>> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email]>
>>> wrote:
>>>
>>>> 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.
>>>
>>
>>   If that's really important then why doesn't
>> BOOST_YAP_USER_FREE_BINARY_OPERATOR take two
>> expr_template parameters?
>
>
> Because there's only one result.   The expr_template parameter governs how
> the result is constructed, not which inputs are accepted.
>

  Doesn't this make BOOST_YAP_USER_FREE_BINARY_OPERATOR
unusable, since expression already uses it, so using it
again with a different ExpressionTemplate would be ambiguous?

In Christ,
Steven Watanabe

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
On Tue, Feb 13, 2018 at 1:51 PM, Steven Watanabe via Boost <
[hidden email]> wrote:

> AMDG
>
> On 02/13/2018 09:47 AM, Zach Laine via Boost wrote:
> > On Tue, Feb 13, 2018 at 11:30 AM, Steven Watanabe via Boost <
> > [hidden email]> wrote:
> >
> >> On 02/12/2018 06:57 PM, Zach Laine via Boost wrote:
> >>> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email]>
> >>> wrote:
> >>>
> >>>> 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.
> >>>
> >>
> >>   If that's really important then why doesn't
> >> BOOST_YAP_USER_FREE_BINARY_OPERATOR take two
> >> expr_template parameters?
> >
> >
> > Because there's only one result.   The expr_template parameter governs
> how
> > the result is constructed, not which inputs are accepted.
> >
>
>   Doesn't this make BOOST_YAP_USER_FREE_BINARY_OPERATOR
> unusable, since expression already uses it, so using it
> again with a different ExpressionTemplate would be ambiguous?
>
>
Sure, if you use both of them.  I expect people only to use expression for
quick-and-dirty small-scale uses of Yap, or for experimentation though.

However, the same problem exists if I have ET1 and ET2 that want to use
BOOST_YAP_USER_FREE_BINARY_OPERATOR.  In that case, the same ambiguity
exists.  The solution is the same as any operator overload ambiguity, I
think: sfinae.  If there's not already an appropriate constraining macro to
replace BOOST_YAP_USER_FREE_BINARY_OPERATOR (I'd have to look around a
bit), I should add one. *TODO*

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list

> On Feb 12, 2018, at 7:57 PM, Zach Laine via Boost <[hidden email]> wrote:
>
> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email]>
> 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/power-of-unpack/ <http://pfultz2.com/blog/2015/09/12/power-of-unpack/>

Which we can discuss more offline.

> C++14-including-MSVC 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 operator-providing 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 common-case 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 trade-off.

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list

> On Feb 14, 2018, at 1:38 PM, Zach Laine <[hidden email]> wrote:
>
> On Tue, Feb 13, 2018 at 8:24 PM, P F <[hidden email] <mailto:[hidden email]>> wrote:
>
>> On Feb 12, 2018, at 7:57 PM, Zach Laine via Boost <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> On Mon, Feb 12, 2018 at 4:15 PM, P F via Boost <[hidden email] <mailto:[hidden email]>>
>> 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




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
On Wed, Feb 14, 2018 at 2:44 PM, P F <[hidden email]> wrote:

>
> 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.
>

Ah, I get it now, thanks.

I think I'd rather document better that the :: is required, as opposed to
doing more template chicanery.  This is both because I want to keep compile
times low, and because I don't want to make something that might be brittle
in ways I don't fully understand.  I'm a metaprogrammer of only moderate
skill.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
On February 14, 2018 4:04:12 PM EST, Zach Laine via Boost <[hidden email]> wrote:

> On Wed, Feb 14, 2018 at 2:44 PM, P F <[hidden email]> wrote:
>
> >
> > 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.
> >
>
> Ah, I get it now, thanks.
>
> I think I'd rather document better that the :: is required, as opposed
> to
> doing more template chicanery.  This is both because I want to keep
> compile
> times low, and because I don't want to make something that might be
> brittle
> in ways I don't fully understand.  I'm a metaprogrammer of only
> moderate skill.

You could prepend :: in the macro.

--
Rob

(Sent from my portable computation device.)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
AMDG

On 02/14/2018 02:48 PM, Rob Stewart via Boost wrote:

> On February 14, 2018 4:04:12 PM EST, Zach Laine via Boost <[hidden email]> wrote:
>> On Wed, Feb 14, 2018 at 2:44 PM, P F <[hidden email]> wrote:
>> <snip>
>> I think I'd rather document better that the :: is required, as opposed
>> to
>> doing more template chicanery.  This is both because I want to keep
>> compile
>> times low, and because I don't want to make something that might be
>> brittle
>> in ways I don't fully understand.  I'm a metaprogrammer of only
>> moderate skill.
>
> You could prepend :: in the macro.
>

:: is only correct for templates in the global namespace.

In Christ,
Steven Watanabe

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost