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

Subject: Re: [boost] [yap] Review part 1: documentation
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2018-02-11 22:54:38

AMDG

On 02/11/2018 01:26 PM, Zach Laine via Boost wrote:
> On Sat, Feb 10, 2018 at 6:01 PM, Steven Watanabe via Boost <
> boost_at_[hidden]> wrote:
> 
> [snip]
> 
> All good notes; done.  If you want to track the changes, I've created a
> boost_review branch where all the during-review changes will be done.
> 
> tutorial/expressions.html:
>>
>> - "...any of the functions in Boost.YAP that takes"
>>   s/takes/take/
>>
> 
> That's not wrong; "takes" needs to agree with "any", not "functions".
> 

  I think "any" is also plural when used as
an indefinite pronoun.

> <snip>
>> tutorial/operator_macros.html:
>>
>> - What about BOOST_YAP_USER_ANY_UDT_BINARY_OPERATOR?
>>
> 
> Do you mean "Why doesn't it exist?"  If so, I don't know what it would do.
> BOOST_YAP_USER_UDT_ANY_BINARY_OPERATOR already accepts a UDT on either the
> right or left side, and any non-Expression type on the other.
> 

  Oh, I see.  I got confused by the fact that
the table says "--" for the second operand type
instead of saying the same as for the first
operand type, so I jumped to the conclusion that
the macro is asymmetric.

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

  I don't believe that that is worse than being
unable to write a tag-transform that *doesn't*
apply the transform.  Nothing else in a transform
implicitly recurses, so I think your choice here
is inconsistent.  In addition, it's not quite
true that you can't apply it explicitly.  I can
think of at least three ways to do so:
- re-wrap the argument in a terminal
- call (*this)(terminal_tag(), d)
- implement terminal evaluation in a separate
  function which can be used as needed.

>  You are just passed a value (like "double
> x") above.  Perhaps I should add something like what I just wrote here to
> the docs as well.
> 
> 
>>   This implies that the correct behavior is not to apply
>>   the terminal_tag transform before accessing negate_tag.
>>   The later statement:
>>   // Only applies the negate_tag transform; prints -9.
>>   makes no sense as the negate_tag transform alone will give 9.
>>
> 
> Thanks.  Done.
> 
> 
>> - "The TagTransform is preferred, except in the case of a
>>   call expression, in which case the ExpressionTransform
>>   is preferred"
>>   Why is call treated specially?  From an outsider's
>>   POV it just seems weird.  At the very least it deserves
>>   an explanation in the rationale.
>>
> 
> Frankly, because I'm not smart enough to fix this.  I tried *a lot* to do
> so.  When and if Paul ever puts Fit into a Boost release, I'm going to use
> that as the fix.
> 

  Okay, so it's an implementation problem rather than
a deliberate interface choice made for some inscrutable
reason.  I can live with that.

>> examples/hello_world_redux.html:
>>
>> - "That's better!"
>>   What's better than what?
>>
> 
> This example is better that the previous example.  The comment is
> tongue-in-cheek, though, as the rest indicates.
> 

  Ah.  Having "That" appear a full paragraph before
its antecedent makes it unintelligible.

> examples/calc1.html:
>>
>> - "Here we can first see how much C++14-and-later language
>>   features help the end user -- the Proto version is much,
>>   much longer."
>>   I disagree with this assertion.  The real reason that this
>>   is shorter than the Proto version is that you've baked
>>   placeholder substitution into evaluate, which I think
>>   is a mistake--are you building a lambda library or are
>>   you building a generic ET library?
>>
> 
> evaluate() hardly qualifies as a lambda library!  However, the point
> remains the same -- making yap::evaluate() is trivial with variadic
> templates, and would be necessarily limited in a C++98 library.  Making
> such a trivial function part of the Yap library instead of asking the user
> to reinvent that same wheel in every Yap terminal use case seems like a
> feature to me, not a bug.
> 

  Positional placeholders are really only
needed if you're doing lambda-like things.
I don't think that the primary library interface
should directly support domain-specific uses.
I also don't think requiring those who actually
need this to implement it themselves is an
excessive burden, as a transform that does
it should be <10 lines of code.  If you
really feel that it's necessary to provide this,
just write the transform yourself and provide
it along with the placeholders.

> 
>> examples/calc2.html:
>>
>> - This really misses the point of proto's calc2 example.
>>   An equivalent example with YAP would be to define a
>>   new ExpressionTemplate with an overloaded call operator.
>>
> 
> I don't think this example missed the point.

  The point of the example in Proto is how to
add members to an expression type, not just how
to make it callable.  Wrapping it in another
class is something quite different.  Now,
admittedly, Yap makes creating your own
ExpressionTemplate types so easy, that this
example is basically redundant...

>  In my calc2a example, I show
> the same functionality, just using lambdas to give the expressions names.

You can give the expressions names in Proto too:

BOOST_PROTO_AUTO(expr_1_fn, _1 + 2.0);

> I like this better for stylistic reasons.  The calc2b version is more
> directly like the Proto version, except that here again it is possible to
> provide a single *very simple* Yap function that replaces all the
> functionality in the Proto calc2 example, yap::make_expression_function().
> This also seems to me like a good thing.
> 
> 
>> examples/calc3.html:
>>
>> - The way you're using get_arity is rather wacky as
>>   you know the arity statically anyway.  The original
>>   Proto example makes more sense.
>>
> 
> Please indicate which of the statements in Proto calc3's main() does *not*
> contain a statically known arity.  Again, I think the only transformation
> that I made which lead to your comment was to give the expressions names by
> lambda-wrapping them.  I prefer this style.
> 

  The arity is known in main, but is *not* known at
the point where get_arity is actually called.  i.e.
in the various overloads of calculator_expression::operator()

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

>  But in the case of a
> vector<unique_ptr> case, yes I would probably have done something different.
> >
>> examples/mixed.html:
>>
>> - I think you'd be better off making sin_tag a regular
>>   function object like in the proto example.
>>
> 
> Could you explain?  It isn't immediately obvious to me what you mean.
> 

struct sin_tag
{
    template<class T>
    T operator()(const T& t) const { using std::sin; return sin(); }
};

Then there's no need to use a yap-specific
customization point (eval_call).

In Christ,
Steven Watanabe