Skip to content

[moveonly] Extract MakeUnique into utilmemory.h #13690

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Jul 17, 2018
Merged

[moveonly] Extract MakeUnique into utilmemory.h #13690

maflcko merged 1 commit into from
Jul 17, 2018

Conversation

@Empact
Copy link
Contributor

@Empact Empact commented Jul 17, 2018 •
edited
Loading

And use it to reduce chainparamsbase.cpp's and remove interfaces/handler.cpp's reliance on util.h
This is a step toward fixing the chainparamsbase -> util circular dependency.

Confirmed no need for the util.h include via iwyu and visual inspection.
Extracted from #13639 for easier review.

@Empact Empact mentioned this pull request Jul 17, 2018
Closed
@practicalswift
Copy link
Contributor

practicalswift commented Jul 17, 2018

Concept ACK

@Empact
Extract MakeUnique into utilmemory.h
33539cb 
And use it to reduce chainparamsbase's direct reliance on util.h to
only args handling.

utilmemory.h can be replaced with <memory> once we move to C++14.
@maflcko
Copy link
Member

maflcko commented Jul 17, 2018

utACK 33539cb

1 similar comment
@theuni
Copy link
Member

theuni commented Jul 17, 2018

utACK 33539cb

@laanwj
Copy link
Member

laanwj commented Jul 17, 2018

yes why not
utACK 33539cb

@maflcko maflcko merged commit 33539cb into bitcoin:master Jul 17, 2018
maflcko pushed a commit that referenced this pull request Jul 17, 2018
Merge #13690: [moveonly] Extract MakeUnique into utilmemory.h
c01ab13 
33539cb Extract MakeUnique into utilmemory.h (Ben Woosley)

Pull request description:

  And use it to reduce chainparamsbase.cpp's and remove interfaces/handler.cpp's reliance on util.h
  This is a step toward fixing the chainparamsbase -> util circular dependency.

  Confirmed no need for the util.h include via iwyu and visual inspection.
  Extracted from #13639 for easier review.

Tree-SHA512: 61cbf9eafb68c3e3706d21c70aeb0586a85364dad32cc49c2d40e963ac3b2c44424ad1522788a0a6b2a689fd9294ebce4482a392ceb88a94eabe09a84f070ce4
@Empact Empact deleted the util-memory branch July 17, 2018 18:59
@Empact
Copy link
Contributor Author

Empact commented Jul 17, 2018

For the record, here's the relevant output from iwyu, as run by my fork here:
https://github.com/Empact/include-what-you-use/tree/llvm-mac-os
Empact/include-what-you-use@5eba4e8

(interfaces/handler.h has correct #includes/fwd-decls)

(interfaces/handler.cpp has correct #includes/fwd-decls)
make[2]: *** [interfaces/libbitcoin_util_a-handler.o] Error 2
  CXX      libbitcoin_util_a-utilstrencodings.o

chainparamsbase.h should remove these lines:
- #include <vector>  // lines 10-10

The full include-list for chainparamsbase.h:
#include <memory>  // for unique_ptr
#include <string>  // for string
---

chainparamsbase.cpp should add these lines:
#include <stdexcept>     // for runtime_error

The full include-list for chainparamsbase.cpp:
#include <chainparamsbase.h>
#include <assert.h>      // for assert
#include <tinyformat.h>  // for strprintf
#include <util.h>        // for ArgsManager, gArgs, OptionsCategory, Options...
#include <utilmemory.h>  // for MakeUnique
#include <stdexcept>     // for runtime_error

@Bushstar Bushstar mentioned this pull request Jul 19, 2018
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 27, 2020
@PastaPastaPasta
Merge bitcoin#13690: [moveonly] Extract MakeUnique into utilmemory.h
07bb09c 
33539cb Extract MakeUnique into utilmemory.h (Ben Woosley)

Pull request description:

  And use it to reduce chainparamsbase.cpp's and remove interfaces/handler.cpp's reliance on util.h
  This is a step toward fixing the chainparamsbase -> util circular dependency.

  Confirmed no need for the util.h include via iwyu and visual inspection.
  Extracted from bitcoin#13639 for easier review.

Tree-SHA512: 61cbf9eafb68c3e3706d21c70aeb0586a85364dad32cc49c2d40e963ac3b2c44424ad1522788a0a6b2a689fd9294ebce4482a392ceb88a94eabe09a84f070ce4
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Empact @practicalswift @maflcko @theuni @laanwj