First sorry for the formatting of my original post.
Le 09/09/2017 à 22:22, P F a écrit :
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.
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.
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.
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.
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.
After // Check that T has member function for
operator* and operator->
Note the typo above.
Yea got it.
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*
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
*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.
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.
That is true if `f` is a pointer, but is not true for all
dereference operators.
For example?
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.
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.
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.
Yes, but these would go under the adaptors section, which they don’t belong.
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.
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?
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:
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<class F, class Sequence>
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<class F, class Sequence>
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.
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`.
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.
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.
Ah, it could cause collision if users do `using
boost::fit`.
Would you move to a nested placeholders namespace?
Yes, I added a issue.
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.
It is kind of similar, but its not saving a file.
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:
Also, I added issues for a lot of the points raised in the email here:
Let me know if I missed anything.
Thanks,
Paul