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

Code: Make copy constructor/assignments explicit #2946

Conversation

hoffie
Copy link
Member

@hoffie hoffie commented Oct 30, 2022

Short description of changes
C++ recommends explicitly defining copy constructors when copy assignment operators are defined and vice versa. When not doing it, newer compiler versions will warn about that.
Also see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)

A test build shows that this change avoids the previously seen warnings on master.

CHANGELOG: Internal: Fix C++ build warnings regarding copy constructor and assignment functions

Context: Fixes an issue?
Fixes: #2681

Does this change need documentation? What needs to be documented and how?
No.

Status of this Pull Request

What is missing until this pull request can be merged?
Thorough review by someone who is really experienced with C++ ;)

Checklist

@hoffie hoffie added this to the Release 3.10.0 milestone Oct 30, 2022
@hoffie hoffie requested a review from pljones October 30, 2022 23:29
@hoffie hoffie added this to Triage in Tracking (old) via automation Oct 30, 2022
C++11 recommends that. When not doing it, newer compiler versions
will warn about that.
Also see
https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)

Fixes: jamulussoftware#2681
@hoffie hoffie force-pushed the autobuild-compiler-warning-copy-constructor branch from ca61c76 to dfd27ea Compare October 30, 2022 23:33
Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

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

I probably don't qualify as really experienced at C++, but it looks good to me!

@hoffie hoffie moved this from Triage to Waiting on Team in Tracking (old) Oct 31, 2022
Copy link
Collaborator

@pljones pljones left a comment

Choose a reason for hiding this comment

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

My level of C++ is below knowing what this does. Approving as the compiler says yes.

Tracking (old) automation moved this from Waiting on Team to In Progress Nov 4, 2022
@hoffie hoffie merged commit d2052e9 into jamulussoftware:master Nov 7, 2022
Tracking (old) automation moved this from In Progress to Done Nov 7, 2022
@pljones pljones removed this from Done in Tracking (old) Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Build: Compiler warnings on Xcode 13.4 (definition of implicit copy assignment operator)
3 participants