<execution>, <numeric>: Change improper direct-initialization to copy-initialization in C++17 numeric algorithms
#4419
Conversation
frederick-vs-ja
changed the title
<numeric>: Change improper direct-initialization to copy-initialization in C++17 numeric algorithms<execution>, <numeric>: Change improper direct-initialization to copy-initialization in C++17 numeric algorithms
- s/_Implicit_construct/_Implicitly_construct - Restore some default-initialization
frederick-vs-ja
force-pushed
the
fix-parallel-numeric-conv
branch
from
ec79508
to
3f811a9
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
|
||
| template <class _Ty, class _BinaryOp, class _ArgTy> | ||
| void _Implicitly_construct_in_place_by_binary_op(_Ty& _Val, _BinaryOp& _Reduce_op, _ArgTy& _Left, _ArgTy& _Right) { | ||
| ::new (static_cast<void*>(_STD addressof(_Val))) _Ty([&]() -> _Ty { return _Reduce_op(_Left, _Right); }()); |
There was a problem hiding this comment.
And capturing it by reference too. Ditto below.
There was a problem hiding this comment.
The point is performing implicit conversion in placement new with workaround for imperfectness of forwarding. So I guess copying should be avoided.
There was a problem hiding this comment.
I'm fine with capturing by reference since it doesn't affect the function signature, even though it should be a reference to a _Pass_fn.
|
|
||
| template <class _Ty, class _BinaryOp, class _ArgTy> | ||
| void _Implicitly_construct_in_place_by_binary_op(_Ty& _Val, _BinaryOp& _Reduce_op, _ArgTy& _Left, _ArgTy& _Right) { | ||
| ::new (static_cast<void*>(_STD addressof(_Val))) _Ty([&]() -> _Ty { return _Reduce_op(_Left, _Right); }()); |
There was a problem hiding this comment.
I'm fine with capturing by reference since it doesn't affect the function signature, even though it should be a reference to a _Pass_fn.
tests/std/tests/GH_004129_conversion_in_new_numeric_algorithms/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004129_conversion_in_new_numeric_algorithms/test.cpp
Outdated
Show resolved
Hide resolved
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks for fixing these algorithms! 🛠️ 🧮 🪄 |
Follows up #4260.
Affected algorithms:
reducetransform_reduceexclusive_scaninclusive_scantransform_exclusive_scantransform_inclusive_scanThe standard wording only uses "convertible" (from the results of operations to the element type) in the requirements of these algorithms, so direct-initialization can be sometimes wrong.
The construction from
_Transform_op(*_First)seems incorrect formeowscanalgorithms as the standard doesn't seem to require this. See also #3783. Perhaps we should remove such construction later._Implicitly_construct_in_place_meowfunction templates are added for performing implicit conversion in placements new and avoiding additional move, which may be important for WG21-P2408R5.