Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

concept Callable should perfectly forward its function to invoke #189

Closed
ericniebler opened this issue Oct 8, 2016 · 13 comments
Closed

Comments

@ericniebler
Copy link
Owner

ericniebler commented Oct 8, 2016

See below for discussion.

Proposed Resolution

(The proposed resolution is relative to P0022R2 (proxy iterators) and also includes the resolution to #237.)

Change the definition of Callable ([concepts.lib.callables.regularcallable]) as follows:

template <class F, class... Args>
concept bool Callable() {
-  return CopyConstructible<F>() &&
-    requires(F f, Args&&... args) {
-      invoke(f, std::forward<Args>(args)...); // not required to be equality preserving
+  return
+    requires(F&& f, Args&&... args) {
+      invoke(std::forward<F>(f), std::forward<Args>(args)...); // not required to be equality preserving
    };
}

Change the definition of Predicate ([concepts.lib.callables.predicate]) as follows:

template <class F, class... Args>
concept bool Predicate() {
  return RegularCallable<F, Args...>() &&
-    Boolean<result_of_t<F&(Args...)>>();
+    Boolean<result_of_t<F(Args...)>>();
}

Change the definition of IndirectCallable in [indirectcallables.indirectfunc] to:

template <class F>
concept bool IndirectCallable() {
-  return Callable<F>();
+  return CopyConstructible<F>() &&
+    Callable<F&>();
}
template <class F, class I>
concept bool IndirectCallable() {
  return Readable<I>() &&
+    CopyConstructible<F>() &&
-    Callable<F, value_type_t<I>>() &&
-    Callable<F, reference_t<I>>() &&
-    Callable<F, iter_common_reference_t<I>>();
+    Callable<F&, value_type_t<I>&>() &&
+    Callable<F&, reference_t<I>>() &&
+    Callable<F&, iter_common_reference_t<I>>();
}
template <class F, class I1, class I2>
concept bool IndirectCallable() {
  return Readable<I1>() && Readable<I2>() &&
+    CopyConstructible<F>() &&
-    Callable<F, value_type_t<I1>, value_type_t<I2>>() &&
-    Callable<F, value_type_t<I1>, reference_t<I2>>() &&
-    Callable<F, reference_t<I1>, value_type_t<I2>>() &&
-    Callable<F, reference_t<I1>, reference_t<I2>>() &&
-    Callable<F, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
+    Callable<F&, value_type_t<I1>&, value_type_t<I2>&>() &&
+    Callable<F&, value_type_t<I1>&, reference_t<I2>>() &&
+    Callable<F&, reference_t<I1>, value_type_t<I2>&>() &&
+    Callable<F&, reference_t<I1>, reference_t<I2>>() &&
+    Callable<F&, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
}

Change IndirectRegularCallable in likewise fashion.

Change IndirectCallablePredicate to:

template <class F, class I>
concept bool IndirectCallablePredicate() {
  return Readable<I>() &&
+    CopyConstructible<F>() &&
-    Predicate<F, value_type_t<I>>() &&
-    Predicate<F, reference_t<I>>() &&
-    Predicate<F, iter_common_reference_t<I>>();
+    Predicate<F&, value_type_t<I>&>() &&
+    Predicate<F&, reference_t<I>>() &&
+    Predicate<F&, iter_common_reference_t<I>>();
}
template <class F, class I1, class I2>
concept bool IndirectCallablePredicate() {
  return Readable<I1>() && Readable<I2>() &&
+    CopyConstructible<F>() &&
-    Predicate<F, value_type_t<I1>, value_type_t<I2>>() &&
-    Predicate<F, value_type_t<I1>& reference_t<I2>>() &&
-    Predicate<F, reference_t<I1>, value_type_t<I2>>() &&
-    Predicate<F, reference_t<I1>, reference_t<I2>>() &&
-    Predicate<F, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
+    Predicate<F&, value_type_t<I1>&, value_type_t<I2>&>() &&
+    Predicate<F&, value_type_t<I1>&, reference_t<I2>>() &&
+    Predicate<F&, reference_t<I1>, value_type_t<I2>&>() &&
+    Predicate<F&, reference_t<I1>, reference_t<I2>>() &&
+    Predicate<F&, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
}

Change IndirectCallableRelation to:

template <class F, class I1, class I2 = I1>
concept bool IndirectCallableRelation() {
  return Readable<I1>() && Readable<I2>() &&
+    CopyConstructible<F>() &&
-    Relation<F, value_type_t<I1>, value_type_t<I2>>() &&
-    Relation<F, value_type_t<I1>, reference_t<I2>>() &&
-    Relation<F, reference_t<I1>, value_type_t<I2>>() &&
-    Relation<F, reference_t<I1>, reference_t<I2>>() &&
-    Relation<F, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
+    Relation<F&, value_type_t<I1>&, value_type_t<I2>&>() &&
+    Relation<F&, value_type_t<I1>&, reference_t<I2>>() &&
+    Relation<F&, reference_t<I1>, value_type_t<I2>&>() &&
+    Relation<F&, reference_t<I1>, reference_t<I2>>() &&
+    Relation<F&, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
}

Change IndirectCallableStrictWeakOrder to:

template <class F, class I1, class I2 = I1>
concept bool IndirectCallableStrictWeakOrder() {
  return Readable<I1>() && Readable<I2>() &&
+    CopyConstructible<F>() &&
-    StrictWeakOrder<F, value_type_t<I1>, value_type_t<I2>>() &&
-    StrictWeakOrder<F, value_type_t<I1>, reference_t<I2>>() &&
-    StrictWeakOrder<F, reference_t<I1>, value_type_t<I2>>() &&
-    StrictWeakOrder<F, reference_t<I1>, reference_t<I2>>() &&
-    StrictWeakOrder<F, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
+    StrictWeakOrder<F&, value_type_t<I1>&, value_type_t<I2>&>() &&
+    StrictWeakOrder<F&, value_type_t<I1>&, reference_t<I2>>() &&
+    StrictWeakOrder<F&, reference_t<I1>, value_type_t<I2>&>() &&
+    StrictWeakOrder<F&, reference_t<I1>, reference_t<I2>>() &&
+    StrictWeakOrder<F&, iter_common_reference_t<I1>, iter_common_reference_t<I2>>();
}

Change the signatures of the generate and generate_n algorithms as follows:

-template <Callable F, OutputIterator<result_of_t<F&()>> O,
-    Sentinel<O> S>
+template <class F, Iterator O, Sentinel<O> S>
+    requires Callable<F&>() && Writable<O, result_of_t<F&()>>()
O generate(O first, S last, F gen);

-template <Callable F, OutputRange<result_of_t<F&()>> Rng>
+template <class F, class Rng>
+    requires Callable<F&>() && OutputRange<Rng, result_of_t<F&()>>()
  safe_iterator_t<Rng>
    generate(Rng&& rng, F gen);

-template <Callable F, OutputIterator<result_of_t<F&()>> O>
+template <class F, Iterator O>
+    requires Callable<F&>() && Writable<O, result_of_t<F&()>>()
  O generate_n(O first, difference_type_t<O> n, F gen);
@CaseyCarter
Copy link
Collaborator

CaseyCarter commented Oct 9, 2016

Originally, the function & callable concepts treated functions as lvalues since the algorithms accept functions by value, and therefore always invoke lvalue expressions. Given that we will almost certainly be using the Callable concepts with non-algorithm library components, and we want to be somewhat consistent with is_callable, I think we have enough reason to change that behavior to perfectly forward.

@ericniebler
Copy link
Owner Author

Then it seems we should also remove the CopyConstructible requirement, or else change it to CopyConstructible<remove_reference_t<T>>(). References are not themselves copy-constructible, IIRC. I prefer the latter since then algorithms will still be able to pass functions by value to other algorithms without having to clutter up their requires clauses.

@CaseyCarter
Copy link
Collaborator

IIRC, the algorithms never use Callable directly, only IndirectCallable et al. We could easily change:

template <class F, class...Is>
concept bool IndirectCallable() {
  return (Readable<Is>() && ...) &&
    Callable<F, value_type_t<Is>...>();
}

to

template <class F, class...Is>
concept bool IndirectCallable() {
  return (Readable<Is>() && ...) &&
    CopyConstructible<F>() &&
    Callable<F&, value_type_t<Is>...>();
}

@ericniebler
Copy link
Owner Author

Sure. And about Callable requiring CopyConstructible, what are your thoughts?

@CaseyCarter
Copy link
Collaborator

If we want Callable to correctly reflect the is_callable trait and be generally applicable, I think we must remove the CopyConstructible requirement. I am happy that we have a way to do that without adding CopyConstructible to every algorithm declaration ;)

@CaseyCarter
Copy link
Collaborator

the algorithms never use Callable directly

except for the generate family, that is.

@ericniebler
Copy link
Owner Author

I am happy that we have a way to do that without adding CopyConstructible to every algorithm declaration

You mean to add CopyConstructible to IndirectlyCallable?

@CaseyCarter
Copy link
Collaborator

Yes.

@ericniebler
Copy link
Owner Author

ericniebler commented Oct 27, 2016

Another interesting test case is:

IndirectlyCallable{C, I}
void foo(I, C&) {}

void bar(int&) {}

int main() {
  int *p=nullptr;
  foo(p, bar);
}

Here, C is deduced to be the function type void(int&). Function types are not CopyConstructible, so this breaks your recommended tweak to IndirectlyCallable.

The algorithm signatures "fix" this by taking all function-like things by value, causing the function reference to decay to function pointers. But still, this is weird and we should think about it.

EDIT: So, our design question is: do we narrowly limit the indirect-callable concepts to standard-like facilities that take their function arguments by value, or do we generalize them to handle perfectly-forwarded function objects? I vote for the former.

@ericniebler ericniebler changed the title concept Callable should perfectly forward its function to invoke, right? concept Callable should perfectly forward its function to invoke Oct 28, 2016
@ericniebler
Copy link
Owner Author

Fix customization point objects' use of Callable

@ericniebler ericniebler added ready and removed new labels Nov 12, 2016
@CaseyCarter
Copy link
Collaborator

Review discussion:

  • Need to apply && to F and Args before forming the function type to pass to result_of to avoid forming "function that returns abstract type" or losing cv-qualifiers. (May occur elsewhere in the TS as well.)

@ericniebler
Copy link
Owner Author

Correct me if I'm wrong: generate needs to constrain F with CopyConstructible after this change, right?

ericniebler added a commit that referenced this issue Nov 19, 2016
@CaseyCarter
Copy link
Collaborator

Correct me if I'm wrong: generate needs to constrain F with CopyConstructible after this change, right?

Agree.

ericniebler added a commit to ericniebler/range-v3 that referenced this issue Dec 30, 2016
ericniebler added a commit that referenced this issue Jan 18, 2017
ericniebler added a commit that referenced this issue Feb 1, 2017
ericniebler added a commit that referenced this issue Feb 15, 2017
ericniebler added a commit that referenced this issue Feb 18, 2017
ericniebler added a commit that referenced this issue Feb 18, 2017
ericniebler added a commit that referenced this issue Feb 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants