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.

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

Using-declaration of assignment operators results in incorrect return type #372

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

Using-declaration of assignment operators results in incorrect return type #372

Lastique opened this issue Apr 4, 2021 · 4 comments · Fixed by #676 · May be fixed by #318
Assignees
Labels

Comments

@Lastique
Copy link
Contributor

Lastique commented Apr 4, 2021

There are a number of instances in oneTBB where the code imports operator= from a base class with a using-declaration. This is incorrect because the imported operator= that are not copy or move assignment operators will have the return type of a reference to the base class instead of the derived class. (For copy and move assignment operators the standard makes an exception, as in this case it forces the implicit generation of the assignment operators in the derived class instead of importing the operators from the base class, see [class.copy.assign]/8.) Here is a demonstration.

The solution is to define operator= in the derived classes explicitly for all argument types, and forward to the base class operator=. #318 implements this for concurrent containers.

Lastique added a commit to Lastique/tbb that referenced this issue Apr 4, 2021
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.
@alexey-katranov
Copy link
Contributor

@kboyarinov, can you please look at the issue?

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

It seems that only the following containers are affected :

While specification for them is correct, implementation is not. Conformance testing does not test for this as well.

@Lastique
Copy link
Contributor Author

You forgot concurrent_queue and concurrent_bounded_queue. See #318.

@anton-potapov
Copy link
Contributor

anton-potapov commented Nov 25, 2021

@Lastique, I believe concurrent_queue and concurrent_bounded_queue are covered by #373

Lastique added a commit to Lastique/tbb that referenced this issue Nov 25, 2021
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
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 <[email protected]>
Lastique added a commit to Lastique/tbb that referenced this issue Dec 1, 2021
…ntainers.

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.

Fixes oneapi-src#372.

Signed-off-by: Andrey Semashev <[email protected]>
isaevil pushed a commit that referenced this issue Dec 2, 2021
…ntainers.

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.

Fixes #372.

Signed-off-by: Andrey Semashev <[email protected]>
anton-potapov pushed a commit that referenced this issue Dec 2, 2021
* Fixed incorrect definitions of assignment operators in associative containers.

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.

Fixes #372.

Signed-off-by: Andrey Semashev <[email protected]>
Signed-off-by: Ilya Isaev <[email protected]>
kboyarinov pushed a commit that referenced this issue Dec 27, 2021
* Fixed incorrect definitions of assignment operators in associative containers.

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.

Fixes #372.

Signed-off-by: Andrey Semashev <[email protected]>
Signed-off-by: Ilya Isaev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants