Subject: Re: [boost] [Fit]Â formal review starts today
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2016-03-05 03:36:02
Le 05/03/2016 01:05, Paul Fultz II a écrit :
> On Friday, March 4, 2016 at 5:00:32 PM UTC-6, Steven Watanabe wrote:
>> On 03/03/2016 07:16 PM, paul Fultz wrote:
>>>> - Some library functions require function objects
>>>> to have a const operator(), without any rhyme
>>>> or reason. e.g. apply requires Callable, but
>>>> apply_eval requires ConstCallable.
>>> This is partly because of the fold done for older compiler. I could
>>> lift this restriction. Almost all of the adaptors use `ConstCallable`.
>>>> It's especially
>>>> weird when a function (e.g. partial) requires
>>>> both ConstCallable, and MoveConstructible.
>>> Almost all of the function adaptor require that. Why is that weird?
>> The fact that you require MoveConstructible
>> implies that you're carrying around a copy.
> Yes that's correct.
>> All the standard library algorithms take function
>> objects by value, and do not constify them.
> Yep, and there's no guarantee how many times it gets copied in an algorithm.
> Which means using mutable objects could return different result depending
> the algorithm decided to copy the function. This becomes even more apparent
> when people start using function adaptors as well.
I would like to see an example where the problem is evident.
> Because of these issues I decided to make mutability explicit. So the user
> will need to write `std::ref` or `boost::fit::mutable_` to use a mutable
It is a good practice to document the motivation of your design and show
how the problem disappear.
BTW, how fit::mutable_ is different from std::ref?
>> std::not1/2 (and also the obsolete std::bind1st/2nd)
>> do require const like you do, but std::bind does not,
>> and std::function even has a const operator(),
>> without requiring the same from its argument.
> Yep, and std::bind suffers the same issue.
>>>> You should not be requiring
>>>> const at all, for anything. See [func.bind.bind]
>>>> for an interface that handles const correctly.
>>> I probably should add Q/A for this. I make mutability explicit because
>>> most cases `std::ref` should be used when using a mutable function
>> object. If
>>> its really necessary(for example working with a third-party library)
>>> `mutable_` adaptor can be used.
>> You don't get to make up your own rules
>> when you're dealing with long established
>> concepts like function objects.
> Whats so problematic with explicit mutability beyond that's not the way
> std::bind works?
There is no problem adding explicit mutability when it solves an problem.
>>>> 18: "Each [`eval`](eval.md) call is always ordered
>>>> from left-to-right."
>>>> It isn't clear to me why this guarantee is important.
>>>> Not to mention that the test case apply_eval.cpp
>>>> doesn't check it.
>>> That is the whole purpose of apply_eval, so the user can order the
>>> that are applied to a function.
>> Yes, but /why/? The name apply_eval
>> doesn't convey any notion of evaluation
>> order to me. It only matters for functions
>> with side effects, and I wouldn't normally
>> be using apply_eval on such functions in
>> the first place. The examples that you
>> provide don't depend on this, either.
> I probably should find a better example.
We will need this better example during the review to be able to say if
it is useful.
>>>> 113: x(apply(f, BOOST_FIT_FORWARD(Ts)(xs)...), 0)
>>>> You need parens around the comma operator.
>>>> Also not tested.
>>> For what?
>> In context:
>> int x;
>> template<class F, class... Ts>
>> constexpr eval_helper(const F& f, Ts&&... xs) : x(apply(f,
>> BOOST_FIT_FORWARD(Ts)(xs)...), 0)
>> This constructor argument of x is essentially like:
>> int x(1, 3);
>> which is ill-formed.
>> I'm presuming that you intended to use the
>> comma operator, like so:
>> x((apply(...), 0))
>> You didn't detect the problem because
>> this template is never instantiated by
>> your tests.
> The tests don't run this because of `BOOST_FIT_NO_CONSTEXPR_VOID` is defined
> to 1, for all platforms. However, there are tests for this.
Please, could you point to them?
Could you add an issue to tract the configuration of a test with
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk