[lambda2] Andrzej's review
Hi Everyone, I recommend to REJECT Boost.Lambda2 library in its current form. My opinion is not very strong, and it is based on the perceived trade-off between usefulness and technical problems. I acknowledge that there are practical use cases that the library can address, that are not provided in documentation. (I recommend that examples with pointers, and maybe a complex one with column-oriented design be added). I consider the ADL issue that Barry pointed out a serious one. This will be an additional place where users would suddenly be caught by a magical problem that they will spend hours understanding. I think that the main problem here is that Boost.Lambda2 defines the operators that should belong to namespace std::placeholders. I also note that if this was added to the Standard Library, the problem would be nonexistent, because they would reside in namespace std. Could I propose an alternative design. Add placeholders _1, _2, ... into namespace `boost::lambda2::placeholders` specialize std::is_placeholder` for them, so that `std::bind` can recognize them. Add operators in the same namespace. Then, recommend the following usage of the library: ``` using namespace boost::lambda2::placeholders; void fun() { auto pred = std::bind(_1, &Employee::name) == std::bind(_2, &Employee::name); } ``` This single using directive would make both `std::bind` and Boost.Lambda2 operators work. I would find such a library acceptable. And in fact I could make it a condition for a conditional acceptance. Now, the standard questions:
- What is your evaluation of the design?
I object to the choice to add operators for placeholders from one namespace (std::placeholders) into another namespace (boost::lambda2). Save for that, I find anything else good. - What is your evaluation of the implementation?
Very sweet and clean. I only expect other operators to also work. But Peter already explained that this is his plan. - What is your evaluation of the documentation?
This library doesn't require much documentation of the interface. But I fill it is missing the Motivation section. I would expect to see practical examples: not long ones, but ones that I will find in real programs, like dealing with pointers. - What is your evaluation of the potential usefulness of the library?
Others have convinced me that it will find sufficient application in real programs. - Did you try to use the library? With which compiler(s)? Did you
have any problems?
I tried to reimplement it, and played with some toy examples in Wandbox. (newest GCC). It worked fine. - How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
I was able to reimplement it, read the entire implementation (which was not difficult: it is like seventy lines). I spent like three hours in total studying it. - Are you knowledgeable about the problem domain?
I used Boost.Lambda and Boost.Bind in the past. Finally, I would like to thank Peter for writing and sharing this library. Regards, &rzej;
Andrzej Krzemienski wrote:
Could I propose an alternative design. Add placeholders _1, _2, ... into namespace `boost::lambda2::placeholders` specialize std::is_placeholder` for them, so that `std::bind` can recognize them. Add operators in the same namespace.
Barry also suggested this - in fact he actually implemented it while using the library in his zip_view implementation, and I also considered it at the time. I wanted to reuse the standard placeholders and not introduce my own, but I'm not sure the lookup issue can be solved in any other way, so that's probably the way to go. Except, it will be just namespace boost::lambda2, because there's nothing else there, so there's no need to introduce another namespace. (The option of "illegally" defining the operators in namespace std::placeholders unfortunately doesn't work reliably either, as the std::bind return value is probably defined in std:: and not in std::placeholders, and in fact, even the placeholders may be of a type defined in std:: and not std::placeholders.) So yes, I think the only realistic way forward given the lookup issue is for Lambda2 to define its own placeholders in the same namespace as the operators.
So yes, I think the only realistic way forward given the lookup issue is for Lambda2 to define its own placeholders in the same namespace as the operators. Then you likely need to wrap the bind-expressions too, don't you? E.g. when `_1 + 1` works by using lamda2::_1, then you'll have the same issue for `(_1 + 1) + 1`
Alexander Grund wrote:
So yes, I think the only realistic way forward given the lookup issue is for Lambda2 to define its own placeholders in the same namespace as the operators. Then you likely need to wrap the bind-expressions too, don't you? E.g. when `_1 + 1` works by using lamda2::_1, then you'll have the same issue for `(_1 + 1) + 1`
It should work. The result of std::bind will encode in its type somewhere the type of _1, which will make boost::lambda2 an associated namespace. std::bind(f, 1) + 2 will not be associated, so it will have the problem in Barry's scenario, but there's nothing we can do about it.
Maybe Boost.Lambda2 could also provide its own thin wrapper for std::bind
which returns a type from namespace boost::lambda2 (for which std::
is_bind_expression
http://en.cppreference.com/w/cpp/utility/functional/is_bind_expression<T>
::value == true). This would move the solution still further from your
ideal, but would on the other hand make a coherent solution.
Anyway, at some point you may need to add something like
`boost::lambda::constant` to handle cases like `"TIME " + time + _1`.
Regards,
&rzej;
śr., 31 mar 2021 o 15:07 Peter Dimov via Boost
Alexander Grund wrote:
So yes, I think the only realistic way forward given the lookup issue is for Lambda2 to define its own placeholders in the same namespace as the operators. Then you likely need to wrap the bind-expressions too, don't you? E.g. when `_1 + 1` works by using lamda2::_1, then you'll have the same issue for `(_1 + 1) + 1`
It should work. The result of std::bind will encode in its type somewhere the type of _1, which will make boost::lambda2 an associated namespace.
std::bind(f, 1) + 2 will not be associated, so it will have the problem in Barry's scenario, but there's nothing we can do about it.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andrzej Krzemienski wrote:
Maybe Boost.Lambda2 could also provide its own thin wrapper for std::bind which returns a type from namespace boost::lambda2
I think (and hope) that this won't be necessary. By the way, one advantage of Lambda2 defining its own placeholders would be that they will now be usable as function objects. The need for just _1 or _2 as a lambda is surprisingly common and it's an annoyance that they don't work. I was going to add a "wrong" unary plus allowing +_1 to be usable as identity instead of _1, but now this hack won't be needed.
Anyway, at some point you may need to add something like `boost::lambda::constant` to handle cases like `"TIME " + time + _1`.
Yes, maybe. That's bind(identity(), x), and your point is probably that boost::lambda2 is not going to be an associated namespace of it. We'll see.
śr., 31 mar 2021 o 20:01 Peter Dimov via Boost
Andrzej Krzemienski wrote:
Maybe Boost.Lambda2 could also provide its own thin wrapper for std::bind which returns a type from namespace boost::lambda2
I think (and hope) that this won't be necessary.
By the way, one advantage of Lambda2 defining its own placeholders would be that they will now be usable as function objects. The need for just _1 or _2 as a lambda is surprisingly common and it's an annoyance that they don't work.
I was going to add a "wrong" unary plus allowing +_1 to be usable as identity instead of _1, but now this hack won't be needed.
So, you are saying the following: std::transform(v1.begin(), v1.end(), v2.begin(), out.begin(), _2); would mean select the second element? Yeah. This looks practical, and uniform with the rest of the library. Regards, &rzej;
Anyway, at some point you may need to add something like `boost::lambda::constant` to handle cases like `"TIME " + time + _1`.
Yes, maybe. That's bind(identity(), x), and your point is probably that boost::lambda2 is not going to be an associated namespace of it.
We'll see.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andrzej Krzemienski wrote:
So, you are saying the following:
std::transform(v1.begin(), v1.end(), v2.begin(), out.begin(), _2);
would mean select the second element? Yeah. This looks practical, and uniform with the rest of the library.
Yes. In this specific example this doesn't look very useful, but the ranges version std::ranges::transform( r1, r2, r2.begin(), _1 ); actually does something useful - it assigns the elements of r1 to r2, but if the sizes of r1 and r2 don't match, it doesn't overrun the destination.
Andrzej Krzemienski wrote:
Maybe Boost.Lambda2 could also provide its own thin wrapper for std::bind which returns a type from namespace boost::lambda2
I think (and hope) that this won't be necessary.
By the way, one advantage of Lambda2 defining its own placeholders would be that they will now be usable as function objects. The need for just _1 or _2 as a lambda is surprisingly common and it's an annoyance that they don't work.
Another advantage would be the ability to supply operator[], which must be a member. This will allow things like _1[x] or _1[0] < _1[1] to work. (Barry needed _1[x] for his zip_view implementation.) So, Andrzej, provided we have an agreement on the need for Lambda2 to provide its own placeholders instead of importing the standard ones, does this change your opinion on whether the library needs to be rejected?
pt., 2 kwi 2021 o 18:28 Peter Dimov via Boost
Andrzej Krzemienski wrote:
Maybe Boost.Lambda2 could also provide its own thin wrapper for std::bind which returns a type from namespace boost::lambda2
I think (and hope) that this won't be necessary.
By the way, one advantage of Lambda2 defining its own placeholders would be that they will now be usable as function objects. The need for just _1 or _2 as a lambda is surprisingly common and it's an annoyance that they don't work.
Another advantage would be the ability to supply operator[], which must be a member. This will allow things like _1[x] or _1[0] < _1[1] to work. (Barry needed _1[x] for his zip_view implementation.)
Sure. This will however set the expect\tion bar even higher, and now one can ask if you could provide the function call operator in the same way. But I guess this is impossible.
So, Andrzej, provided we have an agreement on the need for Lambda2 to provide its own placeholders instead of importing the standard ones, does this change your opinion on whether the library needs to be rejected?
Yes, it does. I change my recommendation to conditionally accept Boost.Lambda2; the condition being to provide its own placeholders. Regards, &rzej;
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
participants (3)
-
Alexander Grund -
Andrzej Krzemienski -
Peter Dimov