Solidity: Replace boost constructs with their C++17 STL equivalents.

Created on 14 Aug 2019  路  27Comments  路  Source: ethereum/solidity

Since we switched to C++17 we can replace several boost constructs by their STL equivalent.
This includes, but may not be limited to:

  • [x] changing boost::regex to std::regex
  • [x] changing boost::optional to std::optional
  • [x] changing boost::variant to std::variant
  • [ ] changing boost::filesystem to std::filesystem (note: this might have to be postponed due to https://github.com/ethereum/solidity/pull/7263#discussion_r314240318)
  • [ ] replace boost::program_options with CLI11 (suggested by @chfast at https://github.com/ethereum/solidity/issues/4612#issuecomment-411027733)
  • [ ] check any other feature which is already part of C++17
  • [ ] for Boost integer we can use the intx library (already seperate ticket: #3851)
  • [ ] with regards to Boost.Ranges we should consider using ranges-v3 that are compatible with C++11/14/17 and is going to be part of C++20, too. (can become a "splitout"-ticket)
  • [ ] replace boost::noncopyable with deleted constructor/assignments (https://github.com/ethereum/solidity/issues/7259#issuecomment-561157399)

Please Note:

Each of those changes should be done in its own PR.

good first issue

All 27 comments

@ekpyron Can I work on this ?

@Patil2099 Sure! Best start with one of them and create a PR for it, so that it won't be too many changes at once. Just leave a comment here, when you picked one and started the work!

@ekpyron can I work on this issue?

@ameyanator Sorry for the late reply - but yes, since it doesn't seem like @Patil2099 is currently still working on this, so feel free to work on this one. Be sure to use a different PR for each change - and we will probably have to stay with boost::filesystem for now due to problems on MacOS, but the other two are possible now. Best to start with replacing boost::optional.
Let us know, if you start working on this.

Ah sorry, I was away for a bit, so now I realize that there's already a PR for boost::optional.
So the next task would be boost::variant.

@ekpyron I was working on web3.py repo and completely forgot about this one. I can start working on it. If you want

@Patil2099 @ameyanator I'd suggest the first one to post a comment like "I'm starting to work on implementing [boost construct]" can take over that boost construct.
Be aware however, that there are more issues connected to this than anticipated (see https://github.com/ethereum/solidity/pull/7467 trying to implement boost::option and the earlier issues with std::filesystem), and that this is currently low priority for the solidity team, so we will probably only be able to offer limited help at the moment.

Hey! Can i work one of the replacement?

I'm sorry I got caught up with my school exams. I'm free now, I'll look into the issue you have mentioned @ekpyron and then I'll start to work on it.

I'm starting to work on implementing boost::variant to std::variant.

If I face any issues I'll mention them over here.

I'm starting to work on boost::filesystem to std::filesystem.

@glanderson42 note that, as mentioned above, we might have to stay with boost::filesystem for now, since there is only limited support for std::filesystem on MacOS. If you have can test on Mac and make sure it works well there, great, otherwise we need to postpone this part for now.

@ekpyron Thank you! Unfortunately I don't have physical Apple computer for testing. :(
Otherwise, I'm going to work on boost::optional replacement.

Wasn't some of this done already?

Hm since we require C++14/17 now, cannot we just replace boost/noncopyable with a deleted constructor or we like that it is easy to search for noncopyable?

Wasn't some of this done already?

Yes, some of it was done, as indicated in the issue description.

Hm since we require C++14/17 now, cannot we just replace boost/noncopyable with a deleted constructor or we like that it is easy to search for noncopyable?

At least I'd like to have it replaced with deleted constructors and assignment operators. Searching for = delete; will still work...

Wasn't some of this done already?

Yes, some of it was done, as indicated in the issue description.

I have updated it after asking the question :wink:

Wasn't some of this done already?

Yes, some of it was done, as indicated in the issue description.

I have updated it after asking the question wink

Ah, ok :-D

I started using CLI11 in EVMC, and I'm not happy. The API is pretty bad and over-complicated and documentation is out-of-sync (I usually browse the code to figure out how to do stuff).

I'm also consider trying https://github.com/Taywee/args now.

changing boost::filesystem to std::filesystem (note: this might have to be postponed due to https://github.com/ethereum/solidity/pull/7263#discussion_r314240318)

Strictly speaking about libsolidity/libsolc (and not the tests and CLI): boost:filesystem is only used via the absolutePath and sanitizePath helpers in libsolutil. Perhaps we could replace those with std::filesystem or even with home brew helpers, if that is a useful reduction of boost dependency. I'm not fully sure it is.

if the issue is still open i can also dive in

@Neyromancer I'm not sure this is a really such a good first issue at this point, mostly because most of these have been tried and stopped on some blocker (dependency management, not working on macOS, not a good replacement, etc.) but you could pick something and try.

I think that boost::noncopyable would be the most straightforward. Also introducing more C++17 features to the code base would be nice if you find anything we're not using yet. Maybe ranges v3 would be good too if there are no objections to introducing them now (instead of waiting for C++20). With any of the others, prepare for a bit of discussion regarding the proposed solution because there's no consensus yet. In the end we still want all of these things so if you're persistent enough, any of them is up for grabs.

In case you'd prefer something else after all, please see https://github.com/ethereum/solidity/issues/7495#issuecomment-731481159. I labeled more issues as good first issue and added some recommendations for you.

Maybe ranges v3 would be good too if there are no objections to introducing them now (instead of waiting for C++20).

Even when C++20 comes out, we'll still have the b_ubu18 build job, don't we? 馃槵

i.e. using ranges-v3 could be a good idea.

Actually, I already kind of got clearance from @chriseth to remove b_ubu18 as well as the PPA builds, replacing the latter by static builds, just as we did for "xenial" for a while, so that shouldn't keep us. But we probably still need stuff like ranges-v3 if we want to transition early (which I vote for doing!).

Actually, I already kind of got clearance from @chriseth to remove b_ubu18 as well as the PPA builds, replacing the latter by static builds, just as we did for "xenial" for a while, so that shouldn't keep us.

When? This is blocking two of my PRs :)

On gitter some time before I went off... I said I was annoyed by b_ubu18, because the old gcc in there has some annoying inadequacies, e.g. it doesn't always properly do copy ellision and return value optimization resulting in various weird workarounds in the Yul optimizer - and asked if we could drop it and move the PPA for ubuntu 18 to a static build (which would be built on ubuntu20) soonish - and @chriseth agreed at the time :-).

Was this page helpful?
0 / 5 - 0 ratings