Yap's formal review is starting now!

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
107 messages Options
1234 ... 6
Reply | Threaded
Open this post in threaded view
|

Yap's formal review is starting now!

Boost - Dev mailing list
Dear Boost community,

The formal review of Zach Laine's Yap library starts Monday, February 5th
and
ends on Wednesday, February 14th.

Yap is a C++14-and-later library to build expression templates. Yap uses
new features introduced in C++14/17 to make the library simpler to use and
compile faster than existing solutions, such as the well known Boost.Proto
library. The documentation is available here:

  https://tzlaine.github.io/yap/doc/html/index.html

The GitHub repository is available here:

  https://github.com/tzlaine/yap

We encourage your participation in this review. At a minimum, please state:

- Whether you believe the library should be accepted into Boost
- Your name
- Your knowledge of the problem domain

You are strongly encouraged to also provide additional information:

- What is your evaluation of the library's:
  * Design
  * Implementation
  * Documentation
  * Tests
  * Usefulness
- Did you attempt to use the library? If so:
  * Which compiler(s)?
  * What was the experience? Any problems?
- How much effort did you put into your evaluation of the review?

All issues discussed during this review will be tracked on the library's
GitHub issue tracker at https://github.com/tzlaine/yap/issues.

Regards,
Louis Dionne

P.S.: I've been having trouble posting to this list. If this ends up being a
duplicate post, let's make this one the official one.

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

Re: Yap's formal review is starting now!

Boost - Dev mailing list

> On Feb 5, 2018, at 8:53 AM, Louis Dionne via Boost <[hidden email]> wrote:
>
> The formal review of Zach Laine's Yap library starts Monday, February 5th
> and
> ends on Wednesday, February 14th.

I have not heard any discussion of Yap yet, so I’ll jump in and start it off.

My review will come later, but for the moment I want to raise an issue for discussion.

First, a bit of context.  I have been using Yap in production for the last year (thanks Zach for your help along the way).  In my application, I am dealing with arbitrary, user-coded expression trees.  Some of the terminals can be function objects that in turn evaluate other user-coded expression trees.  Ideally, evaluation of these expressions would work for any types wrapped in the appropriate expression classes.  Indeed, this is the case, which is great.

However, I also have a common use case that requires changing how the evaluation works depending on context.  For example, it would be useful to write something analogous to evaluate(expr,context) with a stateful context object that would influence how certain terminals are evaluated.  

The official stance on this [1] is that one should instead use transform(expr,xform), where xform could play the role of context above because transforms can “do anything”, including evaluate the expression using contextual information contained within the xform object.

The problem I see with using transform() is that the entire recursion through the expression tree that is provided by the implementation of evaluate() must be duplicated in some way by the user in order to implement the necessary overloads in xform.  This is because “if xform(N) is a well-formed C++ expression, that xform(N) is used, and no nodes under N in expr are visited” [2].  Therefore, the user has to provide whatever recursion is needed to mimic what evaluate() would otherwise do.

This situation raises several questions.  It is my hope that this post will raise a discussion about the appropriate design considerations at work.

- Is context-dependent evaluation a use-case that is valuable to support by the library?  Based on my experience, the answer is clearly yes, but perhaps others wish to weigh in.

- Is it appropriate for a library to require users to reimplement something as intricate as this recursion in order to support a use case like that?

- Is it appropriate for Yap to have something like evaluate_with_context(eval,context,…) that would implement a recursion similar to evaluate() but allow a context argument to control the evaluation at appropriate customization points.  Note, the variadic part is for placeholders, which are supported by evaluate(), and not really part of the issue.  Again, from my experience it seems that implementing this once correctly in the library would save much pain on the users’ part.

I hope this will stimulate some discussion and look forward to seeing where it goes.

Cheers,
Brook

[1] Often when you create an expression, you will want to evaluate it in different contexts, changing its evaluation -- or even entire meaning -- in each context. evaluate() Is wrong for this task, since it only takes values for substitution into placeholders. In these situations, you should instead use an explicit transform that evaluates your expression. https://tzlaine.github.io/yap/doc/html/boost_yap__proposed_/manual/tutorial/transforming_expressions/explicit_transformations.html

[2] https://tzlaine.github.io/yap/doc/html/boost_yap__proposed_/manual/tutorial/transforming_expressions/explicit_transformations.html


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

Re: Yap's formal review is starting now!

Boost - Dev mailing list
AMDG

On 02/07/2018 01:50 PM, Brook Milligan via Boost wrote:
>
> <snip>
> - Is it appropriate for a library to require users to reimplement something as intricate as this recursion in order to support a use case like that?
>
> - Is it appropriate for Yap to have something like evaluate_with_context(eval,context,…) that would implement a recursion similar to evaluate() but allow a context argument to control the evaluation at appropriate customization points.  Note, the variadic part is for placeholders, which are supported by evaluate(), and not really part of the issue.  Again, from my experience it seems that implementing this once correctly in the library would save much pain on the users’ part.
>

  I haven't actually started looking at YAP yet, but I
think the correct way to handle this for YAP to expose
a transform that does the same as evaluate, which
can be selectively overridden by a derived transform.

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's formal review is starting now!

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 2/5/2018 9:53 AM, Louis Dionne via Boost wrote:

> Dear Boost community,
>
> The formal review of Zach Laine's Yap library starts Monday, February 5th
> and
> ends on Wednesday, February 14th.
>
> Yap is a C++14-and-later library to build expression templates. Yap uses
> new features introduced in C++14/17 to make the library simpler to use and
> compile faster than existing solutions, such as the well known Boost.Proto
> library. The documentation is available here:
>
>    https://tzlaine.github.io/yap/doc/html/index.html

Hello Everyone,

My question is on the compiler support instead of a review.  On
https://tzlaine.github.io/yap/doc/html/boost_yap__proposed_/compiler_support.html,
it states that MSVC does not support YAP.  With the ongoing improvements
to support C++17 in the Visual Studio 2017 branch, have the weekly to
bi-weekly updates positively affected using cl.exe 19.12.25834 and newer
(e.g. Visual Studio 2017, 15.5.4)?

--Robert

>
> The GitHub repository is available here:
>
>    https://github.com/tzlaine/yap
>
> We encourage your participation in this review. At a minimum, please state:
>
> - Whether you believe the library should be accepted into Boost
> - Your name
> - Your knowledge of the problem domain
>
> You are strongly encouraged to also provide additional information:
>
> - What is your evaluation of the library's:
>    * Design
>    * Implementation
>    * Documentation
>    * Tests
>    * Usefulness
> - Did you attempt to use the library? If so:
>    * Which compiler(s)?
>    * What was the experience? Any problems?
> - How much effort did you put into your evaluation of the review?
>
> All issues discussed during this review will be tracked on the library's
> GitHub issue tracker at https://github.com/tzlaine/yap/issues.
>
> Regards,
> Louis Dionne
>
> P.S.: I've been having trouble posting to this list. If this ends up being a
> duplicate post, let's make this one the official one.
>
> _______________________________________________
> 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's formal review is starting now!

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Wed, Feb 7, 2018 at 3:50 PM, Brook Milligan via Boost <
[hidden email]> wrote:

>
> > On Feb 5, 2018, at 8:53 AM, Louis Dionne via Boost <
> [hidden email]> wrote:
> >
> > The formal review of Zach Laine's Yap library starts Monday, February 5th
> > and
> > ends on Wednesday, February 14th.
>
> I have not heard any discussion of Yap yet, so I’ll jump in and start it
> off.
>
> My review will come later, but for the moment I want to raise an issue for
> discussion.
>
> First, a bit of context.  I have been using Yap in production for the last
> year (thanks Zach for your help along the way).  In my application, I am
> dealing with arbitrary, user-coded expression trees.  Some of the terminals
> can be function objects that in turn evaluate other user-coded expression
> trees.  Ideally, evaluation of these expressions would work for any types
> wrapped in the appropriate expression classes.  Indeed, this is the case,
> which is great.
>
> However, I also have a common use case that requires changing how the
> evaluation works depending on context.  For example, it would be useful to
> write something analogous to evaluate(expr,context) with a stateful context
> object that would influence how certain terminals are evaluated.
>
> The official stance on this [1] is that one should instead use
> transform(expr,xform), where xform could play the role of context above
> because transforms can “do anything”, including evaluate the expression
> using contextual information contained within the xform object.
>
> The problem I see with using transform() is that the entire recursion
> through the expression tree that is provided by the implementation of
> evaluate() must be duplicated in some way by the user in order to implement
> the necessary overloads in xform.  This is because “if xform(N) is a
> well-formed C++ expression, that xform(N) is used, and no nodes under N in
> expr are visited” [2].  Therefore, the user has to provide whatever
> recursion is needed to mimic what evaluate() would otherwise do.
>
> This situation raises several questions.  It is my hope that this post
> will raise a discussion about the appropriate design considerations at work.
>
> - Is context-dependent evaluation a use-case that is valuable to support
> by the library?  Based on my experience, the answer is clearly yes, but
> perhaps others wish to weigh in.
>
> - Is it appropriate for a library to require users to reimplement
> something as intricate as this recursion in order to support a use case
> like that?
>
> - Is it appropriate for Yap to have something like
> evaluate_with_context(eval,context,…) that would implement a recursion
> similar to evaluate() but allow a context argument to control the
> evaluation at appropriate customization points.  Note, the variadic part is
> for placeholders, which are supported by evaluate(), and not really part of
> the issue.  Again, from my experience it seems that implementing this once
> correctly in the library would save much pain on the users’ part.
>
> I hope this will stimulate some discussion and look forward to seeing
> where it goes.
>

Ok, some things to note:

evaluate() currently has two modes, speaking roughly.  The first mode is to
evaluate an expression by doing whatever the built-in operators and
existing function calls in the expression would do.  This can be extremely
useful in many situations when you write code using Yap, especially
transforms where you want to at least partially default-evaluate some
subexpression.  The second is to evaluate the expression using custom code
that the user has specified using customization points; there are
customization points for every overloadable operator, among others.

This second mode is essentially a way of doing implicit transforms, and is
really only there for Proto parity.  I've never liked it, and am on the
fence about cutting the customization points entirely.  The implicit nature
of the customization is at the heart of my problem with this feature.  A
good abstraction is used explicitly, but hides its implementation details.
These customization points do the implementation hiding bit just fine, but
you can't even tell you're using them when looking at a particular line of
code -- does yap::evaluate(a + b) yield a sum, or launch a missile?  Who's
to say?  I have to go code spelunking to find out.  This is at odds with
good code practice emphasizing local reasoning.  If I had not wanted Proto
feature parity, I *never* would have implemented a library like this.

So, were I to add a new evaluate_with_context(), it would mean that we
would perpetuate this customization-point mis-feature.  This I do not like.

Did I mention that the customization points are implemented done via ADL
trickery, requiring new types and/or namespaces to get slightly different
behaviors?  I also don't like this aspect.

So, I have been very resistant to adding another new evaluation mode.
Instead, I think something like this should be usable in most cases:

// Let's assume the existence of a my_make_term() function that takes a Yap
terminal
// and a Context &, and returns a new Yap terminal.
template <typename Context>
struct transform_terminals_with_context
{
    // Base case. Note that I'm ignoring placeholders entirely for this
    // example (they're easy to special-case if necessary).
    template <typename T>
    auto operator() (yap::terminal_tag, T && t)
    { return my_make_term(std::forward<T>(t), context_); }

    // Recursive case: Match any binary expression.
    template <typename Tag, typename LExpr, typename RExpr>
    auto operator() (Tag, LExpr const & left, RExpr const & right)
    {
        return yap::make_expression<yap::detail::kind_for<Tag>>(
            boost::yap::transform(left, *this),
            boost::yap::transform(right, *this));
    }

    // Recursive case: Match any unary expression.
    template <typename Tag, typename Expr>
    auto operator() (Tag, Expr const & expr)
    {
        return yap::make_expression<yap::detail::kind_for<Tag>>(
            boost::yap::transform(expr, *this));
    }

    // Ternary and call are added as necessary.

    Context & context_;
};

This transforms all your terminals to new terminals, using your custom code
that applies your terminal + context operation.  You can then just eval()
the transformed expression using the default evaluate(), transform() it
again with a different transform, etc.  I had to make up the constexpr
function kind_for<>() that maps expression-kind tag types to yap::expr_kind
values (which I'll probably add now that I see it is needed *TODO*), but
the reset is just typical Yapery.

Now, this has the downside that if you have a very large number of
terminals, you may have some expensive copies going on, because you are
copying the entire tree.  This implies to me that the most important issue
is whether the evaluate-as-you-transform-because-the-tree-is-huge use case
is of primary or secondary concern.  My expectation is that it is of
secondary concern.  To expect otherwise is probably to optimize prematurely
-- even if this issue is important, how often do users see real perf impact?

I don't know the answer to that yet, but if it is a common enough perf
problem, I'm quite happy to come up with a solution.  If not, making your
own transform that evaluates in place actually sounds like the right answer
to me.  In most cases, users also don't care about every possible
expression kind -- they are designing a mini-language that uses a subset.
Using a transform like the one above, evaluate(), or the proposed
evaluate_with_context() allows subexpressions that are not in the purview
of your mini-language.  A custom transform has better type safety.

Zach

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

Re: Yap's formal review is starting now!

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Wed, Feb 7, 2018 at 6:33 PM, Robert via Boost <[hidden email]>
wrote:

> On 2/5/2018 9:53 AM, Louis Dionne via Boost wrote:
>
>> Dear Boost community,
>>
>> The formal review of Zach Laine's Yap library starts Monday, February 5th
>> and
>> ends on Wednesday, February 14th.
>>
>> Yap is a C++14-and-later library to build expression templates. Yap uses
>> new features introduced in C++14/17 to make the library simpler to use and
>> compile faster than existing solutions, such as the well known Boost.Proto
>> library. The documentation is available here:
>>
>>    https://tzlaine.github.io/yap/doc/html/index.html
>>
>
> Hello Everyone,
>
> My question is on the compiler support instead of a review.  On
> https://tzlaine.github.io/yap/doc/html/boost_yap__proposed_/
> compiler_support.html, it states that MSVC does not support YAP.  With
> the ongoing improvements to support C++17 in the Visual Studio 2017 branch,
> have the weekly to bi-weekly updates positively affected using cl.exe
> 19.12.25834 and newer (e.g. Visual Studio 2017, 15.5.4)?
>

The compiler limitations for Yap are exactly those for Hana.  Yap uses Hana
extensively, and MSVC (last I heard) cannot handle Hana.  I've also heard
from Louis that the MSVC team is making great strides there.  I don't have
terribly up-to-date info I'm afraid.

Zach

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

Re: Yap's formal review is starting now!

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

> On Feb 8, 2018, at 2:15 PM, Zach Laine via Boost <[hidden email]> wrote:
>
> Ok, some things to note:
>
> evaluate() currently has two modes, speaking roughly.  The first mode is to
> evaluate an expression by doing whatever the built-in operators and
> existing function calls in the expression would do.  This can be extremely
> useful in many situations when you write code using Yap, especially
> transforms where you want to at least partially default-evaluate some
> subexpression.  The second is to evaluate the expression using custom code
> that the user has specified using customization points; there are
> customization points for every overloadable operator, among others.
>
> This second mode is essentially a way of doing implicit transforms, and is
> really only there for Proto parity.  I've never liked it, and am on the
> fence about cutting the customization points entirely.  The implicit nature
> of the customization is at the heart of my problem with this feature.  A
> good abstraction is used explicitly, but hides its implementation details.
> These customization points do the implementation hiding bit just fine, but
> you can't even tell you're using them when looking at a particular line of
> code -- does yap::evaluate(a + b) yield a sum, or launch a missile?  Who's
> to say?  I have to go code spelunking to find out.  This is at odds with
> good code practice emphasizing local reasoning.  If I had not wanted Proto
> feature parity, I *never* would have implemented a library like this.

Thank you, Zach.  This is the first explanation that I have seen that clearly lays out your design philosophy, although even this is a bit implicit.  To be more explicit, let me restate what I think you are saying.

You feel that expression templates should essentially provide lazy evaluation of expressions with the same semantics they would otherwise have; the semantics of evaluating an expression should not be changeable in the process of the evaluation.  Changing semantics should only result from an explicit transformation of the expression into some new form corresponding to the new semantics.  Coming from a Proto world, this is quite a different view that should be clarified in the documentation.

If this is the world view, then it seems that implicit transforms should be removed.  Alternatively, they should be retained but with _much_ clearer documentation regarding that use case being for "Proto compatibility" but not really "approved".

For my own ongoing use of Yap, your comments above have been really helpful to clarify what you consider to be best practices.  Having the idiom I described in mind (please clarify if I'm getting this wrong) is very helpful to rethink my code base.

However, this points to a concern I have long had, which is that the documentation does not lay out philosophies, guidelines, best practices, etc.  Even though I have worked with Yap for a year and with Proto before (maybe that poisoned me), I have apparently been thinking about this wrongly.

The examples are fine for what they do, but they are not sufficient to explain _why_ a certain solution is appropriate.  Thus, I strongly urge you to revisit the documentation with an eye toward addressing this gap.

> This transforms all your terminals to new terminals, using your custom code
> that applies your terminal + context operation.  You can then just eval()
> the transformed expression using the default evaluate(), transform() it
> again with a different transform, etc.  I had to make up the constexpr
> function kind_for<>() that maps expression-kind tag types to yap::expr_kind
> values (which I'll probably add now that I see it is needed *TODO*), but
> the reset is just typical Yapery.

Thanks for this example.  It clarifies a lot.  And yes, please support this fully.

> Now, this has the downside that if you have a very large number of
> terminals, you may have some expensive copies going on, because you are
> copying the entire tree.  This implies to me that the most important issue
> is whether the evaluate-as-you-transform-because-the-tree-is-huge use case
> is of primary or secondary concern.  My expectation is that it is of
> secondary concern.  To expect otherwise is probably to optimize prematurely
> -- even if this issue is important, how often do users see real perf impact?

I, too, was concerned about copying, but now that I better understand the "proper" use case, I'll offer this.  At least in my experience, I cannot copy some of the terminals, but I can of course copy the values that the terminals would evaluate to.  It seems that in general, those values should always be cheaply copyable, otherwise the whole idea of evaluation of an expression tree is fraught.

Thus, I feel that using a transform that converts terminals into cheap-to-copy values still embedded within an equivalent expression tree would not incur a major cost.

If I am getting this right, then I agree that this issue is unlikely to be a problem and the following principles make perfect sense:

- the semantics of expression tree evaluation is the same as native evaluation

- expression trees are to encode lazy evaluation without the option of new (or implicit) semantics

- a potentially common use case for complex terminals is to transform them into appropriate values while copying the expression, followed by evaluation of the expression newly populated with values

If I am getting this right, then this type of information is what I feel is missing from the documentation.  Including it would go a long way toward making use of Yap clearer.

> In most cases, users also don't care about every possible
> expression kind -- they are designing a mini-language that uses a subset.

Please do not impose this view on the design of the library.  It is absolutely not the case for my use case, as I need to support virtually all the operators.

Cheers,
Brook


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

Re: [yap] Review part 1: documentation

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

  I have not included the doxygen reference in this
as I'll get to that along with the code.

General:

- missing .gitattributes

introduction.html:

- "...expression syntax that ... optimizes your expressions. "
  The syntax doesn't optimize. It's the Eigen implementation
  that optimizes.

- Please use the correct syntax for bulleted lists.
  It's '*' not '-' in quickbook.

http://www.boost.org/doc/html/quickbook/syntax/block.html#quickbook.syntax.block.lists.unordered_lists

header_organization.html:

- I think it's better to put the actual name of the header
  as the primary text since that's what I actually need to
  know to #include it.

tutorial/expressions.html:

- "...any of the functions in Boost.YAP that takes"
  s/takes/take/

- "And if we evaluate it using..."
  And don't begin a sentence with a conjunction.

- "An expression containing an expression<> subexpression
  and subexpressions instantiated from N other ExpressionTemplates
  can be passed as an argument to any of the Expression-accepting
  Boost.YAP functions."
  This sentence is hard to read because it uses "expression"
  too many times in too many different ways.

- "there are lots of examples of how to do this in the Examples section"
  A link on "Examples" would be nice.

tutorial/kinds_of_exceptions.html:

- "kinds of expression"
  s/expression/expressions/

- "represents the leaf-node in an expression tree"
  I think "a leaf-node" would be more correct as
  an expression tree may have more than one leaf.

- "An if_else expression is analogous to the C++
  ternary operator (?:), but without the common-type
  conversion logic."
  So does this mean that such conversions are
  represented explicitly or are they simply not
  allowed?

tutorial/operators.html:

- BOOST_YAP_USER_BINARY_OPERATOR_MEMBER(plus, ::lazy_vector_expr)
  I feel like it should be possible to avoid needing to
  specify the ExpressionTemplate.  It should be possible
  to deduce the return type from `this`.
  Also, for binary operators, I'd really prefer to have
  a single macro that defines both expr + x and x + expr,
  as defining both is usually what we want for symmetric
  operators.

tutorial/explicit_transformations.html:

- "result is created by visiting each node in expr, in top-down,
  breadth-first order"
  Is it really breadth-first?  I would find that quite
  surprising as normal recursion creates a depth-first traversal.

- "In other words, if returns any non-Boost.YAP argument..."
  s/if/it/

- "If situations when you want to..."
  s/If/In/

- "--"
  An mdash is \u2014 (or you can use UTF-8 for the
  qbk source, I think).

- "evaluate() Is wrong for this task"
  s/Is/is/

tutorial/evaluating_expressions.html:

- I don't think that BOOST_YAP_CONVERSION_OPERATOR_TEMPLATE
  is a good idea.  Users can already define their own
  expression template types if the default behavior isn't
  good enough and your documentation encourages this anyway.
  A macro like this is just asking for accidental incompatibility.

tutorial/operator_macros.html:

- What about BOOST_YAP_USER_ANY_UDT_BINARY_OPERATOR?

tutorial/how_expression_operands_are_treated.html:

- "This implies that an rvalue remain treatable as a..."
  s/remain/remains/.  Also this sentence is somewhat
  awkward.

- T & - > PARTIAL_DECAY(T) &
  This can't possibly be right.  If you decay a reference
  to a function or array into a pointer, then you need
  to treat the pointer as an rvalue.

tutorial/customization_points.html:

- what do you need evaluate_as for?  I'm having a
  hard time thinking of uses that wouldn't be better
  handled as an explicit transform (Or perhaps just
  wrapping the expression in an implicit_cast_expr).
  The same goes for transform_expression.  Having more
  knobs and dials just makes the API confusing.  Also it looks
  to me like transform_expression will cause any
  placeholders to get dropped on the floor, as
  transform_expression doesn't take the extra
  arguments that would be used to replace placeholders.

- transform_expression is the odd man out in terms
  of naming.  I think it should be eval_something.
  You're already using the term transform as something
  else.

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

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

- "The above only applies when you have a ExpressionTransform"
  s/a/an/

examples/hello_world.html:

- I have no idea what you're talking about regarding eager
  evaluation in proto.  The corresponding Proto example
  also uses an explicit evaluate.

examples/hello_world_redux.html:

- "That's better!"
  What's better than what?

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?

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.

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.

examples/lazy_vector.html:

- "This move is something of a hack."
  I'm not sure that I would consider it a hack.  The general
  idea is that in evaluate(transform(expr)), any values held
  by expr should get moved into the result of transform so
  that they can be passed to evaluate.

- "We're able to write this more simply than the equivalent Proto code;
  since YAP is always lazy, we can just use the expressions below
  directly."
  Actually the same code works for proto as well.  I suspect that
  BOOST_PROTO_AUTO was just used for symmetry with expr1.

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

examples/mixed.html:

- I think you'd be better off making sin_tag a regular
  function object like in the proto example.

examples/map_list_of:

-       map.emplace(
            Key{std::forward<Key2 &&>(key)},
            Value{std::forward<Value2 &&>(value)}
        );
  Why create extra temporaries here?

-         return transform.map;
  This doesn't allow NRVO.

examples/future_group.html:

- "Assertion that left and right are compatible types"
  I don't think that is_same is the correct test.  What
  you want to guarantee is that the return types are the
  same on both sides.

manual/object_code.html:

- "The object code generated by modern template metaprogramming
  can be quite good, in large part due to the drastic reduction
  in the amount of metaprogramming necessary to determine the
  result type"
  I'm not sure I see the connection here.  Type-based metaprogramming
  generates no object code to begin with.

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 part 1: documentation

Boost - Dev mailing list
On Sat, Feb 10, 2018 at 6:01 PM, Steven Watanabe via Boost <
[hidden email]> wrote:

> AMDG


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


> - "And if we evaluate it using..."
>   And don't begin a sentence with a conjunction.
>

:)  Done.


> - "An expression containing an expression<> subexpression
>   and subexpressions instantiated from N other ExpressionTemplates
>   can be passed as an argument to any of the Expression-accepting
>   Boost.YAP functions."
>   This sentence is hard to read because it uses "expression"
>   too many times in too many different ways.
>

I've re-written it to hopefully make it clearer.  Please let me know if
it's still too hard to parse.

[snip]

Done.

tutorial/operators.html:

>
> - BOOST_YAP_USER_BINARY_OPERATOR_MEMBER(plus, ::lazy_vector_expr)
>   I feel like it should be possible to avoid needing to
>   specify the ExpressionTemplate.  It should be possible
>   to deduce the return type from `this`.
>   Also, for binary operators, I'd really prefer to have
>   a single macro that defines both expr + x and x + expr,
>   as defining both is usually what we want for symmetric
>   operators.
>

Agreed.  There is a bug for this on GitHub already; I plan to add this
after the review.


> tutorial/explicit_transformations.html:
>
> - "result is created by visiting each node in expr, in top-down,
>   breadth-first order"
>   Is it really breadth-first?  I would find that quite
>   surprising as normal recursion creates a depth-first traversal.
>

No, you're right.  That's wrong on its face, but more broadly it's badly
described.  What I was trying to describe was the short-circuiting nature
of the matching.  If a visited subexpression is handled by the transform,
the transform does not recurse from that point, unless you write that
recursion into the transform.  But that's not depth-first, of course.

[snip]

Done.

- "--"
>   An mdash is \u2014 (or you can use UTF-8 for the
>   qbk source, I think).
>

Thanks, I didn't know I could use Unicode characters in the qbk source in
any form.  "\u2104" works int the qbk source, btw.


> - "evaluate() Is wrong for this task"
>   s/Is/is/
>

Done.


> tutorial/evaluating_expressions.html:
>
> - I don't think that BOOST_YAP_CONVERSION_OPERATOR_TEMPLATE
>   is a good idea.  Users can already define their own
>   expression template types if the default behavior isn't
>   good enough and your documentation encourages this anyway.
>   A macro like this is just asking for accidental incompatibility.
>

I agree.  This and the implicit transforms-during-eval are on the chopping
block as far as I'm concerned.  The main reason they have not already been
cut is that I was waiting to see if reviewers really needed them to stay.


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

tutorial/how_expression_operands_are_treated.html:
>
> - "This implies that an rvalue remain treatable as a..."
>   s/remain/remains/.  Also this sentence is somewhat
>   awkward.
>

That's the right conjugation for what I was trying to say, though I agree
about the awkwardness.  I changed that fragment to "This implies that an
rvalue be treated as if it were a temporary".


> - T & - > PARTIAL_DECAY(T) &
>   This can't possibly be right.  If you decay a reference
>   to a function or array into a pointer, then you need
>   to treat the pointer as an rvalue.
>

Thanks!  Fortunately, the code doesn't do what I wrote in that table.


> tutorial/customization_points.html:
>
> - what do you need evaluate_as for?  I'm having a
>   hard time thinking of uses that wouldn't be better
>   handled as an explicit transform (Or perhaps just
>   wrapping the expression in an implicit_cast_expr).
>   The same goes for transform_expression.  Having more
>   knobs and dials just makes the API confusing.  Also it looks
>   to me like transform_expression will cause any
>   placeholders to get dropped on the floor, as
>   transform_expression doesn't take the extra
>   arguments that would be used to replace placeholders.
>
> - transform_expression is the odd man out in terms
>   of naming.  I think it should be eval_something.
>   You're already using the term transform as something
>   else.
>

Yes, I agree with all of this, for the reasons already stated.


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


> - "The above only applies when you have a ExpressionTransform"
>   s/a/an/
>

Done.


> examples/hello_world.html:
>
> - I have no idea what you're talking about regarding eager
>   evaluation in proto.  The corresponding Proto example
>   also uses an explicit evaluate.
>

I don't have any idea what I'm talking about there either.  I went back and
looked at all the Proto examples. and I can't figure out what it was I was
looking at before that made me think that.  I've removed the mention of
Proto altogether.


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

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.


> 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.  In my calc2a example, I show
the same functionality, just using lambdas to give the expressions names.
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.


> examples/lazy_vector.html:
>
> - "This move is something of a hack."
>   I'm not sure that I would consider it a hack.  The general
>   idea is that in evaluate(transform(expr)), any values held
>   by expr should get moved into the result of transform so
>   that they can be passed to evaluate.
>

You're right, of course.  I only meant that it's a little wonky to
std::move() and int to force Yap to make a copy.


> - "We're able to write this more simply than the equivalent Proto code;
>   since YAP is always lazy, we can just use the expressions below
>   directly."
>   Actually the same code works for proto as well.  I suspect that
>   BOOST_PROTO_AUTO was just used for symmetry with expr1.
>

True enough.  Comment removed.


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


> examples/map_list_of:
>
> -       map.emplace(
>             Key{std::forward<Key2 &&>(key)},
>             Value{std::forward<Value2 &&>(value)}
>         );
>   Why create extra temporaries here?
>

 Stupidity?  Sleepiness?  I just don't know.  A very existential question.
Regardless, this is now changed.


> -         return transform.map;
>   This doesn't allow NRVO.
>

Thanks!  I've changed the transform to operate on a reference to the return
type, and changed its use to this:

    template <typename Key, typename Value, typename Allocator>
    operator std::map<Key, Value, Allocator> () const
    {
        std::map<Key, Value, Allocator> retval;
        map_list_of_transform<Key, Value, Allocator> transform{retval};
        boost::yap::transform(*this, transform);
        return retval;
    }

examples/future_group.html:
>
> - "Assertion that left and right are compatible types"
>   I don't think that is_same is the correct test.  What
>   you want to guarantee is that the return types are the
>   same on both sides.
>

Yes, that's right.  I've stricken the static_assert(), as it's not strictly
necessary for the example anyway.


> manual/object_code.html:
>
> - "The object code generated by modern template metaprogramming
>   can be quite good, in large part due to the drastic reduction
>   in the amount of metaprogramming necessary to determine the
>   result type"
>   I'm not sure I see the connection here.  Type-based metaprogramming
>   generates no object code to begin with.
>

This is my intuition about part of what confuses the inliner -- I have seen
the inliner "get confused" (my characterization) and produce worse code in
these type-based metaprogramming-heavy situations, and then produce more
inlined calls when the metaprogramming is removed from the same code.  I
could certainly be wrong!  I'll strike that paragraph.

Thanks for the very thorough review, Steven!  You caught a lot.  I'm
looking forward to the second part, since I'm sure you'll catch a lot there
too.

Zach

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

Re: [yap] Review part 1: documentation

Boost - Dev mailing list
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 <
> [hidden email]> 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

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

Re: [yap] Review part 1: documentation

Boost - Dev mailing list
On Sun, Feb 11, 2018 at 5:54 PM, Steven Watanabe via Boost <
[hidden email]> wrote:

> 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 <
> > [hidden email]> wrote:
>

[snip]


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

It can go either way.  I'm using it in the singular.  The second definition
in Google's online dictionary for "any" is:

*2*.
whichever of a specified class might be chosen.


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

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]

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


True enough.  I hesitate to remove this feature because 1) it's already in
there, and no else has to reinvent it when it's needed in their code, and
2) I don't think it does any real harm.  Contrast this with the implicit
transforms associated with evaluate(), which I think lead to bad code.


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


