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

Remove redundant explicitly defined copy ctors #11161

Merged
merged 1 commit into from Aug 28, 2017

Conversation

danra
Copy link
Contributor

@danra danra commented Aug 26, 2017

CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
(Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

@promag
Copy link
Member

promag commented Aug 26, 2017

Remove periods from PR title and commit message.

Otherwise trivial ACK 8a14a86.

@danra danra changed the title Remove redundant explicitly defined CFeeRate copy ctor. Remove redundant explicitly defined CFeeRate copy ctor Aug 26, 2017
@danra danra force-pushed the fix/redundant-CFeeRate-copy-ctor branch from 8a14a86 to bbf5ad8 Compare August 26, 2017 16:07
@danra
Copy link
Contributor Author

danra commented Aug 26, 2017

@promag Done

@danra danra changed the title Remove redundant explicitly defined CFeeRate copy ctor Remove redundant explicitly defined copy ctors Aug 26, 2017
@promag
Copy link
Member

promag commented Aug 26, 2017

Please rebase (remove merge commit).

@danra danra force-pushed the fix/redundant-CFeeRate-copy-ctor branch from 475b3de to f92b20a Compare August 26, 2017 19:02
@danra
Copy link
Contributor Author

danra commented Aug 26, 2017

@promag Done

@sipa
Copy link
Member

sipa commented Aug 27, 2017

utACK f92b20a85e37b2ce91cc48fa9d7a1f29e353ff13

@maflcko
Copy link
Member

maflcko commented Aug 27, 2017

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits.

No need for two commits on a 8-line-changed patch doing the same thing.

CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
(Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).
CFeeRate has an explicitly defined copy ctor which has the same functionality as the implicit default copy ctor which would h
ave been generated otherwise.
@danra danra force-pushed the fix/redundant-CFeeRate-copy-ctor branch from f92b20a to b426e24 Compare August 27, 2017 22:31
@danra
Copy link
Contributor Author

danra commented Aug 27, 2017

@MarcoFalke Done

@laanwj
Copy link
Member

laanwj commented Aug 28, 2017

utACK b426e24

@laanwj laanwj merged commit b426e24 into bitcoin:master Aug 28, 2017
laanwj added a commit that referenced this pull request Aug 28, 2017
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 19, 2019
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 23, 2019
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 24, 2019
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Nov 19, 2019
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Nov 21, 2019
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 9, 2019
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 1, 2020
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Jan 21, 2021
ae41ebe Remove redundant explicitly defined copy ctors (Dan Raviv)
454aa37 Trivial: explicit (default) copy-constructor for CTransaction (random-zebra)
2af2306 Cleanup: Remove redundant copy-constructor for CSignedMessage (random-zebra)
e28259d Masternode: proper copy-assignment for CMasternode/CmasternodePing (random-zebra)

Pull request description:

  Fix the `-Wdeprecated-copy` warnings listed in #2076

  **Background**
  While perfectly valid in C++98, implicit generation of the copy constructor is declared as deprecated in C++11, when there is already a user-defined copy assignment operator, or vice-versa.
  The rationale behind this is the "rule of three" (https://en.cppreference.com/w/cpp/language/rule_of_three)
  > If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.

  **Issue - Fix**
  We have several places that define redundant copy assignment operators (and then use the implicit copy constructor), or that do need non-default ctors/dtors but don't define copy assignment.
  (See also bitcoin#11161, which is also cherry-picked here)
  Specifically:
  - `CSignedMessage`/`CMasternodePing`/`CMasternode` use a weird copy-assignment that *swaps* with the argument (which, of course, cannot be "const" then). Here we remove all ::swap implementations, and provide actual copy assignment operators.
  - `CTransaction` has an explicit copy-assignment operator, but uses the implicit copy-ctor in `CMerkleTx` constructor.
  - `CFeeRate` and `CTxMemPoolEntry` have explicitly defined copy ctor, which can (and should, for the rule-of-three) be replaced by the default one (bitcoin#11161).

  **Note**
  Reason why these warnings pop up just now.
  Since Clang 10.0 and GCC 9.3, `-Wextra` enables `-Wdeprecated-copy` by default, and that is very spammy, especially when compiling with the QT GUI (bitcoin#15822 / bitcoin#18419).
  It is so spammy (also with `boost-1.72.0`) that Bitcoin decided to temporarily suppress this warning (bitcoin#18738) until QT fixes it upstream, and boost is definitely removed in 0.22 (bitcoin#18967)

ACKs for top commit:
  furszy:
    utACK ae41ebe

Tree-SHA512: 1f59f9f7426abbf4b7d4bb0d1ac50491eb52f776e2a018de3acc8ab381af3b69db6b9a5b60c04b48f0d495e560eaa1e3d19be03d133f230daca66ba66e766289
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 24, 2021
b426e24 Remove redundant explicitly defined copy ctors (Dan Raviv)

Pull request description:

  CFeeRate and CTxMemPoolEntry have explicitly defined copy ctors which has the same functionality as the implicit default copy ctors which would have been generated otherwise.

  Besides being redundant, it violates the rule of three (see https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) ).
  (Of course, the rule of three doesn't -really- cause a resource management issue here, but the reason for that is exactly that there is no need for an explicit copy ctor in the first place since no resources are being managed).

Tree-SHA512: c9294ebf5d955d230b44c6f0d20822975d44a34471a717d656f8b17181bcd2827f47ba897edf5accd650f5998c58aadc8ab3c91a3f556f1f6de36830ed4069ce
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants