Skip to content

Transform iterator assignment change #2753

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.

Sign up for GitHub

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

Open
danhoeflinger wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Transform iterator assignment change #2753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb97228
opssible
danhoeflinger Jul 23, 2026
27562c0
adjusts tests to match new transform_iterator semantics
danhoeflinger Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions include/oneapi/dpl/pstl/iterator_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,14 +516,14 @@ class transform_iterator
{
__my_it_ = __input.__my_it_;

// If copy assignment is available, copy the functor, otherwise skip it.
// For non-copy assignable functors, this copy assignment operator departs from the sycl 2020 specification
// requirement of device copyable types for copy assignment to be the same as a bitwise copy of the object.
// TODO: Explore (ABI breaking) change to use std::optional or similar and using copy constructor to implement
// copy assignment to better comply with SYCL 2020 specification.
if constexpr (std::is_copy_assignable_v<_UnaryFunc>)
// Implement functor copy assignment via destroy + copy-construct rather than _UnaryFunc::operator=.
// This supports copy assignment of functors which are not copy assignable (like c++17 lambdas).
// For a unary functor who's copy assignment operator differs from its copy constructor, the semantics of this
// copy assignment will follow that of its copy constructor.
if (this != std::addressof(__input))
{
__my_unary_func_ = __input.__my_unary_func_;
__my_unary_func_.~_UnaryFunc();
::new (std::addressof(__my_unary_func_)) _UnaryFunc(__input.__my_unary_func_);
}
Comment on lines +523 to 527

@danhoeflinger danhoeflinger Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I considered this.

The problem with std::is_assignable_v is that it only checks for the existence of the function, and does not attempt to instantiate it. For template functions where such an assignment is not well-formed this results in a compilation error (this is part of the motivation for this change).

As for the throwing copy resulting in double delete. In this case there is no easy way out. We could implement this as a std::optional or similar equivalent, changing the layout, and marking such an occasion where no constructed functor exists, but then what happens when someone wants to dereference transform_iterator without a constructed functor? I suppose we would need to throw?

return *this;
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel_api/iterator/transform_iterator_constructible.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ test_copy_assignment()

EXPECT_EQ(9, trans6[5], "transform_iterator returns the incorrect result");

//should NOT copy __x state of functor (but still allows assignment of iterator)
//should still copy __x state of functor (despite not being copy-assignable)
trans6 = trans5;

//trans6 functor.__x remains the same, but iterator has been updated to be 100 elements later in the counting iter
EXPECT_EQ(109, trans6[5], "transform_iterator assignment with non-copy-assignable functor copies functor");
EXPECT_EQ(108, trans6[5], "transform_iterator assignment with non-copy-assignable functor still copies functor");
}

void
Expand Down
Loading