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.

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

Jump to bottom

Concurrent queue types don't define assignment operators #373

Open
Lastique opened this issue Apr 4, 2021 · 4 comments · May be fixed by #318
Open

Concurrent queue types don't define assignment operators #373

Lastique opened this issue Apr 4, 2021 · 4 comments · May be fixed by #318
Assignees
@anton-potapov
Labels
bug

Comments

@Lastique
Copy link
Contributor

Lastique commented Apr 4, 2021

concurrent_queue and concurrent_bounded_queue define non-trivial copy and move constructor, but don't define assignment operators. The implicitly generated copy assignment will be incorrect for concurrent_queue and deleted (because of std::atomic member) for concurrent_bounded_queue, neither of which is expected by user.

Should be fixed by #318.
@Lastique Lastique changed the title Concurrent queue type don't define assignment operators Concurrent queue types don't define assignment operators Apr 4, 2021
Lastique added a commit to Lastique/tbb that referenced this issue Apr 4, 2021
@Lastique
Added missing definitions of assignment operators.
a114406 
This fixes incorrect return types of assignment operators imported from
base classes. Other than copy and move assignment operators, which are
generated by the compiler even if imported with a using-declaration,
the imported operators will be returning a reference to the base class
instead of the derived class.

In concurrent_queue, the assignment operator was missing while copy/move
constructors were present and contained non-trivial logic. The implicitly
generated assignment operator would have been incorrect. The added assignment
operator reuses copy/move constructors to implement assignment.

In concurrent_bounded_queue, the assignment operator was also missing and
would have been deleted because of std::atomic member, while the class also
had copy and move constructors. The added assignment operator also reuses
the existing constructors.

Fixes oneapi-src#372.
Fixes oneapi-src#373.
@Lastique Lastique linked a pull request Apr 4, 2021 that will close this issue
Open
@alexey-katranov
Copy link
Contributor

@kboyarinov, can you please look at the issue and related pull request?

@anton-potapov anton-potapov self-assigned this Oct 20, 2021
@anton-potapov
Copy link
Contributor

while specification for concurrent_queue and concurrent_bounded_queue are missing operator= in any form, it seems at least confusing as one for concurrent_priority_queue has them.

@Lastique
Copy link
Contributor Author

I would argue that if a type has a copy/move constructor, it is expected for it to also have the corresponding assignment operator.

@anton-potapov anton-potapov mentioned this issue Nov 25, 2021
Closed
Lastique added a commit to Lastique/tbb that referenced this issue Nov 25, 2021
@Lastique
Added missing definitions of assignment operators.
0370b6f 
This fixes incorrect return types of assignment operators imported from
base classes. Other than copy and move assignment operators, which are
generated by the compiler even if imported with a using-declaration,
the imported operators will be returning a reference to the base class
instead of the derived class.

In concurrent_queue, the assignment operator was missing while copy/move
constructors were present and contained non-trivial logic. The implicitly
generated assignment operator would have been incorrect. The added assignment
operator reuses copy/move constructors to implement assignment.

In concurrent_bounded_queue, the assignment operator was also missing and
would have been deleted because of std::atomic member, while the class also
had copy and move constructors. The added assignment operator also reuses
the existing constructors.

Fixes oneapi-src#312.
Fixes oneapi-src#372.
Fixes oneapi-src#373.
Lastique added a commit to Lastique/tbb that referenced this issue Nov 25, 2021
@Lastique
Added missing definitions of assignment operators.
9ea86a8 
This fixes incorrect return types of assignment operators imported from
base classes. Other than copy and move assignment operators, which are
generated by the compiler even if imported with a using-declaration,
the imported operators will be returning a reference to the base class
instead of the derived class.

In concurrent_queue, the assignment operator was missing while copy/move
constructors were present and contained non-trivial logic. The implicitly
generated assignment operator would have been incorrect. The added assignment
operator reuses copy/move constructors to implement assignment.

In concurrent_bounded_queue, the assignment operator was also missing and
would have been deleted because of std::atomic member, while the class also
had copy and move constructors. The added assignment operator also reuses
the existing constructors.

Fixes oneapi-src#312.
Fixes oneapi-src#372.
Fixes oneapi-src#373.

Signed-off-by: Andrey Semashev <andrey.semashev@gmail.com>
Lastique added a commit to Lastique/tbb that referenced this issue Dec 1, 2021
@Lastique
Added missing definitions of assignment operators to concurrent queues.
22dd232 
In concurrent_queue, the assignment operator was missing while copy/move
constructors were present and contained non-trivial logic. The implicitly
generated assignment operator would have been incorrect. The added assignment
operator reuses copy/move constructors to implement assignment.

In concurrent_bounded_queue, the assignment operator was also missing and
would have been deleted because of std::atomic member, while the class also
had copy and move constructors. The added assignment operator also reuses
the existing constructors.

Fixes oneapi-src#373.

Signed-off-by: Andrey Semashev <andrey.semashev@gmail.com>
Lastique added a commit to Lastique/tbb that referenced this issue Dec 3, 2021
@Lastique
Added missing definitions of assignment operators to concurrent queues.
b9b38ec 
In concurrent_queue, the assignment operator was missing while copy/move
constructors were present and contained non-trivial logic. The implicitly
generated assignment operator would have been incorrect. The added assignment
operator reuses copy/move constructors to implement assignment.

In concurrent_bounded_queue, the assignment operator was also missing and
would have been deleted because of std::atomic member, while the class also
had copy and move constructors. The added assignment operator also reuses
the existing constructors.

Fixes oneapi-src#373.

Signed-off-by: Andrey Semashev <andrey.semashev@gmail.com>
@anton-potapov
Copy link
Contributor

Specification patch for problem uxlfoundation/oneAPI-spec#396

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@anton-potapov anton-potapov

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add missing definitions of constructors and assignment operators Lastique/tbb
3 participants
@anton-potapov @Lastique @alexey-katranov