Yeah, this last part is why I even left out some of the Proto examples.


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


True.

[snip]

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


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


Oh, I see.  Yeah, that's true.  I just wanted to show how to do a
transformed call I think.  I'll add this alternate way of accomplishing the
same thing to the example.

Zach

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

Re: Yap's formal review is starting now!

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

> On Feb 8, 2018, at 2:15 PM, Zach Laine via Boost <[hidden email]> wrote:
>
> So, I have been very resistant to adding another new evaluation mode.
> Instead, I think something like this should be usable in most cases:
>
> // Let's assume the existence of a my_make_term() function that takes a Yap
> terminal
> // and a Context &, and returns a new Yap terminal.
> template <typename Context>
> struct transform_terminals_with_context
> {
>    // Base case. Note that I'm ignoring placeholders entirely for this
>    // example (they're easy to special-case if necessary).
>    template <typename T>
>    auto operator() (yap::terminal_tag, T && t)
>    { return my_make_term(std::forward<T>(t), context_); }
>
>    // Recursive case: Match any binary expression.
>    template <typename Tag, typename LExpr, typename RExpr>
>    auto operator() (Tag, LExpr const & left, RExpr const & right)
>    {
>        return yap::make_expression<yap::detail::kind_for<Tag>>(
>            boost::yap::transform(left, *this),
>            boost::yap::transform(right, *this));
>    }
>
>    // Recursive case: Match any unary expression.
>    template <typename Tag, typename Expr>
>    auto operator() (Tag, Expr const & expr)
>    {
>        return yap::make_expression<yap::detail::kind_for<Tag>>(
>            boost::yap::transform(expr, *this));
>    }
>
>    // Ternary and call are added as necessary.
>
>    Context & context_;
> };

So just to be sure, I made up what I think is a complete example to illustrate your main Yap idiom.  The features I wanted to capture are:

- the evaluate(transform(expr,xform)) idiom

- move-only terminals

- terminals with no particular expression semantics that are transformed into values that have expression semantics

- a customization point in the transform to support arbitrary user-defined types being added to the transform

- ignore context-dependent transforms for now; given this pattern those are easy to support since a transform object is available everywhere a terminal is transformed into a value

In what follows, I am thinking of the "user" namespace as user-defined code and the "N" namespace as library code.

Have I missed anything of note?

Please also note that yap::detail::kind_for<Tag> in your code above should not be in the "detail" namespace as that is something that library writers building things upon Yap will be using and therefore it is not an implementation detail.  I also think you didn't quite get the yap::make_expression<>() call correct in the above code.

Cheers,
Brook



#include <boost/yap/algorithm.hpp>

namespace user
{

  template < typename T = void >
  class UDT {};

  template < typename T, typename Transform >
  auto transform_terminal (UDT<T>, Transform) { return 1; }

} // namespace user

namespace N
{

  template <boost::yap::expr_kind Kind, typename Tuple>
  struct minimal_expr
  {
    static const boost::yap::expr_kind kind = Kind;
    Tuple elements;
    BOOST_YAP_USER_BINARY_OPERATOR_MEMBER(plus,::N::minimal_expr)
  };

