Removes remove_on_drop field from AppendVec
#32741
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brooksprumo
force-pushed
the
remove-on-drop/field
branch
from
d2dace4 to
6816523
Compare
brooksprumo
commented
Aug 7, 2023
|
|
||
| av.flush().unwrap(); | ||
| let accounts_len = av.len(); | ||
| drop(av); |
There was a problem hiding this comment.
Calling drop on a ManuallyDrop is a clippy lint. So I wrapped the block of code in its own scope to drop av without explicitly calling drop.
yhchiang-sol
approved these changes
Aug 7, 2023
Codecov Report
@@ Coverage Diff @@
## master #32741 +/- ##
=========================================
- Coverage 82.0% 81.9% -0.1%
=========================================
Files 785 785
Lines 211192 211185 -7
=========================================
- Hits 173200 173165 -35
- Misses 37992 38020 +28 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Sometimes we do not want to remove the backing storage file when dropping an AppendVec. This only happens in tests or in store-tool. Instead of keeping a boolean around per instance to decide if we remove the file or not on drop, we can use
ManuallyDropto not remove the file when we want that behavior.Summary of Changes
dropremove_on_dropfield from AppendVecManuallyDropwhen we do not want to remove the backing storage fileNote, the
set_no_remove_on_drop()method is not removed in this PR, just turned into a no-op. There are zero callers of this function, so should be safe. The next PR will remove this method, and I'm not worried about a rogue PR getting merged in between this PR and the next one.