From: Julien Blanc (julien.blanc_at_[hidden])
Date: 2021-03-25 18:10:33
Le lundi 22 mars 2021 à 10:35 +0800, Joel de Guzman via Boost a écrit :
> The Boost formal review of the Lambda2, authored by Peter Dimov,
> starts Monday, March 22, 2021 to March 31, 2021 (inclusive).
Here's my small review of the lambda 2 library.
TLDR: ACCEPT
> Â Â - What is your evaluation of the design?
> Â Â - What is your evaluation of the implementation?
The library is a really small piece of code, like other reviewers
already noticed. During my tests, i came across two things that could
be improved:
* the internal macro BOOST_LAMBDA2_UNARY_LAMBDA and
BOOST_LAMBDA2_BINARY_LAMBDA #undefed at the end of the file. While i
undestand that it is an internal macro, it could be exposed to the user
to allow implementation for other operators
* that macro hardcode the std prefix, it could be part of its call, ie:
BOOST_LAMBDA2_BINARY_LAMBDA(+, std::plus) // instead of
BOOST_LAMBDA2_BINARY_LAMBDA(+, plus)
This would allow the user of the library to get the building blocks to
customize other operators to its needs (I have implemented unary
operator* easily that way).
> Â Â - What is your evaluation of the documentation?
The documentation is really good at telling what the library does, is
clean, nice looking.
> Â Â - What is your evaluation of the potential usefulness of the
> library?
At first glance, i was thinking that this was the kind of library that
i would never use. I find the count_if examples in the documentation
not really convincing.
Then, it happened that i made some experiments with c++20 ranges. And i
find that, in this context, shortening the lambdas gives a real benefit
to the code. It basically started from the need to feed a function that
was needing a list of non owner references to T from a
vector<unique_ptr<T>>. As I was not satisfied with the solution, I
checked whether c++20 ranges could provide an elegant solution to this.
Something like
f(std::views::transform(vec, deref));
implementing deref is easy. Then, out of curiosity, i checked if i
could use Peter's library to be able to write something like:
f(std::views::transform(vec, *_1));
I actually find that this way of writing is more natural than the
former.
So, i wrote :
namespace std // BAD, don't do that
{
template<typename T = void>
struct deref
{
decltype(auto) operator()(T& v) { return *v; }
};
template<>
struct deref<void>
{
template<typename T>
decltype(auto) operator()(T& v) { return *v; }
};
}
BOOST_LAMBDA2_UNARY_LAMBDA(*, deref);
modified Peter's library to remove the #undef, and voila, it just
works.
I see a lot of potential for the short writing form for lambdas, in
conjunction with the ranges library. I find convenient to write:
f(std::views::transform(vec, _1 + 1)) // convert from 0-based to 1-
based index
> Â Â - Did you try to use the library? With which compiler(s)? Did you
> Â Â Â Â have any problems?
No specific issue encountered, with gcc-10. Error messages are a bit
cryptic.
> Â Â - How much effort did you put into your evaluation? A glance? A
> quick
> Â Â Â Â reading? In-depth study?
A few hours playing with the library.
> Â Â - Are you knowledgeable about the problem domain?
Not at all.
My feeling is the following. At first glance, i was wondering what was
the use case the library addressed. After playing a bit with it, i'm
now convinced that people will want to use it, and, like me, will
probably want to expand it to fit their need. I was not suspecting that
it was possible to write code that way. I suspect most developers are
not either. Now that it is known it's possible, people will come up
with their own, probably inferior, solution. Having such a feature in
boost would at least provide everyone with a correct implementation of
it. Because people will want to write code like this, and i believe,
especially with ranges.
Thanks to Peter for writing this library, and proposing it to boost.
Reviewing it has been very interesting and instructive.
Regards,
Julien