Fit review - viboes
Hi,
here it is my review.
I believe that Boost.Fit should be*accepted*.
*Design*
The design is sound and clean.
I have some concerns respect the the ConstFunctionObjects that I believe should merit an explanation on a rationale section. Why the library support only Const function objects?
Wondering if some parts should be moved to Boost.TypeTraits or reused from there (is_callable).
I'm missing the std::invoke function.
The_adaptor<> are not documented. The utility of those classes is not
documented. If the user can use those classes the documentaion must
document them. I guess they are useful to build other adaptors. It is
not clear whether the defined functions would use SFINAE or just fail
when used with types that don't respect the requirements. While I'm
generally for SFINAE, I understand that it will not be the case in order
to improve the compile time. It is not clear if the functions can be
final function objects or not.
*Implementation*
Not seen in depth.
It should use as much as possible Boost.Config and/or Boost.Predef
Possible files contains details that could be shared in Boost or used from other Boost libraries
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/move.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint)
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/forward.h...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/noexcept....
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/and.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint)
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/compresse...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/constexpr...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/intrinsic...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/pp.hpp
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/remove_rv...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/result_of...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/static_co...
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/using.hpp
It uses a lot of macros that make complex the understanding of what is behind. On the same file https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/compose.hpp and only on 169 lines
BOOST_FIT_INHERIT_CONSTRUCTOR, BOOST_FIT_RETURNS_CLASS,
BOOST_FIT_SFINAE_RESULT, BOOST_FIT_MANGLE_CAST,
BOOST_FIT_DECLARE_STATIC_VAR, BOOST_FIT_NOEXCEPT_CONSTRUCTIBLE,
BOOST_FIT_ENABLE_IF_CONSTRUCTIBLE, BOOST_FIT_JOIN *Documentation*
Quite good butthe description of the semantic of the provided functions would need
some extra wording. Maybe a section that states that the semantic
expression is part of the constraints on the types. It takes too much
pages to see what the library proposes.
For some functions we would need more information, examples. See below the detailed
*Potential usefulness of the library*
Very useful.
*Did you try to use the library? With which compiler(s)? Did you have any
problems?***
Not tried yet.
*How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?***
In-depth study of the documentation (about 5 h for this revision).
*Are you knowledgeable about the problem domain?*
In general, yes.
*Were the concerns from the March 2016 review of Fit addressed?***
I believe ;-)
Best,
Vicente
*Documentation:*
In general:
* Navigation only from the top of the page
* Hyper links missing.
* There is no link to real code from the examples.
In particular:
*http://pfultz2.github.io/Fit/doc/html/doc/src/gettingstarted.html*
An explanation of the BOOST_FIT_LIFT limitations would be more than welcome.
Typo: last right parenthesis in
// Pipable sum
BOOST_FIT_STATIC_LAMBDA_FUNCTION(sum) = pipable([](auto x, auto y)
{
return x + y;
});
It is not clear if the following is correct in the body of a function
autosum = pipable([](auto x, auto y)
{
return x + y;
};
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_print.html*
I would suggest to use an heterogeneous tuple in the for_each_tupleexample.
In addition use some kind of brackets to print the sequences and the
tuples would help to identify what is been printed.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_overloading.html*
Typo:
Before
// Check that T has member function for operator* and ope After // Check
that T has member function for operator* and opeerator->
I'm not sure the example is_derreferenceable is fair.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_polymorphic_constructo...
Maybe it is worth adding references to the proposals (P0318R0 and P0338R2)
*http://pfultz2.github.io/Fit/doc/html/doc/src/more_examples.html*
Extension Methods
It is not clear what numbers, filter and transform are in the example.
I believe that the range proposal woul include the pipe operator.
My question is how both pipe operator overloads interact.
http://pfultz2.github.io/Fit/doc/html/doc/src/point_free.html
typo
Before
|b(f)(x, y)|
|after|
||by(f)(x, y)||
*http://pfultz2.github.io/Fit/doc/html/doc/src/definitions.html*
It woul dbe great to have an example associated to each definition
Static Function Adaptor
I would say "It has an additional requirement that the
*class****function* is |DefaultConstructible|:"
Decorator
I would say "The *resulting* Function Adaptor
http://pfultz2.github.io/Fit/doc/html/doc/src/definitions.html#function-adap...
may be an unspecified or private type."
Typo?
Some parts of the documentation provides
*http://pfultz2.github.io/Fit/doc/html/doc/src/concepts.html*
ConstFunctionObject
I would like to note that this concept is not representable using the
Concept TS as it is quantifying universally on the Ts (args) parameters.
The same applies to UnaryFunctionObject, BinaryFunctionObject
Wondering if the documentation should use the term Concepts then.
The use of expresion "Is an object with ..." is confusing. What are we
qualifying the type or the instance?
Maybe it is worth defining FunctionObject.
EvaluatableFunctionObject
It is not clear why this kind of FunctionObjects is useful
Callable
I believe we have moved to call this Invocable in the standard. But
again, I believe that you want to quantify universally the arguments,
isn't it?
Does your definition of INVOKE differs from the one on the standard? If
yes, the differences should be mentioned. If not, the definition on the
standard should be referenced.
You use type T twice with different meaning
"The type |T| satisfies |Callable| if"
"if |f| is a pointer to member function of class |T|:"
I believe that here the T are not the same.
Are final function objects Callable, ConstCallable?
ConstCallable
The brief description is the same as the one for Callable
Is the definition of the second INVOKE different from the first one?
UnaryCallable
The name doesn't convey that it is *Const*Callable
Metafunction and MetafunctionClass
it is not clear what those are, there is no brief description. I guess
that this applies to f.
What do you mean by a type or a template?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/by.html*
It is not clear to me what
by(p)(xs...) == p(xs)... mean
An example of `fit::by` without a function argument will be welcome.
It is not clear how by_adaptor
On Sep 9, 2017, at 6:27 AM, Vicente J. Botet Escriba via Boost
wrote: Hi,
here it is my review.
I believe that Boost.Fit should be*accepted*.
Thanks for the review.
*Design*
The design is sound and clean.
I have some concerns respect the the ConstFunctionObjects that I believe should merit an explanation on a rationale section. Why the library support only Const function objects?
This is described in the FAQ, but maybe it should be moved to a rationale section. The main reasons are they are error prone, and in C++11 can’t be supported with constexpr.
Wondering if some parts should be moved to Boost.TypeTraits or reused from there (is_callable).
But would that imply a C++98 compatible trait? Of course, I don’t see a reason to dump every trait used in boost into Boost.TypeTraits.
I'm missing the std::invoke function.
What do you mean? std::invoke is provided by C++17. There is fit::apply, which will work like std::invoke except fit::apply also uses constexpr and is defined as a function object(so it can be passed to other functions).
The_adaptor<> are not documented.
Maybe in the definitions section I can go over how `foo(f)` is the same as `foo_adaptor<F>{f}`.
The utility of those classes is not documented.
The are mainly useful for when you want to type-based transformations to functions. This is common when you need to pass a function as a template parameter(like with the comparator in std::map, the deleter in std::unique_ptr, or when using Boost.MultiIndex).
If the user can use those classes the documentaion must document them. I guess they are useful to build other adaptors. It is not clear whether the defined functions would use SFINAE or just fail when used with types that don't respect the requirements. While I'm generally for SFINAE, I understand that it will not be the case in order to improve the compile time.
The goal is to be as transparent as possible. So if `foo(f)(x)` is equivalent to `f(x)`, then if `f(x)` causes substitution failure, then `foo(f)(x)` causes substitution failure. The same is true for constexpr and noexcept on compilers that support those well enough.
It is not clear if the functions can be final function objects or not.
They can be. There is no such restriction mentioned for the `Callable` or `ConstCallable` concept.
*Implementation*
Not seen in depth.
It should use as much as possible Boost.Config and/or Boost.Predef
Right now it uses sd-6 macros, but it could move to use Boost.Config(probably not Boost.Predef).
Possible files contains details that could be shared in Boost or used from other Boost libraries
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/move.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint) https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/forward.h... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/forward.h...
Well hana uses a static_cast. Here, I use a macro which uses a static_cast if possible, and then I fallback to a function on compilers that can’t handle the static_cast.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/noexcept.... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/and.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint)
Well, I dont know of a library in boost that implements the fast_and algorithm that works on gcc 4.6+ and msvc.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/compresse... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/compresse...
I dont know of a library that implements this. There is Boost.CompressedPair, but that implementation wouldn’t work at all for the Fit library.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/constexpr... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/intrinsic... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/intrinsic...
This probably could be factored out in a separate boost library, which might be useful for other heavy metaprogramming libraries.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/pp.hpp https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/remove_rv... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/result_of... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/static_co... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/using.hpp
It uses a lot of macros that make complex the understanding of what is behind. On the same file https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/compose.hpp and only on 169 lines BOOST_FIT_INHERIT_CONSTRUCTOR, BOOST_FIT_RETURNS_CLASS, BOOST_FIT_SFINAE_RESULT, BOOST_FIT_MANGLE_CAST, BOOST_FIT_DECLARE_STATIC_VAR, BOOST_FIT_NOEXCEPT_CONSTRUCTIBLE, BOOST_FIT_ENABLE_IF_CONSTRUCTIBLE, BOOST_FIT_JOIN *Documentation*
Some of those are documented. The others I could add some more comments in the code to what those do exactly.
Quite good butthe description of the semantic of the provided functions would need some extra wording. Maybe a section that states that the semantic expression is part of the constraints on the types.
What do you mean is part of the constraints on the type?
It takes too much pages to see what the library proposes. For some functions we would need more information, examples. See below the detailed
*Potential usefulness of the library*
Very useful.
*Did you try to use the library? With which compiler(s)? Did you have any problems?*** Not tried yet.
*How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?*** In-depth study of the documentation (about 5 h for this revision).
*Are you knowledgeable about the problem domain?*
In general, yes.
*Were the concerns from the March 2016 review of Fit addressed?*** I believe ;-)
Best, Vicente
*Documentation:*
In general:
* Navigation only from the top of the page
* Hyper links missing.
* There is no link to real code from the examples.
Most of the examples are a full running program that can be pasted in.
In particular:
*http://pfultz2.github.io/Fit/doc/html/doc/src/gettingstarted.html*
An explanation of the BOOST_FIT_LIFT limitations would be more than welcome.
Good point.
Typo: last right parenthesis in
// Pipable sum BOOST_FIT_STATIC_LAMBDA_FUNCTION(sum) = pipable([](auto x, auto y) { return x + y; });
It is not clear if the following is correct in the body of a function
autosum = pipable([](auto x, auto y) { return x + y; };
That needs to be corrected.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_print.html*
I would suggest to use an heterogeneous tuple in the for_each_tupleexample.
In addition use some kind of brackets to print the sequences and the tuples would help to identify what is been printed.
I was trying to keep the example simple. However, with the print out its a little more confusing.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_overloading.html*
Typo:
Before // Check that T has member function for operator* and ope After // Check that T has member function for operator* and opeerator->
I'm not sure the example is_derreferenceable is fair.
Using is_detected, might be a little simpler, but it don’t think it would help overall.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_polymorphic_constructo...
Maybe it is worth adding references to the proposals (P0318R0 and P0338R2)
*http://pfultz2.github.io/Fit/doc/html/doc/src/more_examples.html*
Extension Methods
It is not clear what numbers, filter and transform are in the example.
I should add an implementation of those.
I believe that the range proposal woul include the pipe operator.
Although, there are some who don’t see the usefulness of it.
My question is how both pipe operator overloads interact.
How so?
http://pfultz2.github.io/Fit/doc/html/doc/src/point_free.html
typo
Before
|b(f)(x, y)|
|after|
||by(f)(x, y)||
*http://pfultz2.github.io/Fit/doc/html/doc/src/definitions.html*
It woul dbe great to have an example associated to each definition
That is a good idea.
Static Function Adaptor
I would say "It has an additional requirement that the *class****function* is |DefaultConstructible|:"
Decorator I would say "The *resulting* Function Adaptor http://pfultz2.github.io/Fit/doc/html/doc/src/definitions.html#function-adap... may be an unspecified or private type.”
Ok.
Typo? Some parts of the documentation provides
*http://pfultz2.github.io/Fit/doc/html/doc/src/concepts.html* ConstFunctionObject
I would like to note that this concept is not representable using the Concept TS as it is quantifying universally on the Ts (args) parameters.
The same applies to UnaryFunctionObject, BinaryFunctionObject
Wondering if the documentation should use the term Concepts then.
The purpose of the Concepts section is to document the type requirements. The current standard may not be able to check those type requirements in its current form, but there could be future versions of C++ that can check these type requirements.
The use of expresion "Is an object with ..." is confusing. What are we qualifying the type or the instance?
This means the type is an object(ie scalar, array, union, or class). Of course, an array or union cannot have a call operator, so this only applies to classes and scalars that are pointers.
Maybe it is worth defining FunctionObject.
Trye, although I never use that concept, it would make sense for completeness sake.
EvaluatableFunctionObject
It is not clear why this kind of FunctionObjects is useful
This mainly used by the `fit::eval` and `fit::apply_eval` functions.
Callable
I believe we have moved to call this Invocable in the standard. But again, I believe that you want to quantify universally the arguments, isn't it?
That is possible to check with the current C++(hence `is_callable`). This should be renamed to `Invocalbe` and `is_invocable`.
Does your definition of INVOKE differs from the one on the standard?
No, it shouldn’t, unless the standard doesn’t allow constexpr.
If yes, the differences should be mentioned. If not, the definition on the standard should be referenced.
Would cppreference be acceptable?
You use type T twice with different meaning
"The type |T| satisfies |Callable| if"
"if |f| is a pointer to member function of class |T|:"
I believe that here the T are not the same.
True, let me fix that.
Are final function objects Callable, ConstCallable?
Yes, why wouldn’t they be?
ConstCallable The brief description is the same as the one for Callable Is the definition of the second INVOKE different from the first one?
No, it isn’t. The only difference is the f is of type `const T`.
UnaryCallable The name doesn't convey that it is *Const*Callable
Metafunction and MetafunctionClass it is not clear what those are, there is no brief description. I guess that this applies to f. What do you mean by a type or a template?
I see, that is a little confusing. These should follow the same definitions as in Boost.MPL.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/by.html*
It is not clear to me what
by(p)(xs...) == p(xs)... mean
An example of `fit::by` without a function argument will be welcome.
Good point.
It is not clear how by_adaptor
and by_adaptor<Projection> can be used by the user?
I could add an example using std::map.
Shouldn't those be hidden?
No they shouldn’t be hidden.
This is a genral remak, why F must be ConstCallable?
For the same reason everything is ConstCallable.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/compose.html *The requirements don't state clearly the constraints on the input parameters of f and the result of g, except by the semantic expression. This is a general remark. I suspect that you mean the semantic expression must be well formed. But, is this SFINAE friendly or is this a requirement?
It is sfinae-friendly.
Can both function be final function objects? ****
In compose? Any of the functions can be final.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/conditional.html* It is not clear what king of parameters this function could have (in particular in comparison with P0051). Does it accept function/data members pointers, final classes, references_wrappers, …?
Yes, it takes any `ConstCallable`
Typo "Proposal for C++ Proposal for" P0051 original proposal uses overload_linearly/overload (as Boost.Hana) instead of conditional/match. Wondering if those names aren't more clear.
I really hate the name overload_linearly. Its long and doesn’t describe what it does.
Why do we need to change the Boost.Hana names?
Those names were chose before Boost.Hana. I use `conditional` because it was like a conditional block, especially when used with fit::if_. Perhaps, there is a better name.
There is also std::conditional :(
Which really should’ve been called std::if_.
Can more than 2 parameters be final function objects?
Yes, of course.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/flip.html** * IIUC, F must not be BinaryCallable, but Callable with at least two arguments.
True, let me update the documentation.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/implicit.html*
Is implicit a StaticFunctionAdaptor?
Yes.
Shouldn't StaticFunctionAdaptors have Functions as parameters?
What do you mean?
I named the `auto_cast` function `explicitly` to do a explicit conversion.
Other uses of this pattern I've see are to create a buffer able to store a specific struct
struct S; S* ptr = create_msg();
create_msg returns an class that is convertible to a T* and that allocates sizeof(T) bytes of memory.
Ah yes, for that too.
Why this pattern respect the DRY principle (we don't repeat S), I believe it abuses of implicit conversions.
Well, there is a caveat when someone uses ``auto`, like in your example: auto ptr = create_msg(); There is a guard to prevent this, but it doesn’t work in C++17.
Anyway, I wanted just to share another use case for implicit.
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/indirect.html Wondering if it could be worth adding some preconditions as I guess that the function call
indirect(f)(xs...)
will be undefined if f is null
That is true if `f` is a pointer, but is not true for all dereference operators.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/infix.html** *
One of the asked infix operations is pow. However we would like to evaluate from right to left and have the higher precedence than * x pow ypow z===pow(x, pow(y,z)) x * ypow z===x * pow(y,z) x pow y* z===pow(x,y) * zWith the proposed infix notation this should be written x <pow> (y<pow> z)===x * pow(y,z) x * (y<pow> z)===x * pow(y,z) (x <pow> y)* z===pow(x,y) * z and the following would be surprising x <pow> y<pow> z===pow(pow(x, y, z) x * y<pow> z===pow(x * y, z) x <pow> y*z===pow(x, y * z) It would work better for swap or compare (proposed operator <=>) as these functions are not associative. I believe a note should prevent the users for such bad use cases, for which it is better to don't define the infix operator.
That is a good example to talk about. I will add that.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/lazy.html* Note p0356r1. It would be good to have a comparison in order either to improve the proposal or your library.
That seems more related to fit::partial.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/match.html* Same comments than for fit::conditional. What are ex typo "Proposal for C++ Proposal for" Can more than 2 parameters be final function objects? *http://pfultz2.github.io/Fit/doc/html/include/boost/fit/mutable.html *Example of bad usage would be welcome.**Why Fit uses always const? **Why not use a reference to the function object instead?
There is no way to safely manage the lifetime with a reference.
Is there a safe way to use MutableFunctionObjects with Fit?
Yes, you can use `std::ref.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/partial.html* Are there any constraints on the type of xs. Shouldn't them be MoveConstructibles?
That is true, since it decays the parameters.
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/pipable.html Are there any constraints on the type of x.
No.
Shouldn't it be MoveConstructible?
No, it is not necessary. Pipable does not capture the parameters by value, since it is almost always used in the same expression(ie `x | f(y)`).
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/protect.html It is not clear to me what this useful for. Could you elaborate?
When you want `bind` or `lazy` to capture a bind expression instead of evaluating it.
Is there a typo here assert(lazy(f)(protect(lazy(g)(_1)))() == f(lazy(g)(_1))) ^ Shouldn't it be assert(lazy(f)(protect(lazy(g)),_1)() == f(lazy(g)(_1)))
Nope, that is correct.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/result.html*** This adaptor converts explicitly the result of the call to the type. Have you considered `convert_to`?
It also adds a `result_type` to the function. Its mainly used to annotate function with the result type, which can be useful for fit::fix or Boost.Variant visitors. The documentation could make this clearer.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reveal.html*
as_failures, with_failures, .... /adaptors/ don't appear on the index.
You mean the table of contents? Yea, for them to appear there they need to be moved to a different section, with a header for each section.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reverse_fold.html*
reverse_fold is not right fold, as right fold is right associative, isn't it?
True, it is right fold with the arguments flipped.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/rotate.html*** Could you show a concrete use case?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/static.html***
What are the differences of this adaptor and the macro STATIC_FUNCTION?
STATIC_FUNCTION just declares a function objects at namespace or global scope, which requires the function to be constexpr constructible. The `static_` adaptor is to declare an adaptor that does not have a constexpr constructor.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack.html*
Why name it unpack instead of apply like in C++17?
First, it doesn’t have the same signature as std::apply. Secondly, its very confusing, as ‘apply' generally means function applications especially in boost. So I would like to avoid this confusion.
Need to introduce the Sequence concept and how it is customized?
Hmm that is probably a better way to define it.
unpack_sequence? How unpack_sequence is used by unpack?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack_sequence.html... Why unpack_sequence, and not the tuple-like traits?
Not every sequence could be defined with indices.
Is unpack_sequence defined if the class defines the tuple-like traits?
No because they are not sfinae-friendly.
Aside the example, it is not defined what the user must define.
That could probably use more explanation.
What the member function apply must return?
The result of calling `f`.
How the unpack_sequence parameter and the Sequence parameter of member function apply are related? I don't understand
template
constexpr static auto apply(F&& f, Sequence&& s) BOOST_FIT_RETURNS ( s(std::forward<F>(f)) ); Is a Sequence a callable taking a Callable or there is a type here? Is there a missing return? I suspect it should be
template
constexpr static auto apply(F&& f, Sequence&& s) BOOST_FIT_RETURNS ( std::forward<F>(f)(s0, ,, sk); ); If this is correct, does it means that unpack just forwards to unpack_sequence?
That example, I guess is a little confusing. As the `Sequence` is a function like fit::pack, but yes unpack calls unpack_sequence to do the unpacking for a single sequence.
Does the default specialization make use of INVOKE?
Its not needed, as the function passed to `unpack_sequence` will always have a call operator.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/capture.html*** Please, could you show how it is more flexible than lambda captures in C++?
You can’t capture by value or reference dependending if it is an lvalue when using lambda captures.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/if.html*
What happens when the parameter is false_type? SFINAE?
Essentially yes. There is no call operator when its false.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/limit.html*** if a function object is overloaded with 2 and 3 parameters, would limit be useful or it is only useful when the number of parameters is fixed?
Yes its still useful, as it only sets a max limit.
I believe this is answered in http://pfultz2.github.io/Fit/doc/html/doc/src/partialfunctions.html, however I believe it merits a clarification here.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/repeat.html*
Add requirement on the function parameter that must be a Unary function such that f(f(x)) is well formed.
True, but the first call to repeat doesn’t need to be unary.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/repeat_while.html* The same here.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/arg.html*** What is the rational for the choice of been 1-based?
This is to match how placeholders work. So arg<1> and _1 are the same number.
Could you add a rationale in the doc?
Yea.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/construct.html*
Maybe add a reference to the factory proposal p03382.
Do you have a link?
What happens when the parameter is not a T is not constructible from the params? SFINAE?
Yes.
What happens when the parameter is not a MetafunctionClass? SFINAE?
Yes.
I believe Boost.Hana uses make instead of construct.
That is a possible name.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/decay.html*
Why not decay_copy?
It doesn’t always copy, like when passing reference_wrapper.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/identity.html *is identity a function or a function object? do we need identity()(x)?
It can be treated as a function. I should update the synopsis to use a function signature instead.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html*** Wondering if it is not better to locate them in a placeholders namespace, so that we can avoid possible collisions with other placeholders.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un...
The same here
Ah, it could cause collision if users do `using boost::fit`.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/function_param_limit... |function_param_limit is not a Metafunction. "|The|function_param_limit| metafunction …"
It is a metafunction(doing ::type works), but its actually more like a type trait.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/is_callable.html*** F must be Callable can not be a requirement ;-)
Add reference to C++17 trait is_invocable.
Wondering if we should have it in type_traits, if not already there.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/apply.html*** Confusion with std::apply. is this related to std::invoke?
Its not like std::apply, its like hana::apply. Of course, maybe I could rename this to fit::invoke, and then add a fit::apply, that only calls the call operator.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/function.html*
"By default, all functions defined with|BOOST_FIT_STATIC_FUNCTION| use thereveal http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reveal.html adaptor to improve error messages."
How to change this default?
There isn’t a way. Perhaps I could add PP define to change it, but I wonder if its better to have some way to change it on a per-variable basis.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/lift.html*** WhyBOOST_FIT_LIFT_CLASSdoesn't defines a instance as do BOOST_FIT_STATIC_FUNCTION?
That maybe a better approach. Maybe I can call it BOOST_FIT_LIFT_FUNCTION.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/pack.html*** How this is related with fit::capture?
fit::capture uses fit::pack, but also does a pack_join to combine the pack and the parameters together.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/tap.html*
How this is related to the tee unix command?
I am not that familiar with unix tee.
*http://pfultz2.github.io/Fit/doc/html/doc/src/configurations.html***Which ones?
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
First sorry for the formatting of my original post. Le 09/09/2017 à 22:22, P F a écrit :
On Sep 9, 2017, at 6:27 AM, Vicente J. Botet Escriba via Boost
mailto:boost@lists.boost.org> wrote: *Design* The design is sound and clean.
I have some concerns respect the the ConstFunctionObjects that I believe should merit an explanation on a rationale section. Why the library support only Const function objects?
This is described in the FAQ, but maybe it should be moved to a rationale section. The main reasons are they are error prone, and in C++11 can’t be supported with constexpr.
Sorry I missed this section :(
Wondering if some parts should be moved to Boost.TypeTraits or reused from there (is_callable).
But would that imply a C++98 compatible trait? Of course, I don’t see a reason to dump every trait used in boost into Boost.TypeTraits.
I'm not talking about any traits. is_callable is special in the sense there is an equivalent in the C++ standard (is_invocable). I don't think we should require that all the traits in Boost.Traits should be compatible C++98 when we are already in C++17.
I'm missing the std::invoke function.
What do you mean? std::invoke is provided by C++17. There is fit::apply, which will work like std::invoke except fit::apply also uses constexpr and is defined as a function object(so it can be passed to other functions).
Why not call it invoke?
The_adaptor<> are not documented.
Maybe in the definitions section I can go over how `foo(f)` is the same as `foo_adaptor<F>{f}`.
This will be useful.
The utility of those classes is not documented.
The are mainly useful for when you want to type-based transformations to functions. This is common when you need to pass a function as a template parameter(like with the comparator in std::map, the deleter in std::unique_ptr, or when using Boost.MultiIndex).
An example will be welcome in the documentation.
If the user can use those classes the documentaion must document them. I guess they are useful to build other adaptors. It is not clear whether the defined functions would use SFINAE or just fail when used with types that don't respect the requirements. While I'm generally for SFINAE, I understand that it will not be the case in order to improve the compile time.
The goal is to be as transparent as possible. So if `foo(f)(x)` is equivalent to `f(x)`, then if `f(x)` causes substitution failure, then `foo(f)(x)` causes substitution failure. The same is true for constexpr and noexcept on compilers that support those well enough.
E.g. assert(compose(f, g)(xs...) == f(g(xs...))); If f and g don't compose, would compose(f, g) SFINAE the operator() or just compile fail. If it SFINAE, I could use compose(f, g) to check if f and g are composable.
It is not clear if the functions can be final function objects or not.
They can be. There is no such restriction mentioned for the `Callable` or `ConstCallable` concept.
Oh, I see it now. I believe there is however an issue when you have more than one final function objet. Do you have a test with two function objects? If I remember the issue was because we can not inherit from two wrapper classes doing perfect forwarding. If you don't have the issue, I will base my implementation of the proposed overload on your implementation ;-)
*Implementation*
Not seen in depth.
It should use as much as possible Boost.Config and/or Boost.Predef
Right now it uses sd-6 macros, but it could move to use Boost.Config(probably not Boost.Predef).
Possible files contains details that could be shared in Boost or used from other Boost libraries
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/move.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint) https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/forward.h...
Well hana uses a static_cast. Here, I use a macro which uses a static_cast if possible, and then I fallback to a function on compilers that can’t handle the static_cast.
What I mean is that IMHO Boost should solve these issues in a single way. If we don't want to use directly e.g. <utility> because it is heavy, we should provide the lighter components in Boost. I don't believe it is a good approach to redefine them in each library that needs such basic facilities..
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/noexcept.... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/and.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint)
Well, I dont know of a library in boost that implements the fast_and algorithm that works on gcc 4.6+ and msvc.
We have std::conditional. If we have a better option (for some compiler version) this should go to the Core library as any library could profit from it. This is a basic feature.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/compresse...
I dont know of a library that implements this. There is Boost.CompressedPair, but that implementation wouldn’t work at all for the Fit library.
My comment included shared. If your compressed pair is better, we should improve Boost.CompressedPair.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/constexpr... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/intrinsic...
This probably could be factored out in a separate boost library, which might be useful for other heavy metaprogramming libraries.
This is my point.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/pp.hpp https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/remove_rv... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/result_of... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/static_co... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/using.hpp
It uses a lot of macros that make complex the understanding of what is behind. On the same file https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/compose.hpp and only on 169 lines BOOST_FIT_INHERIT_CONSTRUCTOR, BOOST_FIT_RETURNS_CLASS, BOOST_FIT_SFINAE_RESULT, BOOST_FIT_MANGLE_CAST, BOOST_FIT_DECLARE_STATIC_VAR, BOOST_FIT_NOEXCEPT_CONSTRUCTIBLE, BOOST_FIT_ENABLE_IF_CONSTRUCTIBLE, BOOST_FIT_JOIN
*Documentation*
Some of those are documented. The others I could add some more comments in the code to what those do exactly.
Yes this would help to understand what the code is doing in detail. I understand that you want to follow DRY principle, but this makes the code more difficult to understand until we are familiar with the macros.
Quite good butthe description of the semantic of the provided functions would need some extra wording. Maybe a section that states that the semantic expression is part of the constraints on the types.
What do you mean is part of the constraints on the type?
Usually we describe the expressions that the types must support on the requirements. You use a semantic clause as requirement. What I mean is that the semantic clause implies the expression must be well formed and so is a requirement on the concerned types. That is the semantic expression is used to define what the function does and is also contains constraints of a not named Concept. What I would like is split the requirements and what the function does.
*Documentation:*
In general:
* Navigation only from the top of the page
* Hyper links missing.
* There is no link to real code from the examples.
Most of the examples are a full running program that can be pasted in.
Humm, I don't see the include on the code fragments, the needed using namespace, ...
In particular:
*http://pfultz2.github.io/Fit/doc/html/doc/src/gettingstarted.html*
An explanation of the BOOST_FIT_LIFT limitations would be more than welcome.
Good point.
Could you tell us here what they are? in detail?
Typo: last right parenthesis in
// Pipable sum BOOST_FIT_STATIC_LAMBDA_FUNCTION(sum) = pipable([](auto x, auto y) { return x + y; });
It is not clear if the following is correct in the body of a function
auto sum = pipable([](auto x, auto y) { return x + y; };
That needs to be corrected.
Could you confirm that the previous code without using the BOOST_FIT_STATIC_FUNCTION is valid? If yes I believe that the documentation could use this form as far as it is inside the body of a function.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_print.html*
I would suggest to use an heterogeneous tuple in the for_each_tupleexample.
In addition use some kind of brackets to print the sequences and the tuples would help to identify what is been printed.
I was trying to keep the example simple. However, with the print out its a little more confusing.
This is my point.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_overloading.html*
Typo:
Before // Check that T has member function for operator* and ope
After // Check that T has member function for operator* and operator->
Note the typo above.
*http://pfultz2.github.io/Fit/doc/html/doc/src/more_examples.html*
My question is how both pipe operator overloads interact.
How so?
My bad. Each overload applies to different types, so that there shouldn't be no conflict. BTW, is operator|() defined only for pipable_adaptor<F>? Could the user define a function object that is a pipable_adaptor for your library and it is also a pipable_xxx for the range library?
Static Function Adaptor
I would say "It has an additional requirement that the *class****function* is |DefaultConstructible|:"
Don't forget this one.
*http://pfultz2.github.io/Fit/doc/html/doc/src/concepts.html* ConstFunctionObject
I would like to note that this concept is not representable using the Concept TS as it is quantifying universally on the Ts (args) parameters.
The same applies to UnaryFunctionObject, BinaryFunctionObject
Wondering if the documentation should use the term Concepts then.
The purpose of the Concepts section is to document the type requirements. The current standard may not be able to check those type requirements in its current form, but there could be future versions of C++ that can check these type requirements.
No, they couldn't if they are based on the Concept TS. The Concept TS doesn't have universal quantification, and I believe it wouldn't never had it from what the authors think about this feature.
The use of expresion "Is an object with ..." is confusing. What are we qualifying the type or the instance?
This means the type is an object(ie scalar, array, union, or class). Of course, an array or union cannot have a call operator, so this only applies to classes and scalars that are pointers.
I believe it is better to use the term "type" here.
EvaluatableFunctionObject
It is not clear why this kind of FunctionObjects is useful
This mainly used by the `fit::eval` and `fit::apply_eval` functions.
I mean, it is not clear in this section, and some clarification is needed here.
Callable
I believe we have moved to call this Invocable in the standard. But again, I believe that you want to quantify universally the arguments, isn't it?
That is possible to check with the current C++(hence `is_callable`). This should be renamed to `Invocalbe` and `is_invocable`.
Well is_invocable requires to know the type of the arguments. I believe that your Callable doesn't require to know with with types the function can be called. Could you confirm? Anyway, as the standard uses is_invocable and the concept is based on INVOKE you should use is_invocable.
Does your definition of INVOKE differs from the one on the standard?
No, it shouldn’t, unless the standard doesn’t allow constexpr.
If yes, the differences should be mentioned. If not, the definition on the standard should be referenced.
Would cppreference be acceptable?
I would reference both then.
You use type T twice with different meaning
"The type |T| satisfies |Callable| if"
"if |f| is a pointer to member function of class |T|:"
I believe that here the T are not the same.
True, let me fix that.
cppreference has the same error :(
Are final function objects Callable, ConstCallable?
Yes, why wouldn’t they be?
Ok. I missed that.
ConstCallable The brief description is the same as the one for Callable Is the definition of the second INVOKE different from the first one?
No, it isn’t. The only difference is the f is of type `const T`.
I believe it is not worth duplicating the description for this difference. Just say reference the standard and say INVOKE(...) is valid with a const T object or a non-const object.
UnaryCallable The name doesn't convey that it is *Const*Callable
Any concern here?
Metafunction and MetafunctionClass it is not clear what those are, there is no brief description. I guess that this applies to f. What do you mean by a type or a template?
I see, that is a little confusing. These should follow the same definitions as in Boost.MPL.
Wondering if it isn't better to reference (and copy if you want the definition)
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/by.html*
It is not clear to me what
by(p)(xs...) == p(xs)... mean
An example of `fit::by` without a function argument will be welcome.
Good point.
It is not clear how by_adaptor
and by_adaptor<Projection> can be used by the user? I could add an example using std::map.
We should add some real example as motivation. Maybe not here, in the tutorial.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/compose.html *The requirements don't state clearly the constraints on the input parameters of f and the result of g, except by the semantic expression. This is a general remark. I suspect that you mean the semantic expression must be well formed. But, is this SFINAE friendly or is this a requirement?
It is sfinae-friendly.
Could you add it to the reference documentation. This is an important feature. In the standard they use "This function should not participate in overload resolution until ... or something like that.
Can both function be final function objects? ****
In compose? Any of the functions can be final.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/conditional.html* It is not clear what king of parameters this function could have (in particular in comparison with P0051). Does it accept function/data members pointers, final classes, references_wrappers, …?
Yes, it takes any `ConstCallable`
Sorry, I missed the specific nature of ConstCallable.
Typo "Proposal for C++ Proposal for" P0051 original proposal uses overload_linearly/overload (as Boost.Hana) instead of conditional/match. Wondering if those names aren't more clear.
I really hate the name overload_linearly. Its long and doesn’t describe what it does.
I don't find that conditional/match describe what they do. This is the eternal naming issue :)
Why do we need to change the Boost.Hana names?
Those names were chose before Boost.Hana. I use `conditional` because it was like a conditional block, especially when used with fit::if_. Perhaps, there is a better name.
Sure. I don't think you should use conditional, as we have std::conditional, we a really different meaning.
There is also std::conditional :(
Which really should’ve been called std::if_.
Too late. The standard committee doesn't like names as if_, mutable_, int_ :(
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/implicit.html*
Is implicit a StaticFunctionAdaptor?
Yes.
Shouldn't StaticFunctionAdaptors have Functions as parameters?
What do you mean?
That implicit has a template, not a class :(
Why this pattern respect the DRY principle (we don't repeat S), I believe it abuses of implicit conversions.
Well, there is a caveat when someone uses ``auto`, like in your example:
auto ptr = create_msg();
yes, this is the caveat of returning a proxy that is convertible to anything. Could you warm the user in the documentation of this possible bad usage?
There is a guard to prevent this, but it doesn’t work in C++17.
Could you elaborate?
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/indirect.html Wondering if it could be worth adding some preconditions as I guess that the function call
indirect(f)(xs...)
will be undefined if f is null
That is true if `f` is a pointer, but is not true for all dereference operators.
For example?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/lazy.html* Note p0356r1. It would be good to have a comparison in order either to improve the proposal or your library.
That seems more related to fit::partial.
Correct.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/match.html* Same comments than for fit::conditional. What are ex typo "Proposal for C++ Proposal for" Can more than 2 parameters be final function objects?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/mutable.html *Example of bad usage would be welcome.**Why Fit uses always const? **Why not use a reference to the function object instead?
There is no way to safely manage the lifetime with a reference. I was talking of reference_wrapper.
IIUC, mutable should be used *only* to wrap existing functions that where no const but that don't modify the object (but it is a3rd party code and cannot be changed). I'm wrong?
Is there a safe way to use MutableFunctionObjects with Fit?
Yes, you can use `std::ref.
But reference_wrapper is a ConstFunctionObject. I mean when mutable_ is safe? The 3rd party use case? Do you have a good example of MutableFunctionObject that is safe? After all I wonder if Mutable is the correct name. In C++ mutable means modifiable by a const function. I'll suggest NonConst as the function applicable to non-const objects.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/partial.html* Are there any constraints on the type of xs. Shouldn't them be MoveConstructibles?
That is true, since it decays the parameters.
This kind of information is useful.
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/pipable.html.
Shouldn't it be MoveConstructible?
No, it is not necessary. Pipable does not capture the parameters by value, since it is almost always used in the same expression(ie `x | f(y)`).
Ok, I see.
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/protect.html It is not clear to me what this useful for. Could you elaborate?
When you want `bind` or `lazy` to capture a bind expression instead of evaluating it.
and ... could you elaborate? Could you show how we can do it without fit::protect and with?
Is there a typo here assert(lazy(f)(protect(lazy(g)(_1)))() == f(lazy(g)(_1))) ^ Shouldn't it be assert(lazy(f)(protect(lazy(g)),_1)() == f(lazy(g)(_1)))
Nope, that is correct.
It is incoherent with the example. So maybe the example is wrong auto lazy_apply = lazy(apply)(protect(lazy_id), _1);` Or I'm missing something
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/result.html*** This adaptor converts explicitly the result of the call to the type. Have you considered `convert_to`?
It also adds a `result_type` to the function. Its mainly used to annotate function with the result type, which can be useful for fit::fix or Boost.Variant visitors. The documentation could make this clearer.
Yes, I know that knowing the result type allows to optimize visitors. What is the main intent? to force the conversion or to have a way to retrieve the result type of a function object? The name should convey the main intent. We have std::result_of until C++14 and invoke_result since C++17 to get the result type.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reveal.html*
as_failures, with_failures, .... /adaptors/ don't appear on the index.
You mean the table of contents? Yea, for them to appear there they need to be moved to a different section, with a header for each section.
You have items in the index that don't have his own file, e.g. http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un..., so you should be able to add to it anything without needed a file, isn't it?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reverse_fold.html*
reverse_fold is not right fold, as right fold is right associative, isn't it?
True, it is right fold with the arguments flipped.
right fold don't flip the argument ;-) This is way reverse_fold is a good name, otherwise it should be named fold_right
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/rotate.html*** Could you show a concrete use case?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/static.html***
What are the differences of this adaptor and the macro STATIC_FUNCTION?
STATIC_FUNCTION just declares a function objects at namespace or global scope, which requires the function to be constexpr constructible. The `static_` adaptor is to declare an adaptor that does not have a constexpr constructor.
It is weird the example don't show the specific case as it uses static_ at global global scope. Could you show another example that is not at the namesapce o global scope. Why static_ couldn't declare its default constructor constexpr? I believe the description is on the other sense, which is why it is confusing. static_ requires a default constructor but don't requires the constructor to be constexpr and don't provides one constexpr default constructor, but I suspect that if the default constructor is constexpr it should work also. IIUC, you are saying that if it is the case then it is better to use the macro BOOST_FIT_STATIC_FUNCTION.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack.html*
Why name it unpack instead of apply like in C++17?
First, it doesn’t have the same signature as std::apply.
Because you curry the function?
Secondly, its very confusing, as ‘apply' generally means function applications especially in boost. So I would like to avoid this confusion.
We are doing a function application here, isn't it?
Need to introduce the Sequence concept and how it is customized?
Hmm that is probably a better way to define it.
unpack_sequence? How unpack_sequence is used by unpack?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack_sequence.html... Why unpack_sequence, and not the tuple-like traits?
Not every sequence could be defined with indices.
For example?
Is unpack_sequence defined if the class defines the tuple-like traits?
No because they are not sfinae-friendly.
Why? Could you elaborate?
Aside the example, it is not defined what the user must define.
That could probably use more explanation.
What the member function apply must return?
The result of calling `f`.
These things need to be defined in the documentation. When a user specialize unpack_sequence, what are the constraints of the parameter F. Should it be ConstCallable?
How the unpack_sequence parameter and the Sequence parameter of member function apply are related? I don't understand
template
constexpr static auto apply(F&& f, Sequence&& s) BOOST_FIT_RETURNS ( s(std::forward<F>(f)) ); Is a Sequence a callable taking a Callable or there is a type here? Is there a missing return? I suspect it should be
template
constexpr static auto apply(F&& f, Sequence&& s) BOOST_FIT_RETURNS ( std::forward<F>(f)(s0, ,, sk); ); If this is correct, does it means that unpack just forwards to unpack_sequence? That example, I guess is a little confusing. As the `Sequence` is a function like fit::pack, but yes unpack calls unpack_sequence to do the unpacking for a single sequence.
Please, fix it.
Does the default specialization make use of INVOKE?
Its not needed, as the function passed to `unpack_sequence` will always have a call operator.
I was talking of what the *default* specialization for tuple does.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/capture.html*** Please, could you show how it is more flexible than lambda captures in C++?
You can’t capture by value or reference dependending if it is an lvalue when using lambda captures.
This merits a concrete example in the documentation just after you say "It provides more flexibility in capturing than the lambda capture list in C++" or at least have here a link to a rationale section.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/if.html*
What happens when the parameter is false_type? SFINAE?
Essentially yes. There is no call operator when its false.
This merits to be said in the documentation.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/repeat.html*
Add requirement on the function parameter that must be a Unary function such that f(f(x)) is well formed.
True, but the first call to repeat doesn’t need to be unary.
And what it could be? You lost me.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/arg.html*** What is the rational for the choice of been 1-based?
This is to match how placeholders work. So arg<1> and _1 are the same number.
I was sure that you had a good reason:)
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/construct.html*
Maybe add a reference to the factory proposal p03382.
Do you have a link?
:) http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0338r1.pdf
What happens when the parameter is not a T is not constructible from the params? SFINAE?
Yes.
What happens when the parameter is not a MetafunctionClass? SFINAE?
Yes.
Please, document it.
I believe Boost.Hana uses make instead of construct.
That is a possible name.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/decay.html*
Why not decay_copy?
It doesn’t always copy, like when passing reference_wrapper.
It copies the reference_wrapper, isn't it?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html*** Wondering if it is not better to locate them in a placeholders namespace, so that we can avoid possible collisions with other placeholders.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un...
The same here
Ah, it could cause collision if users do `using boost::fit`.
Would you move to a nested placeholders namespace?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/function_param_limit... |function_param_limit is not a Metafunction. "|The|function_param_limit| metafunction …"
It is a metafunction(doing ::type works), but its actually more like a type trait.
Right. Please fix it.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/is_callable.html*** F must be Callable can not be a requirement ;-)
Add reference to C++17 trait is_invocable.
Please fix this.
Wondering if we should have it in type_traits, if not already there.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/apply.html*** Confusion with std::apply. is this related to std::invoke?
Its not like std::apply, its like hana::apply.
I know that it is not like std::apply, hence the possible confusion.
Of course, maybe I could rename this to fit::invoke, and then add a fit::apply, that only calls the call operator.
I don't see any reason to use a different name when the function are curried.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/function.html*
"By default, all functions defined with|BOOST_FIT_STATIC_FUNCTION| use thereveal http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reveal.html adaptor to improve error messages."
How to change this default?
There isn’t a way. Perhaps I could add PP define to change it, but I wonder if its better to have some way to change it on a per-variable basis.
No problem. My comment was due to this sentence. I believed that there were a way to change it.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/lift.html*** WhyBOOST_FIT_LIFT_CLASSdoesn't defines a instance as do BOOST_FIT_STATIC_FUNCTION?
That maybe a better approach. Maybe I can call it BOOST_FIT_LIFT_FUNCTION.
Right.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/pack.html*** How this is related with fit::capture?
fit::capture uses fit::pack, but also does a pack_join to combine the pack and the parameters together.
We need some examples of these uses.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/tap.html*
How this is related to the tee unix command?
I am not that familiar with unix tee.
https://en.wikipedia.org/wiki/Tee_(command)
*http://pfultz2.github.io/Fit/doc/html/doc/src/configurations.html***Which ones?
There are none now. Are there some or should this section be removed? Vicente
On Sep 10, 2017, at 4:53 AM, Vicente J. Botet Escriba
wrote: First sorry for the formatting of my original post.
Le 09/09/2017 à 22:22, P F a écrit :
On Sep 9, 2017, at 6:27 AM, Vicente J. Botet Escriba via Boost
mailto:boost@lists.boost.org> wrote: Wondering if some parts should be moved to Boost.TypeTraits or reused from there (is_callable).
But would that imply a C++98 compatible trait? Of course, I don’t see a reason to dump every trait used in boost into Boost.TypeTraits.
I'm not talking about any traits. is_callable is special in the sense there is an equivalent in the C++ standard (is_invocable). I don't think we should require that all the traits in Boost.Traits should be compatible C++98 when we are already in C++17.
Ah yes, but I don't see why they need to be in Boost.TypeTraits.
I'm missing the std::invoke function.
What do you mean? std::invoke is provided by C++17. There is fit::apply, which will work like std::invoke except fit::apply also uses constexpr and is defined as a function object(so it can be passed to other functions).
Why not call it invoke?
I was trying to stay consistent with boost libraries.
If the user can use those classes the documentaion must document them. I guess they are useful to build other adaptors. It is not clear whether the defined functions would use SFINAE or just fail when used with types that don't respect the requirements. While I'm generally for SFINAE, I understand that it will not be the case in order to improve the compile time.
The goal is to be as transparent as possible. So if `foo(f)(x)` is equivalent to `f(x)`, then if `f(x)` causes substitution failure, then `foo(f)(x)` causes substitution failure. The same is true for constexpr and noexcept on compilers that support those well enough.
E.g.
assert(compose(f, g)(xs...) == f(g(xs...)));
If f and g don't compose, would compose(f, g) SFINAE the operator() or just compile fail.
If `f(g(xs…))` is a substitution failure then `compose(f, g)(xs…)` is a substitution failure.
If it SFINAE, I could use compose(f, g) to check if f and g are composable.
You need to check `compose(f, g)(xs…)`.
It is not clear if the functions can be final function objects or not.
They can be. There is no such restriction mentioned for the `Callable` or `ConstCallable` concept.
Oh, I see it now.
I believe there is however an issue when you have more than one final function objet. Do you have a test with two function objects? If I remember the issue was because we can not inherit from two wrapper classes doing perfect forwarding. If you don't have the issue, I will base my implementation of the proposed overload on your implementation ;-)
I added tests with two final objects and see no problem. I am not sure the issue you were seeing.
Possible files contains details that could be shared in Boost or used from other Boost libraries
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/move.hpp https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/move.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint) https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/forward..... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/forward.h...
Well hana uses a static_cast. Here, I use a macro which uses a static_cast if possible, and then I fallback to a function on compilers that can’t handle the static_cast.
What I mean is that IMHO Boost should solve these issues in a single way. If we don't want to use directly e.g. <utility> because it is heavy, we should provide the lighter components in Boost. I don't believe it is a good approach to redefine them in each library that needs such basic facilities..
Yes, I agree there should be a library for this. However, this doesn’t exists yet. In the future, I can factor this out and put it in a separate library, but right now, I am proposing the Fit library.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/noexcept.... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/noexcept.... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/and.hpp https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/and.hpp - I believe Hana use something like this also to reduce the dependencies (compile footprint)
Well, I dont know of a library in boost that implements the fast_and algorithm that works on gcc 4.6+ and msvc.
We have std::conditional. If we have a better option (for some compiler version) this should go to the Core library as any library could profit from it.
It should not go into Core, there should be a separate library for this. If we just dump random stuff into Core, it will become like Utility.
This is a basic feature.
https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/compresse... https://github.com/pfultz2/Fit/blob/boost/include/boost/fit/detail/compresse...
I dont know of a library that implements this. There is Boost.CompressedPair, but that implementation wouldn’t work at all for the Fit library.
My comment included shared. If your compressed pair is better, we should improve Boost.CompressedPair.
Perhaps, but this class is to solve very specific cases for the Fit library, so its built and tested around that. Some compilers have problems dealing with complex inheritance when using constexpr, so it is not as aggressive about empty optimization as Boost.CompressedPair.
Quite good butthe description of the semantic of the provided functions would need some extra wording. Maybe a section that states that the semantic expression is part of the constraints on the types.
What do you mean is part of the constraints on the type?
Usually we describe the expressions that the types must support on the requirements. You use a semantic clause as requirement. What I mean is that the semantic clause implies the expression must be well formed and so is a requirement on the concerned types.
The expression is not required to be well-formed.
That is the semantic expression is used to define what the function does and is also contains constraints of a not named Concept. What I would like is split the requirements and what the function does.
It doesn’t describe requirements on the types as those are listed in the documentation already. Rather it is showing the semantically equivalent behavior. So when you write `compose(f, g)(xs…)` you could paste `f(g(xs…))` and the program should be practically equivalent even when the replacement happens in a decltype, noexcept, constexpr, or substitution failure.
*Documentation:*
In general:
* Navigation only from the top of the page
* Hyper links missing.
* There is no link to real code from the examples.
Most of the examples are a full running program that can be pasted in.
Humm, I don't see the include on the code fragments, the needed using namespace, …
Those don’t, I will try to setup a more fuller example.
In particular:
*http://pfultz2.github.io/Fit/doc/html/doc/src/gettingstarted.html* http://pfultz2.github.io/Fit/doc/html/doc/src/gettingstarted.html*
An explanation of the BOOST_FIT_LIFT limitations would be more than welcome.
Good point.
Could you tell us here what they are? in detail?
The limitation is in constexpr, but this only applies to c++14. This is because `BOOST_FIT_LIFT` uses a generic lambda.
Typo: last right parenthesis in
// Pipable sum BOOST_FIT_STATIC_LAMBDA_FUNCTION(sum) = pipable([](auto x, auto y) { return x + y; });
It is not clear if the following is correct in the body of a function
auto sum = pipable([](auto x, auto y) { return x + y; };
That needs to be corrected.
Could you confirm that the previous code without using the BOOST_FIT_STATIC_FUNCTION is valid?
Which one? You can’t write `BOOST_FIT_STATIC_FUNCTION(sum)` in c++14, however, in c++17 that is possible.
If yes I believe that the documentation could use this form as far as it is inside the body of a function.
I was trying to demonstrate how to write functions other people can use.
*http://pfultz2.github.io/Fit/doc/html/doc/src/example_overloading.html* http://pfultz2.github.io/Fit/doc/html/doc/src/example_overloading.html*
Typo:
Before // Check that T has member function for operator* and ope
After // Check that T has member function for operator* and operator->
Note the typo above.
Yea got it.
*http://pfultz2.github.io/Fit/doc/html/doc/src/more_examples.html* http://pfultz2.github.io/Fit/doc/html/doc/src/more_examples.html*
My question is how both pipe operator overloads interact.
How so?
My bad. Each overload applies to different types, so that there shouldn't be no conflict.
BTW, is operator|() defined only for pipable_adaptor<F>?
Essentially yes.
Could the user define a function object that is a pipable_adaptor for your library and it is also a pipable_xxx for the range library?
What do you mean?
Static Function Adaptor
I would say "It has an additional requirement that the *class****function* is |DefaultConstructible|:"
Don't forget this one.
got it.
*http://pfultz2.github.io/Fit/doc/html/doc/src/concepts.html* http://pfultz2.github.io/Fit/doc/html/doc/src/concepts.html* ConstFunctionObject
I would like to note that this concept is not representable using the Concept TS as it is quantifying universally on the Ts (args) parameters.
The same applies to UnaryFunctionObject, BinaryFunctionObject
Wondering if the documentation should use the term Concepts then.
The purpose of the Concepts section is to document the type requirements. The current standard may not be able to check those type requirements in its current form, but there could be future versions of C++ that can check these type requirements.
No, they couldn't if they are based on the Concept TS. The Concept TS doesn't have universal quantification, and I believe it wouldn't never had it from what the authors think about this feature.
I agree, but it doesn’t mean we won’t have a future sematic-based concepts added that actually will help simplify generic programming.
The use of expresion "Is an object with ..." is confusing. What are we qualifying the type or the instance?
This means the type is an object(ie scalar, array, union, or class). Of course, an array or union cannot have a call operator, so this only applies to classes and scalars that are pointers.
I believe it is better to use the term "type" here.
It is a “type” but fulfills the requirements of `std::is_object`.
EvaluatableFunctionObject
It is not clear why this kind of FunctionObjects is useful
This mainly used by the `fit::eval` and `fit::apply_eval` functions.
I mean, it is not clear in this section, and some clarification is needed here.
Callable
I believe we have moved to call this Invocable in the standard. But again, I believe that you want to quantify universally the arguments, isn't it?
That is possible to check with the current C++(hence `is_callable`). This should be renamed to `Invocalbe` and `is_invocable`.
Well is_invocable requires to know the type of the arguments. I believe that your Callable doesn't require to know with with types the function can be called. Could you confirm?
Callable needs the type of the arguments, that is how `is_callable` is written.
UnaryCallable The name doesn't convey that it is *Const*Callable
Any concern here?
Since there is no standard UnaryCallable, I thought maybe I could sneak const in. I guess not. I will rename them to UnaryConstCallable.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/compose.html http://pfultz2.github.io/Fit/doc/html/include/boost/fit/compose.html *The requirements don't state clearly the constraints on the input parameters of f and the result of g, except by the semantic expression. This is a general remark. I suspect that you mean the semantic expression must be well formed. But, is this SFINAE friendly or is this a requirement?
It is sfinae-friendly. Could you add it to the reference documentation. This is an important feature. In the standard they use "This function should not participate in overload resolution until ... or something like that.
Is this really necessary? I think I would need to document when I change the semantic equivalence of the expression, not when I preserve it. I think it would be better in the definitions section to discuss the transparency aspect of the adaptors.
Typo "Proposal for C++ Proposal for" P0051 original proposal uses overload_linearly/overload (as Boost.Hana) instead of conditional/match. Wondering if those names aren't more clear.
I really hate the name overload_linearly. Its long and doesn’t describe what it does.
I don't find that conditional/match describe what they do. This is the eternal naming issue :)
Its called `match` so it picks the best match using C++ overload resolution. Its also used for a form of pattern matching in visitors. I could use the name `overload`, but there a different ways of overloading. This is why I chose `match`. Its called `conditional` because it takes functions with conditions, and picks the first one. Perhaps the name `select` or `prefer` could be used. I am not sure.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/implicit.html* http://pfultz2.github.io/Fit/doc/html/include/boost/fit/implicit.html*
Is implicit a StaticFunctionAdaptor?
Yes.
Shouldn't StaticFunctionAdaptors have Functions as parameters?
What do you mean? That implicit has a template, not a class :(
I should make the definition more flexible then.
Why this pattern respect the DRY principle (we don't repeat S), I believe it abuses of implicit conversions.
Well, there is a caveat when someone uses ``auto`, like in your example:
auto ptr = create_msg();
yes, this is the caveat of returning a proxy that is convertible to anything. Could you warm the user in the documentation of this possible bad usage?
There is a guard to prevent this, but it doesn’t work in C++17.
Could you elaborate?
The guaranteed elision is in C++17 prevents this. As the class returned can only be copied by implicit conversion, by the copy elision makes this possible.
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/indirect.html http://pfultz2.github.io/Fit/doc/html/include/boost/fit/indirect.html Wondering if it could be worth adding some preconditions as I guess that the function call
indirect(f)(xs...)
will be undefined if f is null
That is true if `f` is a pointer, but is not true for all dereference operators.
For example?
http://pfultz2.github.io/Fit/doc/html/doc/src/faq.html#q-is-the-reinterpret-...
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/mutable.html http://pfultz2.github.io/Fit/doc/html/include/boost/fit/mutable.html *Example of bad usage would be welcome.**Why Fit uses always const? **Why not use a reference to the function object instead?
There is no way to safely manage the lifetime with a reference. I was talking of reference_wrapper.
IIUC, mutable should be used *only* to wrap existing functions that where no const but that don't modify the object (but it is a3rd party code and cannot be changed). I'm wrong?
I believe, or if the mutations dont matter(perhaps it is just a cache).
Is there a safe way to use MutableFunctionObjects with Fit?
Yes, you can use `std::ref.
But reference_wrapper is a ConstFunctionObject. I mean when mutable_ is safe? The 3rd party use case?
Yes pretty much.
Do you have a good example of MutableFunctionObject that is safe?
Well if the mutations don’t matter. Perhaps there is a cache, and so if the value isn’t there it just recomputes it.
After all I wonder if Mutable is the correct name. In C++ mutable means modifiable by a const function. I'll suggest NonConst as the function applicable to non-const objects.
Well mutable is an actual keyword in the language, that is why I chose that.
http://pfultz2.github.io/Fit/doc/html/include/boost/fit/protect.html http://pfultz2.github.io/Fit/doc/html/include/boost/fit/protect.html It is not clear to me what this useful for. Could you elaborate?
When you want `bind` or `lazy` to capture a bind expression instead of evaluating it.
and ... could you elaborate? Could you show how we can do it without fit::protect and with?
You can’t do it without fit::protect, unless you use boost::protect.
Is there a typo here assert(lazy(f)(protect(lazy(g)(_1)))() == f(lazy(g)(_1))) ^ Shouldn't it be assert(lazy(f)(protect(lazy(g)),_1)() == f(lazy(g)(_1)))
Nope, that is correct.
It is incoherent with the example. So maybe the example is wrong
auto lazy_apply = lazy(apply)(protect(lazy_id), _1);`
Or I'm missing something
In the semantic requirements it assumes only a unary function. I could add a binary version in the semantic requirements.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/result.html*** http://pfultz2.github.io/Fit/doc/html/include/boost/fit/result.html*** This adaptor converts explicitly the result of the call to the type. Have you considered `convert_to`?
It also adds a `result_type` to the function. Its mainly used to annotate function with the result type, which can be useful for fit::fix or Boost.Variant visitors. The documentation could make this clearer.
Yes, I know that knowing the result type allows to optimize visitors. What is the main intent? to force the conversion or to have a way to retrieve the result type of a function object?
Both actually.
The name should convey the main intent. We have std::result_of until C++14 and invoke_result since C++17 to get the result type.
Those aren’t the same as they try to deduce the type. The result adaptor provides a way to know the result type without deduction.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reveal.html* http://pfultz2.github.io/Fit/doc/html/include/boost/fit/reveal.html*
as_failures, with_failures, .... /adaptors/ don't appear on the index.
You mean the table of contents? Yea, for them to appear there they need to be moved to a different section, with a header for each section.
You have items in the index that don't have his own file, e.g. http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un... http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un..., so you should be able to add to it anything without needed a file, isn't it?
Yes, but these would go under the adaptors section, which they don’t belong.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/static.html*** http://pfultz2.github.io/Fit/doc/html/include/boost/fit/static.html***
What are the differences of this adaptor and the macro STATIC_FUNCTION?
STATIC_FUNCTION just declares a function objects at namespace or global scope, which requires the function to be constexpr constructible. The `static_` adaptor is to declare an adaptor that does not have a constexpr constructor.
It is weird the example don't show the specific case as it uses static_ at global global scope. Could you show another example that is not at the namesapce o global scope.
The purpose is to use at global or namespace scope, but the example should be update to use `BOOST_FIT_STATIC_FUNCTION`.
Why static_ couldn't declare its default constructor constexpr? I believe the description is on the other sense, which is why it is confusing. static_ requires a default constructor but don't requires the constructor to be constexpr and don't provides one constexpr default constructor
It does provide constexpr constructor. Which is why it is useful because you can constepxr initialize a function object without a constexpr constructor.
, but I suspect that if the default constructor is constexpr it should work also. IIUC, you are saying that if it is the case then it is better to use the macro BOOST_FIT_STATIC_FUNCTION.
They both can be used together. Although `static_` can be used at class scope.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack.html* http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack.html*
Why name it unpack instead of apply like in C++17?
First, it doesn’t have the same signature as std::apply.
Because you curry the function?
Kind of. It is an adaptor. It also can take multiple sequences.
Secondly, its very confusing, as ‘apply' generally means function applications especially in boost. So I would like to avoid this confusion.
We are doing a function application here, isn't it?
Eventually yes, but so does all the other adaptors. It doesn’t mean the other adaptors should be named apply either.
Need to introduce the Sequence concept and how it is customized?
Hmm that is probably a better way to define it.
unpack_sequence? How unpack_sequence is used by unpack?
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack_sequence.html... http://pfultz2.github.io/Fit/doc/html/include/boost/fit/unpack_sequence.html... Why unpack_sequence, and not the tuple-like traits?
Not every sequence could be defined with indices.
For example?
fit::pack has no index-based access, or if the sequence is defined as: auto seq = [](auto f) { reuturn f(1, 2, 3); };
Is unpack_sequence defined if the class defines the tuple-like traits?
No because they are not sfinae-friendly.
Why? Could you elaborate?
The standard mandates that they are not sfinae-friendly. See: https://stackoverflow.com/a/41708982/375343 https://stackoverflow.com/a/41708982/375343 As such there is no way to know if a type implements the std tuple-like traits.
Aside the example, it is not defined what the user must define.
That could probably use more explanation.
What the member function apply must return?
The result of calling `f`.
These things need to be defined in the documentation.
When a user specialize unpack_sequence, what are the constraints of the parameter F. Should it be ConstCallable?
It will always be a FunctionObject.
How the unpack_sequence parameter and the Sequence parameter of member function apply are related? I don't understand
template
constexpr static auto apply(F&& f, Sequence&& s) BOOST_FIT_RETURNS ( s(std::forward<F>(f)) ); Is a Sequence a callable taking a Callable or there is a type here? Is there a missing return? I suspect it should be
template
constexpr static auto apply(F&& f, Sequence&& s) BOOST_FIT_RETURNS ( std::forward<F>(f)(s0, ,, sk); ); If this is correct, does it means that unpack just forwards to unpack_sequence? That example, I guess is a little confusing. As the `Sequence` is a function like fit::pack, but yes unpack calls unpack_sequence to do the unpacking for a single sequence.
Please, fix it.
Does the default specialization make use of INVOKE?
Its not needed, as the function passed to `unpack_sequence` will always have a call operator.
I was talking of what the *default* specialization for tuple does.
For any specialization, INVOKE is not necessary.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/repeat.html* http://pfultz2.github.io/Fit/doc/html/include/boost/fit/repeat.html*
Add requirement on the function parameter that must be a Unary function such that f(f(x)) is well formed.
True, but the first call to repeat doesn’t need to be unary. And what it could be? You lost me.
Its the parameters passed by the user. That is `repeat(n)(f)(1, 2)` will call `f(1, 2)` the first time, and every time after that it will call `f` with the result of `f`.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/arg.html*** http://pfultz2.github.io/Fit/doc/html/include/boost/fit/arg.html*** What is the rational for the choice of been 1-based?
This is to match how placeholders work. So arg<1> and _1 are the same number. I was sure that you had a good reason:)
I went back and forth on this, but hana uses the convention.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/decay.html* http://pfultz2.github.io/Fit/doc/html/include/boost/fit/decay.html*
Why not decay_copy?
It doesn’t always copy, like when passing reference_wrapper.
It copies the reference_wrapper, isn't it?
Nope, it returns a reference to the object being wrapped.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html*** http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html*** Wondering if it is not better to locate them in a placeholders namespace, so that we can avoid possible collisions with other placeholders.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un... http://pfultz2.github.io/Fit/doc/html/include/boost/fit/placeholders.html#un...
The same here
Ah, it could cause collision if users do `using boost::fit`.
Would you move to a nested placeholders namespace?
Yes, I added a issue.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/apply.html*** http://pfultz2.github.io/Fit/doc/html/include/boost/fit/apply.html*** Confusion with std::apply. is this related to std::invoke?
Its not like std::apply, its like hana::apply.
I know that it is not like std::apply, hence the possible confusion.
Of course, maybe I could rename this to fit::invoke, and then add a fit::apply, that only calls the call operator.
I don't see any reason to use a different name when the function are curried.
fit::apply is not curried.
*http://pfultz2.github.io/Fit/doc/html/include/boost/fit/tap.html* http://pfultz2.github.io/Fit/doc/html/include/boost/fit/tap.html*
How this is related to the tee unix command?
I am not that familiar with unix tee. https://en.wikipedia.org/wiki/Tee_(command) https://en.wikipedia.org/wiki/Tee_(command)
It is kind of similar, but its not saving a file.
*http://pfultz2.github.io/Fit/doc/html/doc/src/configurations.html***Which http://pfultz2.github.io/Fit/doc/html/doc/src/configurations.html***Which ones?
There are none now. Are there some or should this section be removed?
Yea, for some reason they dont show. I dont know if something happened with the boost conversion. You can see the pre-boost version of this page here: http://fit.readthedocs.io/en/latest/doc/src/configurations.html http://fit.readthedocs.io/en/latest/doc/src/configurations.html Also, I added issues for a lot of the points raised in the email here: https://github.com/pfultz2/Fit/issues https://github.com/pfultz2/Fit/issues Let me know if I missed anything. Thanks, Paul
participants (2)
-
P F
-
Vicente J. Botet Escriba