-
Notifications
You must be signed in to change notification settings - Fork 38.7k
scripted-diff: TxoutType C++11 scoped enum class #19114
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
Conversation
|
Concept ACK: Prefer class enums over “plain” enums (C++ Core Guidelines) These are the remaining non-class enums that might be worth investigating too: |
|
Concept ACK. |
|
@practicalswift It might be best to open a new brainstorming issue for discussion about those. I think enum flags can not (or should not?) be switched to |
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa38a7472ab475806cf50f302db9c69dfb8ac0f5, tested on Linux Mint 19.3 (x86_64).
fa868d3a8de56582538a3e4ef8de6082f2b8c747:
maybe make a separate utility function to derive an underlying type of an enumerator?
fa69f11 to
fa5997b
Compare
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa5997bd6fc82e16b597ea96e3c5c665f1f174ab, since fa38a7472ab475806cf50f302db9c69dfb8ac0f5 suggested changes applied.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, 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. |
This commit does not change behavior
Don't blindly assume it is int. In practice this is usually `unsigned` or `int`, so this commit should not change behavior.
Also, remove scope of txnouttype in fuzz tests temporarily. The next commit will add scopes to all txnouttype.
-BEGIN VERIFY SCRIPT-
# General rename helper: $1 -> $2
rename_global() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1"); }
# Helper to rename TxoutType $1
rename_value() {
sed -i "s/ TX_$1,/ $1,/g" src/script/standard.h; # First strip the prefix in the definition (header)
rename_global TX_$1 "TxoutType::$1"; # Then replace globally
}
# Change the type globally to bring it in line with the style-guide
# (clsses are UpperCamelCase)
rename_global 'enum txnouttype' 'enum class TxoutType'
rename_global 'txnouttype' 'TxoutType'
# Now rename each enum value
rename_value 'NONSTANDARD'
rename_value 'PUBKEY'
rename_value 'PUBKEYHASH'
rename_value 'SCRIPTHASH'
rename_value 'MULTISIG'
rename_value 'NULL_DATA'
rename_value 'WITNESS_V0_KEYHASH'
rename_value 'WITNESS_V0_SCRIPTHASH'
rename_value 'WITNESS_UNKNOWN'
-END VERIFY SCRIPT-
fa5997b to
fa32adf
Compare
|
Rebased due to merge conflict in adjacent line. Can be trivially re-ACKed with git range-diff |
|
ACK fa32adf -- patch looks correct |
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK fa32adf, since fa5997bd6fc82e16b597ea96e3c5c665f1f174ab (#19114 (review)) rebased only (verified with git range-diff).
|
@MarcoFalke from which key-server I could receive your non-expired key to verify commit signatures? |
|
Thanks. It was helpful! |
Summary: ``` Non-scoped enums can accidentally and silently decay into an integral type. Also, the symbol names of the keys are exported to the surrounding (usually global) namespace. Fix both issues by switching to an enum class TxoutType in a (mostly) scripted-diff. ``` Backport of core [[bitcoin/bitcoin#19114 | PR19114]]. Depends on D9124. Test Plan: ninja all check-all ninja bitcoin-fuzzers Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Subscribers: PiRK Differential Revision: https://reviews.bitcoinabc.org/D9126
Non-scoped enums can accidentally and silently decay into an integral type. Also, the symbol names of the keys are exported to the surrounding (usually global) namespace.
Fix both issues by switching to an
enum class TxoutTypein a (mostly) scripted-diff.