Skip to content
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

[Refactor] Emplace elements in maps and vectors #1830

Merged
merged 4 commits into from
Sep 18, 2020

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Sep 1, 2020

With C++11, variadic templates and perfect forwarding offer a new way of adding elements into a container by means of emplacing (creating in place).

This is particularly useful.
With the traditional insert()/push_back() we are likely to make unnecessary copies (or move); for example, with maps, first the std::pair<const K, V> is created, and then it's copied to the container.
With emplace(), instead, references to the key/value objects are forwarded directly to the constructor of the value_type inside the data structure without making any copy of std::pair<const K,V>.

This PR replaces most instances of:

  • .insert(std::make_pair(K,V)) with .emplace(K, V)
  • .push_back(Class(Args...)) with .emplace_back(Args...)

in the whole code base.
It touches many files (therefore is surely conflicting with many open pull requests), but changes are trivial.

@random-zebra
Copy link
Author

Rebased.

@random-zebra
Copy link
Author

Rebased.

@random-zebra
Copy link
Author

Rebased.

@random-zebra
Copy link
Author

Rebased.

@furszy
Copy link

furszy commented Sep 17, 2020

Was going to ACK it but needs another rebase.

@random-zebra
Copy link
Author

Rebased again.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

utACK e7eb446

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

LGTM

utACK e7eb446

@random-zebra random-zebra merged commit a9946da into PIVX-Project:master Sep 18, 2020
furszy added a commit that referenced this pull request Oct 2, 2020
…cripts

9d40ace [BUG] Fix regression with emplace_back on cold-staking scripts (random-zebra)

Pull request description:

  Bug introduced in #1830 (hunted down with @furszy ).

  Emplacing `uint160(key1)` into a vector of `CTxDestination` makes it construct the destination as a `CScriptID` instead of `CKeyID`. Therefore `ExtractDestinations` returns wrong addresses for cold staking scripts.

ACKs for top commit:
  furszy:
    obvious ACK 9d40ace 🍺 .
  Fuzzbawls:
    ACK 9d40ace

Tree-SHA512: 6d816500a3e5fcf1ed615c2adebe0f1cd04a61677e9b8d9a7b3055446bf295aef2a1cd898ab391c463298119f34a1bcd2bc72a5daeda37afd6b2115d1a3ab448
random-zebra added a commit that referenced this pull request Oct 2, 2020
…tion.

b189cc3 Created coldstaking_tests unit test, checking cold staking script keys proper extraction. (furszy)

Pull request description:

  Essentially proving #1830 bug and #1888 subsequent fix.

ACKs for top commit:
  Fuzzbawls:
    ACK b189cc3
  random-zebra:
    ACK b189cc3 (failing on master, passing on #1888). Merging...

Tree-SHA512: 9af3975fe099a0e26feddf2fd52abee12864bf77bb511b2fbed6f39a9ea029cdc057d15157fafe842cd7e2b438447a08dc455c763feeecdc1378af637541eaf1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants