On Mon, 2017-09-18 at 19:48 -0700, Jason Rice via Boost wrote:

Please consider this review for the proposed Boost.Fit library.

I recommend the review manager ACCEPT this library for inclusion in Boost without condition.

# What is your evaluation of the design?   - Making high order functions easier and more concise is a good idea.   - I like the point-free programming features.   - Some of the names, conventions, and errors would take     any kind of user some getting used to.   - A version of `alias_value` that works in a generic context where the     type might not be an alias would be more useful I think.

Hmm, interesting, do you have a use case where this would be useful?

# What is your evaluation of the implementation?   - The code is consistent across implementations and the files are pretty flat   - Each function can be included independently which is nice.   - It appears that there is some effort towards making errors easier to read.     For instance, I called `fit::partial` with too many arguments and the error     was "error: no member named 'fit_function_param_limit'"

The error should be 'no matching function'. Do you have the code that produced this error?

  - There is some readability issues due to compiler compatibility support and     specific compiler error workarounds.   - I noticed `fit::decay` is not used in the tests at all. It gets included in     `example.h` but is never used.

It is used by `pack`: https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/pack.hpp#L353 But its probably good to have unit tests for that component.

# What is your evaluation of the documentation?   - I like that the documentation is easy to read in the comments of each file.   - Some of the wording in the descriptions weren't very helpful for me personally,     but the "Semantics" section seemed to always get the point of the function across.

# What is your evaluation of the potential usefulness of the library?   - It seems parts of the library might not be that useful if you have     constexpr lambdas, but I believe there are still features that enable more     concise coding even against latest C++17 features.   - I will probably use this library now that I am familiar with it.

# Did you try to use the library? With which compiler(s)? Did you have any problems?   - I made a couple of my own examples.   - I used a fairly recent fork of clang trunk with c++1z.   - BOOST_FIT_RETURNS *crashed the compiler* when I was trying to construct an std::array     with some pack expansion. It looked like it crashed the parser.

Do you have an example of this? I could possibly workaround this.

# How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?   - I spent a couple of hours looking at the docs in the source and making my own test     programs to see how it could be used.

# Are you knowledgeable about the problem domain?   - I've used Boost.Hana a lot with its "functional" features.   - I've done a lot of point free programming with RamdaJS in Javascript

# Were the concerns from the March 2016 review of Fit addressed?   N/A

For what it's worth here is a link to some of my attempts to sample the library: https://gist.github.com/ricejasonf/9bfd233b3bc282eae9ecb8bc3198d948

Thanks for the review.

Thank You,

Jason Rice

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boo st