refactor: return std::nullopt instead of {} #21498
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In bitcoin#21415 we decided to return `std::optional` rather than `{}` for uninitialized values. This PR repalces the two remaining usages of `{}` with `std::nullopt`. As a side-effect, this also quells the spurious GCC 10.2.x warning that we've had reported quite a few times. i.e bitcoin#21318, bitcoin#21248, bitcoin#20797. ```bash txmempool.cpp: In member function ‘CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>&) const’: txmempool.cpp:898:13: warning: ‘<anonymous>’ may be used uninitialized in this function [-Wmaybe-uninitialized] 898 | return {}; | ^ ```
|
Posthumous ACK |
|
Looks like gcc-11 fixed the bug. I couldn't reproduce the warning before and after this patch. |
|
This seems completely pointless unless you're going to update the style guide to say that we shouldn't use |
This was worthwhile just to stop the flow of GCC false positive reports. GCC 10.2 wont be going anywhere for a long time. |
I think that's a slightly better motivation than "we decided to return std::optional rather than {}" |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Mar 22, 2021
5294f0d refactor: return std::nullopt instead of {} (fanquake) Pull request description: In bitcoin#21415 [we decided](bitcoin#21415 (comment)) to return `std::optional` rather than `{}` for uninitialized values. This PR replaces the two remaining usages of `{}` with `std::nullopt`. As a side-effect, this also quells the spurious GCC 10.2.x warning that we've had reported quite a few times. i.e bitcoin#21318, bitcoin#21248, bitcoin#20797. ```bash txmempool.cpp: In member function ‘CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>&) const’: txmempool.cpp:898:13: warning: ‘<anonymous>’ may be used uninitialized in this function [-Wmaybe-uninitialized] 898 | return {}; | ^ ``` ACKs for top commit: hebasto: ACK 5294f0d, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 5b776be79ab26e5a3a5fc2b463b394ea5ce6797ed5558424873fa4ecee2898170eff76d6da9d69394d28f8f98974117fc63b922a3e19c52f5294c83073e79bb0
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Apr 6, 2022
Summary: This is a backport of [[bitcoin/bitcoin#21498 | core#21498]] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D11305
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #21415 we decided to return
std::optionalrather than{}foruninitialized values. This PR replaces the two remaining usages of
{}with
std::nullopt.As a side-effect, this also quells the spurious GCC 10.2.x warning that
we've had reported quite a few times. i.e #21318, #21248, #20797.