On Friday, March 4, 2016 at 5:00:32 PM UTC-6, Steven Watanabe wrote:
AMDG
On 03/03/2016 07:16 PM, paul Fultz wrote:
<snip>
- 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
easily
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 how the algorithm decided to copy the function. This becomes even more apparent when people start using function adaptors as well. 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 function.
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?
<snip>
Details:
<snip> - [ test_all r ] "r" is completely meaningless here.
That is the way it was from where I copied it.
Then your source is probably also wrong. All it does is to pass the string "r" as an argument to test_all, but since test_all doesn't use any arguments, it has no effect.
- Actually, everything from rule test-all down can be reduced to: for local fileb in [ glob *.cpp ] { run $(fileb) ; } You don't need to create a special target to group the tests, unless you want more granularity than "run everything" or "run specific tests by name."
Interesting. I really could find no reference on this information.
run etc. are documented here: http://www.boost.org/build/doc/html/bbv2/builtins/testing.html test-suite is at the bottom of: http://www.boost.org/build/doc/html/bbv2/reference/rules.html
Once upon a time, I believe that test-suite was actually necessary. It has survived even to the present day, because most people create Jamfiles by copy-pasting existing Jamfiles.
<snip>
apply_eval.hpp:
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 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.
<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.
arg.hpp:
29: /// template<class IntegralConstant> /// constexpr auto arg(IntegralConstant); /// /// template<std::size_t N, class... Ts> /// constexpr auto arg_c(Ts&&...); arg and argc should do the same thing. It looks like arg creates a function object, while arg_c evaluates it directly.
Yes, this because you write `arg(int_<5>)(...)` and `arg_c<5>(...)`. In the future, I may make this a template variable(so `arg_c<5>` will be a function object).
Making arg_c a variable template would answer my concerns.
<snip>
by.hpp:
22: "Also, if just a projection is given, then the projection will be called for each of its arguments." I don't see any good reason for this to be an overload of by. It's essentially a for_each.
It seems like a natural extension:
by(p, f)(xs...) == f(p(xs)...) by(p)(xs...) == p(xs)...;
Sure they're similar, but the name "by" makes no sense for the latter.
This is a lot of code to implement what is essentially a one-liner: apply_eval(f, capture_forward(x)(p)...)
It does that for older compilers, but on compilers that support it, it uses initializers lists.
My problem is that apply_eval already does all the necessary #ifdefing. Scattering #ifdefs around just makes the code unmaintainable.
135: #define BOOST_FIT_BY_VOID_RETURN BOOST_FIT_ALWAYS_VOID_RETURN 138: #define BOOST_FIT_BY_VOID_RETURN boost::fit::detail::swallow You should really have a single BOOST_FIT_VOID_RETURN and use it consistently everywhere.
The BOOST_FIT_BY_VOID_RETURN is only used in one place so it could be replaced with preprocessor ifs.
My point is that having multiple void_ types means that you have to keep track of which void_ you need depending on which other functions you use as implementation details.
I can move to changing this to one void type.
capture.hpp:
32: // Capture lvalues by reference and rvalues by value. The default should be to capture everything by value. Capturing by reference is potentially dangerous, and switching based on whether the argument is an lvalue is even more so.
I don't see how it makes it more dangerous. Everything in scope is captured by reference and temporaries have there lifetimes extened so they will remain in scope as well. This works great when forwarding is added on.
In general a function that accepts both lvalues and rvalues should have the same behavior for both, modulo optimizations. Anything that doesn't is just asking for trouble.
In particular, I claim that if the function g returns a temporary, then the following transformation should either be valid and should leave the behavior unchanged, or it should fail to compile: f(g()); => auto x = g(); f(x);
I don't see how that prevents that.
79: return always_ref(*this)(xs...); Why not return *this? The always_ref dance serves no purpose, other than making the code hard to follow.
This is necessary because of the eagerness of constexpr.
I changed it and your tests still passed. I suspect that the problem that required this workaround was when you used BOOST_FIT_RETURNS or otherwise used this construct in the function prototype. There shouldn't be any problems when it only appears in the function body, as it does here.
There are probably some places where this can removed. I mostly like did it that way for consistency.
placeholders.hpp:
We really don't need another lambda library, let alone a half-baked one.
Its not designed to be a full lambda library. Just enough to handle some constexpr cases.
Exactly my point.
In Christ, Steven Watanabe
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost