Skip to content

Conversation

practicalswift
Copy link
Contributor

Meta: This is the second and final part of the const refactoring series (part one: #20581). I promise: no more refactoring PRs from me in a while! :) I'll now go back to focusing on fuzzing/hardening!

Changes in this PR:

  • Don't declare de facto const member functions as non-const
  • Don't declare de facto const reference variables as non-const

Awards for finding candidates for the above changes go to:

See #18920 for instructions on how to analyse Bitcoin Core using Clang Static Analysis, clang-tidy and cppcheck.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK 31b136e

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 9, 2020

🕵️ @sipa has been requested to review this pull request as specified in the REVIEWERS file.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 17, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK 31b136e ❄️

@ajtowns
Copy link
Contributor

ajtowns commented Jan 7, 2021

ACK 31b136e

Don't double negatives? ("Declare de facto const reference variables/member functions as const" -- might be worth updating the PR title at least if you don't need to invalidate acks for some other reason)

@maflcko maflcko changed the title Don't declare de facto const member functions as non-const. Don't declare de facto const reference variables as non-const. Declare de facto const reference variables/member functions as const Jan 7, 2021
@maflcko maflcko merged commit f13e03c into bitcoin:master Jan 7, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 7, 2021
@practicalswift practicalswift deleted the const-member-fn-and-const-ref-var branch April 10, 2021 19:43
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 24, 2022
Summary:
This is a backport of [[bitcoin/bitcoin#20584 | core#20584]]

Note: in net.h, `InactivityCheck` was already `const`ed in D10874

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D11095
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
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.

6 participants