Subject: Re: [boost] [review][Fit] Review of Fit starts today
From: paul (pfultz2_at_[hidden])
Date: 2017-09-19 14:28:52
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
> Â - Each function can be included independently which is nice.
> Â - It appears that there is some effort towards making errors easier to
> Â Â Â Â For instance, I called `fit::partial` with too many arguments and the
> Â Â Â Â was "error: no member named 'fit_function_param_limit'"
The error should be 'no matching function'. Do you have the code that produced
> Â - There is some readability issues due to compiler compatibility support
> Â Â Â Â 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`:
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
> Â - Some of the wording in the descriptions weren't very helpful for me
> Â Â Â Â 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
> Â Â Â Â 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
> Â - 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.
> # 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
Thanks for the review.
> Thank You,
> Jason Rice
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boo
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk