Subject: Re: [boost] [Fit]Â formal review starts today
From: Paul Fultz II (pfultz2_at_[hidden])
Date: 2016-03-05 19:05:03
On Saturday, March 5, 2016 at 5:07:58 PM UTC-6, Vicente J. Botet Escriba
wrote:
>
> 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<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_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?
That is part of this issue here: https://github.com/pfultz2/Fit/issues/113
> Vicente
>
> _______________________________________________
> Unsubscribe & other changes:
> http://listarchives.boost.org/mailman/listinfo.cgi/boost
>
>