Boost-announce

Download

boost-announce@lists.boost.org

March 2018

1 participants
1 discussions

[boost] [Boost.Yap] Review results
by Louis Dionne via Boost 01 Mar '18

01 Mar '18

Dear Boost, I am pleased to announce that the Yap library is CONDITIONALLY ACCEPTED into Boost. I counted the following formal reviews: - Alex Hagen-Zanker: REJECT / CONDITIONAL ACCEPT - Barrett Adair: ACCEPT - Brook Milligan: CONDITIONAL ACCEPT - John Fletcher: ACCEPT - Paul Fultz II: ACCEPT - Steven Watanabe: ACCEPT I'd like to thank everyone who participated to the review for their feedback, 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 expression. Zach fixed that in https://github.com/tzlaine/yap/issues/63 by introducing 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 copy-and-recurse 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 transforms and applying terminal transforms before unwrapping would result in counter intuitive behavior. After some discussion, it was determined that automatic application of transforms before unwrapping could indeed lead to surprising results and was removed. However, unwrapping of terminals is necessary to keep the matching of terminals reasonable, so it was kept. I am comfortable with that decision, as the unwrapping of terminals, which leads to a very 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 library. - Positional placeholders should stay or go? Some of the reviewers felt like it was not desirable to provide positional 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 discretion. - 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. - BOOST_YAP_CONVERSION_OPERATOR_TEMPLATE 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 compilers, 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 barrier 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 are also not too well supported by MSVC, so users would probably hit limitations there anyway. Plus, as soon as MSVC is close enough, I'll start working on 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 may 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 option 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 anyway. - 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 the 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: https://github.com/tzlaine/yap/issues?q=label%3A%22Boost+Review%22 Based on the concerns above, the following lists the conditions for acceptance: - 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 readers that are not already expression template gurus. It is very difficult to set a clear action item for "improving the documentation". For this reason, 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 to 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 Boost.Build 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 met. 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. Regards, Louis Dionne _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

1 0