On Sat, Apr 8, 2017 at 1:09 PM, Zach Laine via Boost
On Mon, Mar 27, 2017 at 11:38 PM, Louis Dionne via Boost < boost@lists.boost.org> wrote:
Dear Boost community,
[snip]
We encourage your participation in this review. At a minimum, kindly state: - Whether you believe the library should be accepted into Boost
Yes.
- Your name
Zach Laine
- Your knowledge of the problem domain
I am reasonably knowledgable. I do extensive metaprogramming from time to time, though this is not my specialty.
You are strongly encouraged to also provide additional information: - What is your evaluation of the library's: * Design
This seems good to me. There's one thing I consider to be a glaring omission, though. I would love to have pre-17 versions of these traits:
template
struct is_invocable; template struct is_invocable_r; template struct is_nothrow_invocable; template struct is_nothrow_invocable_r;
I had implemented something similar, even to the point of passing libc++'s std::invoke test suite, but I removed it after learning about these C++17 additions. You aren't the only one to request this feature. I've opened an issue to add these. On a related note, at one point I also implemented is_constexpr_invokable as an experiment. Do you think this would also have merit for inclusion? The merit of this trait is questionable to me, and the linked implementation does things it shouldn't do (coercive template instantiation), but here it is anyway: https://github.com/badair/constexpr_checks/blob/master/constexpr_checks.hpp#...
* Implementation
I did not look at the implementation.
* Documentation
From the first page of the docs, this:
" The use cases for CallableTraits overlaop significantly with those of Boost.FunctionTypes http://www.boost.org/doc/libs/1_61_0/libs/function_types/doc/html/index.html. Here are some reasons why you might prefer CallableTraits over the latter: "
is awkward enough that I had to re-read it. It mentions two things, but in the order A B A, then uses "latter", which is off-putting. Can I suggest instead:
" The use cases for CallableTraits overlap significantly with those of Boost.FunctionTypes http://www.boost.org/doc/libs/1_61_0/libs/function_types/doc/html/index.html. Here are some reasons why you might prefer CallableTraits: "
Also, "overlaop" is a typo. :)
Thanks, this is being addressed.
I also find it odd that the FAQ immediately follows the intro page. I think this would be better placed before the Contact section. Keeping it where it is now disturbs the flow of the reader's thinking when getting to know the library for the first time (or at least it did so for me).
Good point, I'll move this.
Nearly every section of the reference docs says this:
" Compatibility Notes
Full support on GCC 4.7.4+, Clang 3.5+, Visual Studio 2015, and XCode 6.4+. "
This is not a problem per se, but it's noise considering that the same statement is made in the intro section. I think the reference entry should only contain such a notice *only* when it differs from the general note from the intro. This has the benefit of being clearer -- I won't see the note above and wonder if it differs slightly from the general intro note.
Agreed, will fix.
* Tests
They seem to provide decent coverage, and line up with the documented semantics. The docs in fact seem to use quoted test code to illustrate behavior, which is nice.
* Usefulness
Very useful. As someone who is not a metaprogramming specialist, I often need facilities like this when doing MP, and I often define my versions with subtle defects.
- Did you attempt to use the library?
Not really. I only ran the tests on Clang 3.9.
- How much effort did you put into your evaluation of the review?
About 5 hours.
Zach
Zach, thank you for spending so much time on this review.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost