Le 05/03/2016 16:48, Paul Fultz II a écrit :
On Saturday, March 5, 2016 at 2:36:28 AM UTC-6, Vicente J. Botet Escriba wrote:
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:
std::ref just stores a pointer to the object, whereas fit::mutable_ stores a copy of the object.
I see.
>> 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. > > >>>> <snip> >>>> 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 >> for >>> 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) >> then >>> `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. >>>> <snip> >>>> >>>> apply_eval.hpp: >>>> >>>> 18: "Each [`eval`](eval.md http://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 >> parameters >>> 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. > >>>> <snip> >>>> >>>> 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
>> 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_FIT_NO_CONSTEXPR_VOID 0? There is a line that needs to be removed in the source code, which defines it to 1, this disables the configuration of the macro based on the platform. I didn't realized that this line was in the source code. Essentially, what happens is that it falls back to using an alternative for `void` even when its supported on newer platforms. There is a test for `apply_eval` at line 15, that checks it being called with a function returning `void`.
So, essentially, the code pointed out by steven is never used by the library. However, once I remove the define, the tests will fail, and I will need to correct the problem.
Please, could you add an issue, if not already done? Vicente