  template < typename T = double >
  class number
  {
  public:
    number () = default;
    explicit number (T value) : value_(value) {}
    number (number const&) = delete;
    number (number&&) = default;
    auto value () const { return value_; }
  private:
    T value_ = 0;
  };

  template < typename Tag >
  class Kind;

  template <>
  struct Kind< boost::yap::plus_tag >
  {
    static constexpr boost::yap::expr_kind value = boost::yap::expr_kind::plus;
  };

  struct xform
  {
    // recursion: any binary operator
    template < typename Tag, typename Left, typename Right >
    decltype(auto) operator () (Tag, Left&& left, Right&& right) const
    {
      return boost::yap::make_expression<minimal_expr,Kind<Tag>::value>
        (boost::yap::transform(std::forward<Left>(left),*this),
         boost::yap::transform(std::forward<Right>(right),*this));
    }

    // base case: any terminal (requires transform_terminal() overload)
    template < typename Terminal >
    decltype(auto) operator () (boost::yap::terminal_tag, Terminal const& terminal) const
    { return transform_terminal(terminal,*this); }

    // base case: number<T>: this can be in the transform (as here) or
    // a transform_terminal overload (as for user::UDT)
    template < typename T >
    auto operator () (boost::yap::terminal_tag, number<T> const& n) const
    { return boost::yap::make_terminal<minimal_expr>(n.value()); }

    // ... more overloads for other terminals, unary operators, etc.
  };

  template < typename T >
  auto lit (T&& t)
  { return boost::yap::make_terminal<minimal_expr>(std::forward<T>(t)); }

} // namespace N

int main ()
{
  using boost::yap::evaluate;
  using boost::yap::transform;
  using N::lit;

  auto plus_expr = lit(N::number<>{41}) + user::UDT<>{};
  auto plus_result = evaluate(transform(plus_expr,N::xform{}));
  assert(plus_result == 42);

  return 0;
}


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

Re: [yap] Review part 1: documentation

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

On 02/12/2018 09:45 AM, Zach Laine wrote:

> On Sun, Feb 11, 2018 at 5:54 PM, Steven Watanabe via Boost <
> [hidden email]> 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.

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

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's formal review is starting now!

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Mon, Feb 12, 2018 at 11:28 AM, Brook Milligan <[hidden email]> wrote:

>
> > On Feb 8, 2018, at 2:15 PM, Zach Laine via Boost <[hidden email]>
> wrote:
> >
> > So, I have been very resistant to adding another new evaluation mode.
> > Instead, I think something like this should be usable in most cases:
> >
> > // Let's assume the existence of a my_make_term() function that takes a
> Yap
> > terminal
> > // and a Context &, and returns a new Yap terminal.
> > template <typename Context>
> > struct transform_terminals_with_context
> > {
> >    // Base case. Note that I'm ignoring placeholders entirely for this
> >    // example (they're easy to special-case if necessary).
> >    template <typename T>
> >    auto operator() (yap::terminal_tag, T && t)
> >    { return my_make_term(std::forward<T>(t), context_); }
> >
> >    // Recursive case: Match any binary expression.
> >    template <typename Tag, typename LExpr, typename RExpr>
> >    auto operator() (Tag, LExpr const & left, RExpr const & right)
> >    {
> >        return yap::make_expression<yap::detail::kind_for<Tag>>(
> >            boost::yap::transform(left, *this),
> >            boost::yap::transform(right, *this));
> >    }
> >
> >    // Recursive case: Match any unary expression.
> >    template <typename Tag, typename Expr>
> >    auto operator() (Tag, Expr const & expr)
> >    {
> >        return yap::make_expression<yap::detail::kind_for<Tag>>(
> >            boost::yap::transform(expr, *this));
> >    }
> >
> >    // Ternary and call are added as necessary.
> >
> >    Context & context_;
> > };
>
> So just to be sure, I made up what I think is a complete example to
> illustrate your main Yap idiom.  The features I wanted to capture are:
>
> - the evaluate(transform(expr,xform)) idiom
>
> - move-only terminals
>

I'm not sure what you mean by this, because the terminal type in your
example is copyable.  Do you mean showing how to force copies with moves,
or something else?

- terminals with no particular expression semantics that are transformed

> into values that have expression semantics
>
> - a customization point in the transform to support arbitrary user-defined
> types being added to the transform
>
> - ignore context-dependent transforms for now; given this pattern those
> are easy to support since a transform object is available everywhere a
> terminal is transformed into a value
>
> In what follows, I am thinking of the "user" namespace as user-defined
> code and the "N" namespace as library code.
>
> Have I missed anything of note?
>

Yes, that covers the main moving parts as far as I can see.


> Please also note that yap::detail::kind_for<Tag> in your code above should
> not be in the "detail" namespace as that is something that library writers
> building things upon Yap will be using and therefore it is not an
> implementation detail.  I also think you didn't quite get the
> yap::make_expression<>() call correct in the above code.
>

Agreed.  Once added, kind_for<>() would go in yap::.

Zach

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

Re: [yap] Review part 1: documentation

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Mon, Feb 12, 2018 at 12:08 PM, Steven Watanabe via Boost <
[hidden email]> 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 <
> > [hidden email]> 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

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

Re: [yap] Review part 1: documentation

Boost - Dev mailing list
AMDG

On 02/12/2018 06:09 PM, Zach Laine wrote:

> On Mon, Feb 12, 2018 at 12:08 PM, Steven Watanabe via Boost <
> [hidden email]> wrote:
>
>> <snip>
>> 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.
>

I don't think as_expr helps with the problems I'm thinking of.

Unless I'm misunderstanding something, the following
(seemingly reasonable) code will blow up the stack with
infinite recursion:

int check_int(int i)
{
  assert(i >= -42 && i < 42);
  return i;
}

int checked_add(int i, int j)
{
  return check_int(i + j);
}

struct checked_call
{
  template<class F>
  int operator()(F f, int i)
  {
    return check_int(f(i));
  }
};

struct int_checker
{
  auto operator()(terminal_tag, placeholder<I>)
  {
    return make_expression<expr_kind::call>(check_int,
      placeholder<I>{});
  }
  template<class L, class R>
  auto operator()(plus_tag, L && lhs, R && rhs)
  {
    return make_expression<expr_kind::call>(checked_add,
      transform(lhs, *this), transform(rhs, *this));
  }
  template<class F, class E>
  auto operator()(call_tag, F&& f, E&&... e)
  {
    return make_expression<expr_kind::call>(checked_call{},
      f, transform(e, *this)...);
  }
};

abs_ = make_terminal((int(*)(int))&std::abs);

evaluate(transform(abs_(1_p + 2_p), int_checker{}), -5, 20);

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's formal review is starting now!

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

> On Feb 12, 2018, at 6:02 PM, Zach Laine <[hidden email]> wrote:
>
> - move-only terminals
>
> I'm not sure what you mean by this, because the terminal type in your example is copyable.  Do you mean showing how to force copies with moves, or something else?

How is this copyable?

number (number const&) = delete;

> Have I missed anything of note?
>
> Yes, that covers the main moving parts as far as I can see.

Good.

Cheers,
Brook


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

Re: Yap's formal review is starting now!

Boost - Dev mailing list
On Mon, Feb 12, 2018 at 11:38 PM, Brook Milligan <[hidden email]> wrote:

>
> > On Feb 12, 2018, at 6:02 PM, Zach Laine <[hidden email]>
> wrote:
> >
> > - move-only terminals
> >
> > I'm not sure what you mean by this, because the terminal type in your
> example is copyable.  Do you mean showing how to force copies with moves,
> or something else?
>
> How is this copyable?
>

Ah!  I totally missed the deleted copy operations the first time.  Thanks.

Zach

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

Re: [yap] Review part 1: documentation

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Mon, Feb 12, 2018 at 10:13 PM, Steven Watanabe via Boost <
[hidden email]> wrote:

> AMDG
>
> On 02/12/2018 06:09 PM, Zach Laine wrote:
> > On Mon, Feb 12, 2018 at 12:08 PM, Steven Watanabe via Boost <
> > [hidden email]> wrote:
> >
> >> <snip>
> >> 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.


I guess I was reading too fast.  I missed or misread this last sentence
entirely:


> In particular, if the terminal
> >> transform returns an Expression, that Expression will
> >> get processed again.
>

I get what you're concerned about now.  That's not an issue.  An expression
is processed at most once by transform().

If, during a call to transform(), an expression E matches the
transform-object TO, the result of TO(E) is used in the output of
transform().  If, in the TO(E) call, its children are processed in some
way, it is the user's responsibility not to infinitely recurse.
transform() will copy or move the partial result TO(E), but will never
revisit/re-transform it.

[snip]

Unless I'm misunderstanding something, the following

> (seemingly reasonable) code will blow up the stack with
> infinite recursion:
>
> int check_int(int i)
> {
>   assert(i >= -42 && i < 42);
>   return i;
> }
>
> int checked_add(int i, int j)
> {
>   return check_int(i + j);
> }
>
> struct checked_call
> {
>   template<class F>
>   int operator()(F f, int i)
>   {
>     return check_int(f(i));
>   }
> };
>
> struct int_checker
> {
>   auto operator()(terminal_tag, placeholder<I>)
>   {
>     return make_expression<expr_kind::call>(check_int,
>       placeholder<I>{});
>   }
>   template<class L, class R>
>   auto operator()(plus_tag, L && lhs, R && rhs)
>   {
>     return make_expression<expr_kind::call>(checked_add,
>       transform(lhs, *this), transform(rhs, *this));
>   }
>   template<class F, class E>
>   auto operator()(call_tag, F&& f, E&&... e)
>   {
>     return make_expression<expr_kind::call>(checked_call{},
>       f, transform(e, *this)...);
>   }
> };
>
> abs_ = make_terminal((int(*)(int))&std::abs);
>
> evaluate(transform(abs_(1_p + 2_p), int_checker{}), -5, 20);


 Let's look at the call graph for this:

First transform() will call operator()(call_tag, ...) on abs_(1_p + 2_p).
The result is make_expression<expr_kind::call>(checked_call{}, abs_,
transform(1_p + 2_p), *this).

To get that result, transform() will call (a *new* call to transform() at
the user's request, not as part of the top-level call): operator()(plus_tag,
...) on 1_p and 2_p.  The result is make_expression<expr_kind::
call>(checked_add{}, transform(1_p, *this), transform(2_p, *this)).

Just following the left side: to evaluate that, transform() (again, a *new*
call): operator()(terminal_tag, 1_p), which returns
make_expression<expr_kind::call>(check_int, 1_p).

The right does something very similar, just with 2_p.

No infinite recursion.

Zach

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

Re: [yap] Review part 1: documentation

Boost - Dev mailing list
AMDG

On 02/13/2018 09:44 AM, Zach Laine wrote:

> On Mon, Feb 12, 2018 at 10:13 PM, Steven Watanabe via Boost <
> [hidden email]> wrote:
>
>> On 02/12/2018 06:09 PM, Zach Laine wrote:
>>> On Mon, Feb 12, 2018 at 12:08 PM, Steven Watanabe via Boost <
>>> [hidden email]> wrote:
>>>
>>>> <snip>
>>>> 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.
>
>
> I guess I was reading too fast.  I missed or misread this last sentence
> entirely:
>
>
>> In particular, if the terminal
>>>> transform returns an Expression, that Expression will
>>>> get processed again.
>>
>
> I get what you're concerned about now.  That's not an issue.  An expression
> is processed at most once by transform().
> > If, during a call to transform(), an expression E matches the
> transform-object TO, the result of TO(E) is used in the output of
> transform().  If, in the TO(E) call, its children are processed in some
> way, it is the user's responsibility not to infinitely recurse.
> transform() will copy or move the partial result TO(E), but will never
> revisit/re-transform it.
>
> [snip]
>
> Unless I'm misunderstanding something, the following
>> (seemingly reasonable) code will blow up the stack with
>> infinite recursion:
>>
>> int check_int(int i)
>> {
>>   assert(i >= -42 && i < 42);
>>   return i;
>> }
>>
>> int checked_add(int i, int j)
>> {
>>   return check_int(i + j);
>> }
>>
>> struct checked_call
>> {
>>   template<class F>
>>   int operator()(F f, int i)
>>   {
>>     return check_int(f(i));
>>   }
>> };
>>
>> struct int_checker
>> {
>>   auto operator()(terminal_tag, placeholder<I>)
>>   {
>>     return make_expression<expr_kind::call>(check_int,
>>       placeholder<I>{});
>>   }
>>   template<class L, class R>
>>   auto operator()(plus_tag, L && lhs, R && rhs)
>>   {
>>     return make_expression<expr_kind::call>(checked_add,
>>       transform(lhs, *this), transform(rhs, *this));
>>   }
>>   template<class F, class E>
>>   auto operator()(call_tag, F&& f, E&&... e)
>>   {
>>     return make_expression<expr_kind::call>(checked_call{},
>>       f, transform(e, *this)...);
>>   }
>> };
>>
>> abs_ = make_terminal((int(*)(int))&std::abs);
>>
>> evaluate(transform(abs_(1_p + 2_p), int_checker{}), -5, 20);
>
>
>  Let's look at the call graph for this:
>
> First transform() will call operator()(call_tag, ...) on abs_(1_p + 2_p).
> The result is make_expression<expr_kind::call>(checked_call{}, abs_,
> transform(1_p + 2_p), *this).
>
> To get that result, transform() will call (a *new* call to transform() at
> the user's request, not as part of the top-level call): operator()(plus_tag,
> ...) on 1_p and 2_p.  The result is make_expression<expr_kind::
> call>(checked_add{}, transform(1_p, *this), transform(2_p, *this)).
>

  I think you're forgetting that the terminal transform is
applied before calling operator()(xxx_tag, ...).
Thus, the result is actually:

make_expression<expr_kind::call>(checked_add{},
  transform(transform(1_p, *this), *this),
  transform(transform(2_p, *this), *this)).

  Since the extra transform tacks on another call expr,
we end up up coming right back around to operator()(call_tag).

  The bottom line is, any transform which is not idempotent
for terminals is badly broken if you unwrap terminals
automatically.

  If you don't apply the terminal transform, but still
unwrap terminals, then it's *still* broken, because
then the terminal transform gets skipped entirely.

  Actually, the code I posted fails to compile (even after
fixing the obvious typos) because the recursion makes
it impossible for auto to deduce the return type.

> Just following the left side: to evaluate that, transform() (again, a *new*
> call): operator()(terminal_tag, 1_p), which returns
> make_expression<expr_kind::call>(check_int, 1_p).
>
> The right does something very similar, just with 2_p.
>
> No infinite recursion.
>

In Christ,
Steven Watanabe

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