Re: [boost] [yap] Review part 1: documentation

Subject: Re: [boost] [yap] Review part 1: documentation
From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2018-02-13 01:09:13

On Mon, Feb 12, 2018 at 12:08 PM, Steven Watanabe via Boost <
boost_at_[hidden]> wrote:

> AMDG
>
> On 02/12/2018 09:45 AM, Zach Laine wrote:
> > On Sun, Feb 11, 2018 at 5:54 PM, Steven Watanabe via Boost <
> > boost_at_[hidden]> wrote:
> >
> >> <snip>
> >>>> tutorial/transform_matching.html:
> >>>>
> >>>> - The negate example:
> >>>> struct xform
> >>>> {
> >>>>     // This transform negates terminals.
> >>>>     auto operator() (boost::yap::terminal_tag, double x)
> >>>>     { return -x; }
> >>>>
> >>>>     // This transform removes negations.
> >>>>     auto operator() (boost::yap::negate_tag, double x)
> >>>>     { return x; }
> >>>> }
> >>>>   seems somewhat confused.  The behavior I would expect
> >>>>   from this transform is that it can match expressions
> >>>>   of the form `d` or `-d` and will return -d or d respectively.
> >>>>
> >>>
> >>> This gets right at the point of the example.  It's maybe a
> >> counterintuitive
> >>> way to evaluate expressions, but the alternative is worse.  The
> >> alternative
> >>> is that terminal transforms are not auto-applied before a terminal is
> >>> unwrapped and passed as a value to a tag-transform call.  If that were
> >> how
> >>> the library worked, there would be no way to write a tag-transform
> >> function
> >>> that *did* apply the transform, even explicitly, because at that point
> >> the
> >>> terminal is no longer visible.
> >>
> >>   <snip>
> >> - re-wrap the argument in a terminal
> >> - call (*this)(terminal_tag(), d)
> >>
> >
> > In either of those cases, any information you had about the details of
> the
> > terminal are now gone, and you can't get them back.  The value that you
> > re-wrap or call as above may have come from a terminal that was an
> lvalue,
> > an rvalue, a reference-expression, const, mutable, etc.
> >
> > - implement terminal evaluation in a separate
> >>   function which can be used as needed.
> >>
> >
> > True.  I had to make a decision about which convention violated my sense
> of
> > surprise the least.  This is the one I chose.  It seems less surprising
> to
> > get stuck with transforms you did not yet get to in the terminals (mainly
> > because they happen only since you wrote those into your transform
> > explicitly), than it does to get stuck because you lost important
> > information about the properties of the unwrapped terminal.
> >
> > It may be that the community consensus is that my sense of surprise is
> > wrong in this case; I'm open to being persuaded.
> >
> > [snip]
> >
>
>   Maybe you should reconsider whether terminals should
> be unwrapped in the first place.  If you don't unwrap
> terminals, then this isn't an issue.
>
> If I'm writing a generic function like:
>
> auto operator()(plus_tag, Expr&& lhs, Expr&& rhs)
> {
>   transform(lhs, *this);
>   transform(rhs, *this)
> }
>
> then the behavior that I want is for terminals
> to be processed by the nested call to transform.
> Unwrapping the terminals causes surprising behavior,
> which is only sort of fixed by automatically applying
> the terminal transform and making transform a no-op
> for non-Expressions.  In particular, if the terminal
> transform returns an Expression, that Expression will
> get processed again.

Yes, I'm not entirely satisfied with this either, for the same reasons.
FWIW, the design has settled here after previously not doing any automatic
terminal unwrapping, but I found it substantially harder to use when that
was the case.  I find that code like what you wrote here infrequently trips
over whether Expr is an Expression or a non-Expression type, but when it
does you can write it like this:

auto operator()(plus_tag, Expr&& lhs, Expr&& rhs)
{
  transform(as_expr(lhs), *this);
  transform(as_expr(rhs), *this)
}

Not the greatest, but like I said, I don't think the full generality here
is required all that often.

>
> >>> examples/vector.html:
> >>>>
> >>>> - return boost::yap::make_terminal(std::move(vec[n]));
> >>>>   Move is probably wrong as you're working with a reference
> >>>>   to begin with.  (Of course, it doesn't really matter for
> >>>>   double, but imagine that you have a vector<unique_ptr>
> >>>>   instead.)
> >>>>
> >>>
> >>> The move is required to force a copy; just because the reference is
> valid
> >>> now doesn't mean it won't dangle later.
> >>
> >>   Sure, but when using the evaluate(transform()) idiom,
> >> you're guaranteed that the references returned by transform
> >> will not be left dangling.
> >
> >
> > Sure.  But that's just in this example, and using that (admittedly
> > dominant) idiom.  I still want to reinforce in the example code how you
> > make copies of value types, especially built-in ones.
> >
>
>   Perfect-forwarding through a chain of
> transforms is also an important use case that
> needs to be explained.  I don't mind having
> the examples demonstrate copying as long as
> they are written such in a way that copying
> is really the correct behavior.  In this particular
> example, I don't think that it is.

That's a really good point.  I'll revisit that example. *TODO*.

Zach