[boost] [Boost.Yap] Review results

Next Topic
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view

[boost] [Boost.Yap] Review results

Boost - Announce mailing list
Dear Boost,

I am pleased to announce that the Yap library is CONDITIONALLY ACCEPTED
into Boost. I counted the following formal reviews:

- Barrett Adair: ACCEPT
- John Fletcher: ACCEPT
- Paul Fultz II: ACCEPT
- Steven Watanabe: ACCEPT

I'd like to thank everyone who participated to the review for their
and also Zach Laine for his submission. The following is a summary of the
main issues that were raised during the review:

- Contextual evaluation of terminals
   The concern was that there was no way of evaluating an expression tree
   with a custom context other than to first transform it with a custom
   transform, and then evaluate it. This, however, requires encoding the
   recursion into the transform even when we only want to modify terminals.
   After a lot of discussion, everybody seemed to agree that the following
   would be desirable (paraphrasing Steven):

       `transform_evaluate` would behave exactly like `transform`, except
       that the default behavior for nodes that are not handled by the
       context is to evaluate the operators instead of building a new

   Zach fixed that in https://github.com/tzlaine/yap/issues/63 by
   a new transform called `evaluation()` which evaluates terminals, and the
   ability to pass multiple transforms `t1`, ..., `tN` to `transform`. The
   first matching transform is executed, and `transform` will
   if no transforms match. `transform_evaluate(expr, tform)` can then be
   implemented as `yap::transform(expr, tform, yap::evaluation())`.

- Auto-unwrapping of terminals in transform and automatic application of
 terminal transforms before unwrapping
   There was a concern that automatically unwrapping terminals in
   and applying terminal transforms before unwrapping would result in
   intuitive behavior. After some discussion, it was determined that
   application of transforms before unwrapping could indeed lead to
   results and was removed. However, unwrapping of terminals is necessary
   keep the matching of terminals reasonable, so it was kept. I am
   with that decision, as the unwrapping of terminals, which leads to a
   intuitive way of matching terminals, was one of the features that sprung
   to me as being a great usability boost when I first looked at the

- Positional placeholders should stay or go?
   Some of the reviewers felt like it was not desirable to provide
   placeholders as part of Yap, because those aren't the primary purpose of
   the library and C++14 obsoletes them with lambdas. Other reviewers felt
   like they were in fact very useful and shouldn't be removed. Since no
   clear consensus could be reached, no acceptance condition is attached to
   this issue. However, I personally favor the removal of placeholders from
   the core API on the basis of minimality. I would like to encourage Zach
   to check whether their removal would result in API simplifications; if
   so, I would encourage (but not require) their removal, at Zach's

- Documentation
   The documentation is not sufficiently extensive, and takes too much for
   granted from the reader. This was raised by a couple of reviewers and it
   has been my largest problem with the library so far. More on this below.

- The eval_xxx customization points.
   Nobody liked them, and they are already gone.

   Nobody liked it, and it is already gone.

- Short-circuiting of && and || in evaluate
   Steven brought up the fact that `yap::evaluate` would not short-circuit
   the evaluation of && and ||. This issue was mooted when customization
   points were removed. Also, the new `evaluation()` transform, which could
   suffer from the same short-circuiting problems, doesn't because it was
   implemented properly.

- Support for Boost.Build is missing.

- Hana vs not Hana
   Full disclosure: I am the author of Boost.Hana.
   A couple of reviewers raised concerns with Hana being part of the public
   interface. The main concern seemed to be about MSVC not being able to
   compile Hana, and hence Yap being unusable on MSVC. While I agree that
   MSVC is a big player, I believe that should not dictate what all Boost
   libraries do. Some libraries are meant to bridge gaps with older
   some libraries are cutting edge. Yap is the latter, and that's fine. The
   MSVC folks are working hard on their C++ conformance, and they will
   eventually get there. Also, it is likely that Hana wouldn't be the
   anyway: Yap relies a lot on SFINAE and MSVC is known to be poor at it.
   Many of the features that one would likely want to use when using Yap
   also not too well supported by MSVC, so users would probably hit
   there anyway. Plus, as soon as MSVC is close enough, I'll start working
   Hana's side to support it.

   Taking my review manager hat off for a second, I'd also like to point
   out that Yap is exactly the kind of use case that Hana was built for.
   Just like MPL was (and is) used in many libraries' implementation and
   APIs, I don't see a problem with using Hana in Yap's API, since that
   is the right tool for the job.

   That being said, if Zach wishes to explore using Hana's concepts (like
   hana::Sequence) to conceptify his API, I encourage him to do so. This
   make it possible to use any tuple-like sequence for the children of an
   expression, thereby reducing the coupling between the two libraries. If
   Zach wants to explore using Fusion instead of Hana, that is also an
   that would make MSVC support achievable.

- A single macro for symmetric binary operators
   It was brought up that there's no macro to define both `arg <op> expr`
   and `expr <op> arg` at once. I believe this was initially done to avoid
   potential conflicts with the member variants, which already provide
   `expr <op> arg`. I believe it should still be possible to do it, since,
   as Steven said, most of the time a user will need symmetric behavior

- External dependencies checked-into the repository
   The library includes internal copies of some of its dependencies, which
   is unusual for a Boost library. Zach said it wasn't his intent to put
   library into Boost that way if it were accepted.

To see all the issues that were raised during the review, you can search
for tickets labeled with `Boost Review` on GitHub:


Based on the concerns above, the following lists the conditions for

- Documentation
   In my opinion, this was the biggest concern to be addressed. A lot of
   progress was made during and after the review, but I still think the
   documentation can be improved and made more complete. I especially
   believe that integrating more examples inside the "tutorial" (not just
   at the end) and introducing new concepts more gradually would help
   that are not already expression template gurus. It is very difficult to
   set a clear action item for "improving the documentation". For this
   Brook Milligan and I will work with Zach to define what it means for the
   documentation to meet this acceptance condition. Brook was kind enough
   agree to this when I asked him in a private communication.

- A single macro for symmetric binary operators
   The library must provide a way to define symmetric binary operators in
   one go. Exactly how this can be achieved and whether we should be able
   to SFINAE-constrain the arguments is left to Zach's discretion.

- External dependencies
   The external dependencies must not be packaged with the library itself.
   Another mechanism should be used; I leave that mechanism unspecified,
   but it should be possible to test the library using the usual
   workflow (e.g. an ExternalPackage download that would only work in CMake
   is not acceptable).

- Boost.Build
   Yap needs to support Boost.Build like all the other Boost libraries.

I will post an update to this list when the conditions of the review are
If they wish to do so, reviewers can then verify that the conditions are met
in a satisfactory fashion and the library will automatically be accepted
unless someone objects.

I would like to thank everyone who participated in the review, and
congratulations to Zach for his library.

Louis Dionne

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