Skip to content

Avoid using boost::optional in PassField() #17

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

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

vasild
Copy link
Contributor

@vasild vasild commented Jan 15, 2020

Refactor PassField() to an equivalent one that does not use
boost::optional.

Refactor PassField() to an equivalent one that does not use
boost::optional.
@vasild vasild requested a review from ryanofsky January 15, 2020 19:56
@vasild
Copy link
Contributor Author

vasild commented Jan 15, 2020

@ryanofsky as a followup to #16 I crudely stripped down absl::optional to about 4k lines of code (:-O). With finer stripping maybe it could go down to below 1000 lines, but it does not look good - what if a bug is introduced due to the stripping? Also, changes from upstream would be very tedious to merge and it would be one black box inside the source code. Its better to require the user to install absl externally.

While playing with it the patch in this PR materialized - we could remove some more boost without adding a boost::optional replacement.

@ryanofsky
Copy link
Collaborator

This seems like a good approach, and I can merge this PR after a little testing. I agree that replacing optional isn't good if it's going to add a lot of new code. I was thinking a small optional class with basic emplace/get functionality could be implemented in something like ~100 lines, but if this isn't the case it's probably better to avoid adding a new optional class. And maybe it's better to avoid using optional altogether, since most of the changes you've made so far do seem like simplifications.

Copy link
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK a616312

ryanofsky added a commit that referenced this pull request Jan 16, 2020
a616312 Avoid using boost::optional in PassField() (Vasil Dimov)

Pull request description:

  Refactor PassField() to an equivalent one that does not use
  boost::optional.

ACKs for top commit:
  ryanofsky:
    Code review ACK a616312

Tree-SHA512: 43e3b107b8ba28d2ba8fb6f2c86f02585db7a588dc67aefece08e91ff79e0d850bf6312c5800cf87680f8da8cd3a649145d07886ad84bf138a5d567a80d8dbad
@ryanofsky ryanofsky merged commit a616312 into bitcoin-core:master Jan 16, 2020
@vasild vasild deleted the remove-boost-from-PassField branch January 17, 2020 15:31
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants