[WIP DONOTMERGE] Replace boost with C++17 (std::shared_mutex) #19183

No description provided.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #20736 (rpc: Replace boost::variant with std::variant for RPCArg.m_fallback by MarcoFalke)
• #20480 (Replace boost::variant with std::variant by MarcoFalke)
• #19288 (fuzz: Add fuzzing harness for TorController by practicalswift)
• #19064 (refactor: Cleanup thread ctor calls by hebasto)
• #18710 (Add local thread pool to CCheckQueue by hebasto)
• #18194 (Bugfix: GUI: Remove broken ability to edit the address field in the sending address book by luke-jr)
• #15294 ([moveonly] wallet: Extract RipeMd160 by Empact)

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.

@MarcoFalke Just an idea - it looks like boost::fs can be replaced with std::filesystem:

The Filesystem library provides facilities for performing operations on file systems and their components, such as paths, regular files, and directories.

The filesystem library was originally developed as boost.filesystem, was published as the technical specification ISO/IEC TS 18822:2015, and finally merged to ISO C++ as of C++17. The boost implementation is currently available on more compilers and platforms than the C++17 library.

https://en.cppreference.com/w/cpp/filesystem

Maybe it's already on your roadmap. I don't know. But I can give a hand if you like. (edit: see #19245)

concept ACK & lite cr-ACK.

Maybe it's already on your roadmap. I don't know. But I can give a hand if you like.

I am not familiar with the filesystem library so I don't feel qualified to change that code. If you have a patch handy that compiles and works on all our supported operating systems, I am happy to include it here.

Concept ACK. The changes look good to me.

Maybe it's already on your roadmap. I don't know. But I can give a hand if you like.

This would be the eventual goal. However, I think it'd make sense to leave the filesystem change for another PR, as I think it involves quite a lot of ancillary changes to make it work, as well as to be confident it covers everything the current code does (I'm especially a bit afraid with regard to windows and unicode, we've had a lot of issues with this in the past).

Concept ACK @MarcoFalke What is left to do here until the "[WIP DONTMERGE]" can be dropped? :)

commit a52ecc9 was required, which was only merged today

Could 2d483142a7051389afe74c57a216843e6306f1a8 also be reverted?

Concept ACK

Just an idea - it looks like boost::fs can be replaced with std::filesystem:

and also like this idea

@MarcoFalke CI is happily green now. Do you want to make any further changes before removing the draft status, or do you plan to submit these changes as individual PRs?

I'm particularly looking forward to the std::optional change. I'm a bit afraid that mixing boost::optional and std::optional in our code base might lead to some unnecessary confusion.

A green CI doesn't mean there are no bugs. No one knows if the shared_mutex can be replaced at all: #19183 (review) #16684 (comment)

Also, commit 8b173d4f143c4db4cfe5f88d4fd07e10c146525a introduces a bug in the gui, which isn't covered by tests.

Finally, optional can't be replaced with mechanical changes because the standard library doesn't have a non-throwing accessor by pointer. So the imo confusing use of the non-throwing accessor should be removed first. See #19426

Finally, optional can't be replaced with mechanical changes because the standard library doesn't have a non-throwing accessor by pointer. So the imo confusing use of the non-throwing accessor should be removed first. See #19426

std::optional acts like a "non-throwing accessor by pointer" by itself, e.g.:

void print_if(const std::optional<int>& val) {
    if(val) {
        std::cout << *val << "\n";
    } else {
        std::cout << "empty\n";
    }
}

https://godbolt.org/z/jPndjP

What I meant to say was that it doesn't have a get_ptr member function. Our current usage would need to be replaced by val.has_value() ? &val.value() : nullptr. So my goal is to first get rid of the get_ptr call.

Closing for now due to #19183 (review) and #16684 (comment)