[lambda2] Andrzej's review

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[lambda2] Andrzej's review

Boost - Dev mailing list
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;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
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.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
> 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`



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

smime.p7s (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
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
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
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 <[hidden email]>
napisał(a):

> 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
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
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.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
śr., 31 mar 2021 o 20:01 Peter Dimov via Boost <[hidden email]>
napisał(a):

> 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
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
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.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> 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?



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [lambda2] Andrzej's review

Boost - Dev mailing list
pt., 2 kwi 2021 o 18:28 Peter Dimov via Boost <[hidden email]>
napisał(a):

> > 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
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost