Workarounds for gcc 4.8 #1514

Merged
merged 1 commit into from Jan 9, 2017

Projects

None yet

4 participants

@vtnerd
Contributor
vtnerd commented Dec 30, 2016

@Jaqueeee

Replacement for

@vtnerd vtnerd Workarounds for gcc 4.8
fa0ee42
- login() = delete;
+ login() : username(), password() {}
+ login(std::string username_, std::string password_)
+ : username(std::move(username_)), password(std::move(password_))
@moneromooo-monero
moneromooo-monero Dec 30, 2016 Contributor

Out of curiosity, is this a better way in general than: login(const std::string &username_): username(username_) {} ?

@vtnerd
vtnerd Dec 30, 2016 Contributor

I think pass-by-value is preferable in this situation. This blog post by Dave Abrahams changed my view on pass-by-value years back. Compilers are really "smart". The one that has changed since that post is C++11 - if RVO is not possible, a C++11 compiler is required to do a move if possible.

An alternative would be to have const std::string& and std::string&& constructors, then have the former call the latter via delegated constructor. But with two arguments, 4 constructors are needed for all permutations. This implementation covers all of those cases, and is identical to the aggregate initialization I was doing before (with slightly more boilerplate).

There are two downsides: (1) the type cannot be forward-declared, and (2) every caller that needs to copy/move has to "call" the appropriate constructor first, instead of the callee doing it once (small code bloat for function call). Dave Abrahams discusses both in that post. (1) Isn't an issue with std::string since you cannot realistically forward-declare it. And for (2), in some cases the compiler applies RVO, and neither a move or copy is done anyway.

@moneromooo-monero
moneromooo-monero Jan 2, 2017 Contributor

That was an interesting article, thanks.

@Jaqueeee
Contributor

thanks @vtnerd !

@fluffypony

Reviewed

@fluffypony fluffypony merged commit fa0ee42 into monero-project:master Jan 9, 2017

7 of 10 checks passed

buildbot/monero-static-debian-armv8 Build done.
Details
buildbot/monero-static-ubuntu-arm7 Build done.
Details
buildbot/monero-static-win32 Build done.
Details
buildbot/monero-static-freebsd64 Build done.
Details
buildbot/monero-static-osx-10.10 Build done.
Details
buildbot/monero-static-osx-10.11 Build done.
Details
buildbot/monero-static-osx-10.12 Build done.
Details
buildbot/monero-static-ubuntu-amd64 Build done.
Details
buildbot/monero-static-ubuntu-i686 Build done.
Details
buildbot/monero-static-win64 Build done.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment