[SYCL] Follow rule of three in sycl headers #16080
Conversation
| TempAssignGuard(const TempAssignGuard<T> &) = delete; | ||
| TempAssignGuard operator=(const TempAssignGuard<T> &) = delete; |
There was a problem hiding this comment.
Whole point of this class is to use the constructor to temporary assign a value. Since the copy assignment/copy constructor cant take an extra assignment for the temp value to assign these functions are not useful.
| pipe_base() = default; | ||
| ~pipe_base() = default; |
There was a problem hiding this comment.
Exists so derived classes can instantiate pipe_base, so just marking as default explicitly.
| barrier(barrier &&other) noexcept = delete; | ||
| barrier &operator=(const barrier &other) = delete; | ||
| barrier &operator=(barrier &&other) noexcept = delete; | ||
| ~barrier() = default; |
There was a problem hiding this comment.
this class only has a state member variable, so the default destructor should be fine.
sycl/include/sycl/detail/common.hpp
Outdated
| tls_code_loc_t(const tls_code_loc_t &TLSCodeLoc) = default; | ||
| tls_code_loc_t &operator=(const tls_code_loc_t &TLSCodeLoc) = default; |
There was a problem hiding this comment.
This class just has the MLocalScope member variable, so default works fine here
There was a problem hiding this comment.
I think this should be delete instead. One of the main jobs of tls_code_loc_t is to maintain the global state in GCodeLocTLS. If we copy object, we could end up with the copy resetting the global state before the top-level copy of it.
There was a problem hiding this comment.
ok, fixed in the latest commit
| image_mem_impl(const image_mem_impl&) = delete; | ||
| image_mem_impl& operator=(const image_mem_impl&) = delete; |
There was a problem hiding this comment.
This class is managed by another class image_mem, which keeps this in a shared pointer. So it probably does not make sense to be able to copy.
ayylol
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
@steffenlarsen deleting the |
Alternatively, you could put the deleted ctors under the |
OK, i've done that in the latest commit, thanks. |
|
@intel/bindless-images-reviewers @Seanst98 Friendly ping. |
przemektmalon
left a comment
przemektmalon
left a comment
There was a problem hiding this comment.
bindless_images_memory.hpp LGTM
5 participants
Addresses rule-of-three coverity hits in sycl headers