Re: [boost] [Fit]Â formal review starts today

Subject: Re: [boost] [Fit] formal review starts today
From: Paul Fultz II (pfultz2_at_[hidden])
Date: 2016-03-05 18:05:01

On Saturday, March 5, 2016 at 4:43:45 PM UTC-6, Vicente J. Botet Escriba 
wrote:
>
> Le 05/03/2016 17:11, Paul Fultz II a écrit :
> >
> >
> > On Saturday, March 5, 2016 at 6:40:49 AM UTC-6, Vicente J. Botet 
> > Escriba wrote:
> >
> >     Le 05/03/2016 13:22, Peter Dimov a écrit :
> >     > Rob Stewart wrote:
> >     >> On March 4, 2016 3:27:08 PM EST, Paul Fultz II
> >     <pfu..._at_[hidden] <javascript:>>
> >     >> wrote:
> >     >> >
> >     >> > The library could switch to using `Callable`, if everyone feels
> >     >> that is > better. However, I honestly prefer to make mutable
> >     >> explicit. I would > like to hear feedback from other reviewers
> >     as well.
> >     >
> >     > I do not object to your requiring ConstCallable. I made
> >     > boost/std::bind const-neutral because this felt the right thing
> >     to do
> >     > - it's not bind's job to tell you what your function object
> >     ought to
> >     > do, it just reflects whatever it does. At the same time, stateful
> >     > function objects do have limited applicability outside of for_each
> >     > which specifically guarantees order and returns the function
> >     object.
> >     > You could pass bind( ++_1, 0 ) to generate_n and that's pretty
> >     much it.
> >     >
> >     > So I do not see the requirement as a defect warranting
> >     rejection. If
> >     > users demand Callable support, you'll add it. If not, not.
> >     >
> >     >> If your library only requires Callable, then users can supply a
> >     >> Callable or a ConstCallable. When you require a ConstCallable,
> >     they
> >     >> cannot supply a Callable. There may be valid reasons for that
> >     >> restriction in certain cases, but they should be documented rather
> >     >> than just imposed as a convention.
> >     >
> >     > That's true in principle. In practice though function objects
> >     that are
> >     > not ConstCallable are in 90% of the cases the result of a forgotten
> >     > 'const' on operator() and not a deliberate decision. So requiring
> >     > ConstCallable catches errors.
> >     >
> >     For me the question is if the functions provided by the library need
> >     that the function object must be const or not.
> >
> > They do not need it. However, there can be some unexpected behavior 
> > when the
> > user uses mutable function objects, because the functions can be copied 
> or
> > moved multiple times.
> Copied or moved? 

I believe the issue is mainly with copying.
 

> Does the library accepts move only function objects?
>

Yes it does.
 

> Do you have an example of unexpected behavior?

This:

struct counter
{
    int i;
    counter() : i(0)
    {}

    template<class... Ts>
    int operator()(Ts&&...)
    {
        return i++;
    }
};

counter c{};
by(mutable_(c))(1,1);
// Prints 0
std::cout << c.i << std::endl;

Instead `std::ref` should be used:

counter c{};
by(std::ref(c))(1,1);
// Prints 2
std::cout << c.i << std::endl;