-
Notifications
You must be signed in to change notification settings - Fork 38.7k
rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{} #18946
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
… of T{}
Optional::emplace() was only added in boost 1.56, see
boostorg/optional@2e583aa
To simply work around bitcoin#18943,
replace it with assignment of T{}
pwallet is never null everywhere where it is dereferenced, so simply replace it with a reference, which can not be null by definition.
ryanofsky
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.
Code review ACK fa1f840 and thanks for using a standalone commit for the fix
|
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. |
promag
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.
Code review ACK fa1f840.
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 fa1f840, tested on Linux Mint 19.3.
|
ACK fa1f840 |
…ith simple assignment of T{}
fa1f840 rpcwallet: Replace pwallet-> with wallet. (MarcoFalke)
fa182a8 rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{} (MarcoFalke)
Pull request description:
Closes bitcoin#18943
ACKs for top commit:
laanwj:
ACK fa1f840
ryanofsky:
Code review ACK fa1f840 and thanks for using a standalone commit for the fix
promag:
Code review ACK fa1f840.
hebasto:
ACK fa1f840, tested on Linux Mint 19.3.
Tree-SHA512: 0838485d1f93f737ce5bf12740669dcafeebb78dbc3fa15dbcc511edce64bf024f60f0497a04149a1e799d893d57b0c9ffe442020c1b9cfc3c69db731f50e712
Summary:
> rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{}
>
> Optional::emplace() was only added in boost 1.56, see
> boostorg/optional@2e583aa
>
> To simply work around bitcoin/bitcoin#18943,
> replace it with assignment of T{}
bitcoin/bitcoin@fa1f840
Backport note: Bitcoin ABC already enforces boost > 1.59, and uses `std::optional` anyway, so this first commit
is included only for future backporting convenience.
----------------------
> rpcwallet: Replace pwallet-> with wallet.
>
> pwallet is never null everywhere where it is dereferenced, so simply
> replace it with a reference, which can not be null by definition.
bitcoin/bitcoin@fa1f840
-----------
This is a backport of Core [[bitcoin/bitcoin#18946 | PR18946]]
Test Plan: `ninja all check-all`
Reviewers: #bitcoin_abc, majcosta
Reviewed By: #bitcoin_abc, majcosta
Differential Revision: https://reviews.bitcoinabc.org/D9112
Closes #18943