I accidentally sent this to boost-announce where it is blocked in
moderation - resending to the main list.
On Fri, 8 Sep 2017 07:02:27 -0400
Matt Calabrese via Boost via Boost-announce
A formal review of the Fit library developed by Paul Fultz II starts today, September 8, and runs through September 17.
A branch has been made for the review. You can find Fit for review at these links:
Source: https://github.com/pfultz2/Fit/tree/boost Docs: http://pfultz2.github.io/Fit/doc/html/doc/
[...]
Requirements
This requires a C++11 compiler. There are no third-party dependencies. This has been tested on clang 3.5-3.8, gcc 4.6-6.2, and Visual Studio 2015. Gcc 5.1 is not supported at all, however, gcc 5.4 is supported.
Contexpr support:
Both MSVC and gcc 4.6 have limited constexpr support due to many bugs in the implementation of constexpr. However, constexpr initialization of functions is supported when using the FIT_STATIC_FUNCTION and FIT_STATIC_LAMBDA_FUNCTION constructs.
`FIT_LIFT` uses `auto` in a C++14-style lambda, and thus does not compile when in C++11 mode.
Noexcept support:
On older compilers such as gcc 4.6 and gcc 4.7, noexcept is not used due to many bugs in the implementation. Also, most compilers don’t support deducing noexcept with member function pointers. Only newer versions of gcc(4.9 and later) support this.
====================
Please provide in your review whatever information you think is valuable to understand your final choice of ACCEPT or REJECT including Fit as a Boost library. Please be explicit about your decision.
I would vote to accept the library.
Some other questions you might want to consider answering:
- What is your evaluation of the design?
Overall I have a positive impression of the design. In general there is a logical consistency to the adaptors / decorators. The `infix` operator still feels like a gimmick - why include it in the library? I cannot think of an instance where I would use such a utility, especially considering the fragility of operator precedence.
- What is your evaluation of the implementation?
I think there are too many macros internally, but I think its mostly to work around compilers. So its about average for a boost library. I also do not like the inheritance from user-callables, as it seems unnecessary in some cases (although perhaps easiest for `match`, etc.?).
- What is your evaluation of the documentation?
The documentation seems to be improved from what I remember. At first I felt like the getting started and examples section was "too much too fast", but I am not sure there is an easy way to introduce a programmer to some of these concepts. Is the library inspired by another, or entirely by "point-free programming"? I am trying to think of some outside resource to reference, but I cannot think of any.
- What is your evaluation of the potential usefulness of the library?
Some of the utilities will be valuable for C++11 projects (move capture), and I also like the "pack" handling functions that will not be available until C++17. A number of the adaptors seem like they could be useful, but I've also not written anything similar or considered doing so. Which might mean I need to do more functional programming.
- Did you try to use the library? With which compiler(s)? Did you have any problems?
This time I just used the `BOOST_LIFT` function, tried out the auto-placeholder and pipable adaptor to study their behavior.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I spent significantly more the first review, particularly with the implementation. I spent only a 1-2 hours looking at the code / documentation this time.
- Are you knowledgeable about the problem domain?
This is a fairly unique project; I do not know specifics about this type of point-free or functional programming. I am fairly familiar with std/boost bind and the usage of basic callables.
- Were the concerns from the March 2016 review of Fit addressed?
I brought up inheritance from user types as a potential issue. The author disagreed? A small implementation quirk, but can lead to surprising behavior with operator overloads, etc. I also suggested (i.e. not a concern necessarily) that the placeholders be placed into their own namespace. This would allow `using boost::fit::placeholders` without bringing the entire `boost::fit` namespace into scope. The library could still alias the placeholders into the `boost::fit` namespace as well. There is also this strange oddity: struct { int operator()(int x, int y = 10) const noexcept { return x + y; } } sum{}; const auto pipe_sum = boost::fit::pipable(sum); int main() { std::cout << (1 | pipe_sum) << std::endl; return 0; } The function is immediately evaluated, without `()` on pipe_sum and outputs `11`. I looked through my post last time, and recommended that something in the documentation mention that `pipable` behaves like a `partial` evaluation. Or again, maybe that's obvious from the pack listed in the documentation? So while the partial evaluation might be obvious due to the documentation, the immediate evaluation without the extra `()` is still intriguing.
More information about the Boost Formal Review Process can be found here: http://www.boost.org/community/reviews.html
Thank you for participating!
Lee