This question is a follow-up to this one. Of note, by removing the <execution> header and using the std::for_each() overload with no execution strategy, it works well.
I have a setup with a minimal reproducing example shown below.
Base class A is pure virtual and the two derived classes have different versions of a function d() that is supposed to do some operations.
I use the factory function choose_operation to choose between class B and C at runtime, and it returns a std::unique_ptr<A>. Then I use a std::for_each, and I need to pass the correct version of A::d() into std::for_each, and from there onto a function do_stuff() which does more work. The error is the same whether I use std::execution::seq or std::execution::par.
#include <algorithm>
#include <iostream>
#include <vector>
#include <memory>
#include <cmath>
#include <execution>
struct A {
A() {};
virtual ~A() = default;
virtual double d(double x) = 0;
};
struct B : A {
double d(double x) override { return std::sqrt(std::abs(x)); }
};
struct C : A {
double d(double x) override { return std::abs(x); }
};
std::unique_ptr<A> choose_operation(std::string s) {
if (s == "B") {
return std::make_unique<B>();
} else {
return std::make_unique<C>();
}
}
double do_stuff(double x, const std::unique_ptr<A>& dfun) {
x -= 2;
return dfun->d(x);
}
int main() {
std::vector<double> v {3, -4, 2, -8, 15, 267};
auto dfun = choose_operation("B");
std::for_each(std::execution::seq, v.begin(), v.end(), [dfun = std::move(dfun)](double& x) {
x = do_stuff(x, dfun);
});
return 0;
}
I compile it with g++-13 -std=c++23 main.cpp.
Here is the error message:
In file included from /opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/execution:40,
from main.cpp:6:
/opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/pstl/glue_algorithm_impl.h: In instantiation of '__pstl::__internal::__enable_if_execution_policy<_ExecutionPolicy, void> std::for_each(_ExecutionPolicy&&, _ForwardIterator, _ForwardIterator, _Function) [with _ExecutionPolicy = const __pstl::execution::v1::sequenced_policy&; _ForwardIterator = __gnu_cxx::__normal_iterator<double*, vector<double> >; _Function = main()::<lambda(double&)>; __pstl::__internal::__enable_if_execution_policy<_ExecutionPolicy, void> = void]':
main.cpp:39:18: required from here
/opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/pstl/glue_algorithm_impl.h:59:40: error: use of deleted function 'main()::<lambda(double&)>::<lambda>(const main()::<lambda(double&)>&)'
59 | __pstl::__internal::__pattern_walk1(
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
60 | std::forward<_ExecutionPolicy>(__exec), __first, __last, __f,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
61 | __pstl::__internal::__is_vectorization_preferred<_ExecutionPolicy, _ForwardIterator>(__exec),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
62 | __pstl::__internal::__is_parallelization_preferred<_ExecutionPolicy, _ForwardIterator>(__exec));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
main.cpp:39:83: note: 'main()::<lambda(double&)>::<lambda>(const main()::<lambda(double&)>&)' is implicitly deleted because the default definition would be ill-formed:
39 | std::for_each(std::execution::seq, v.begin(), v.end(), [dfun = std::move(dfun)](double& x) {
| ^
main.cpp:39:83: error: use of deleted function 'std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = A; _Dp = std::default_delete<A>]'
In file included from /opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/memory:78,
from main.cpp:4:
/opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/bits/unique_ptr.h:522:7: note: declared here
522 | unique_ptr(const unique_ptr&) = delete;
| ^~~~~~~~~~
In file included from /opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/pstl/glue_execution_defs.h:50,
from /opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/execution:34:
/opt/homebrew/Cellar/gcc/13.2.0/include/c++/13/pstl/algorithm_impl.h:107:98: note: initializing argument 4 of 'void __pstl::__internal::__pattern_walk1(_ExecutionPolicy&&, _ForwardIterator, _ForwardIterator, _Function, _IsVector, std::false_type) [with _ExecutionPolicy = const __pstl::execution::v1::sequenced_policy&; _ForwardIterator = __gnu_cxx::__normal_iterator<double*, std::vector<double> >; _Function = main()::<lambda(double&)>; _IsVector = std::integral_constant<bool, false>; std::false_type = std::integral_constant<bool, false>]'
107 | __pattern_walk1(_ExecutionPolicy&&, _ForwardIterator __first, _ForwardIterator __last, _Function __f,
|
Can someone please help interpret the error message, and suggest appropriate ways of passing the unique pointer into the lambda capture?
I also should add that the object dfun is used other places in the code, so simply putting its definition inside the lambda body is not a good solution.
I notice that the following version for the for-loop does compile, but is it good practice to pass a reference here?
std::for_each(std::execution::seq, v.begin(), v.end(), [dfun = &dfun](double& x) {
x = do_stuff(x, *dfun);
});
The parallel algorithm tries to copy the lambda, which doesn't work because the unique_ptr can only be moved.
C++ standard section "Algorithms requirements" [algorithms.requirements] paragraph 10:
Unless otherwise specified, algorithms that take function objects as arguments are permitted to copy those function objects freely. Programmers for whom object identity is important should consider using a wrapper class that points to a noncopied implementation object such as
reference_wrapper<T>(20.14.5), or some equivalent solution
Then section 25.5.4 "For each" [alg.foreach] states for the serial version:
Preconditions: Function meets the Cpp17MoveConstructible requirements (Table 26). [Note: Function need not meet the requirements of Cpp17CopyConstructible (Table 27).
while it states for the parallel version
Preconditions: Function meets the Cpp17CopyConstructible requirements
In short: Different algorithms define different requirements on moving or copying. Normally it works, because why should a basic algorithm copy its functor, right?
However, with parallel algorithm, things are more complicated. Additionally, as a programmer you actually often want to have a copy so that you can put thread-local state such as allocations into the functor and reuse them between invocations. Sadly, the standard makes this pretty much impossible because it is not specified whether a copy will actually take place or not. See for example Thread-specific variables when using parallel algorithms
Anyway, try std::ref, switch to shared_ptr, or hold the unique_ptr outside the lambda and then use a plain reference or raw pointer in the lambda.