Skip to content

Fix prevector bugs.#4990

Merged
zkbot merged 4 commits into
zcash:masterfrom
nuttycom:backport/7888_prevector_bugs
Feb 17, 2021
Merged

Fix prevector bugs.#4990
zkbot merged 4 commits into
zcash:masterfrom
nuttycom:backport/7888_prevector_bugs

Conversation

@nuttycom

Copy link
Copy Markdown
Contributor

kazcw and others added 4 commits February 17, 2021 10:56
Fixes a bug in which pop_back did not call the deleted item's destructor.

Using the most general erase() implementation to implement all the others
prevents similar bugs because the coupling between deallocation and destructor
invocation only needs to be maintained in one place.
Also reduces duplication of complex memmove logic.

(cherry picked from commit 1e2c29f)
- add a swap operation to prevector tests (fails due to broken prevector::swap)
- fix 2 prevector test operation conditions that were impossible

(cherry picked from commit 4ed41a2)
swap was using an incorrect condition to determine when to apply an optimization
(not swapping the full direct[] when swapping two indirect prevectors).

Rather than correct the optimization I'm removing it for simplicity. Removing
this optimization minutely improves performance in the typical (currently only)
usage of member swap(), which is swapping with a freshly value-initialized
object.

(cherry picked from commit a7af72a)
@nuttycom

Copy link
Copy Markdown
Contributor Author

replaced #4984

@nuttycom

Copy link
Copy Markdown
Contributor Author

@zkbot r+

(See approvals on #4984)

@zkbot

zkbot commented Feb 17, 2021

Copy link
Copy Markdown
Contributor

📌 Commit 372ed59 has been approved by nuttycom

@zkbot

zkbot commented Feb 17, 2021

Copy link
Copy Markdown
Contributor

⌛ Testing commit 372ed59 with merge cd00fb0...

@zkbot

zkbot commented Feb 17, 2021

Copy link
Copy Markdown
Contributor

☀️ Test successful - pr-merge
Approved by: nuttycom
Pushing cd00fb0 to master...

@zkbot
zkbot merged commit cd00fb0 into zcash:master Feb 17, 2021
@nuttycom
nuttycom deleted the backport/7888_prevector_bugs branch February 17, 2021 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants