Unique cleanup #4763
Merged
Unique cleanup #4763
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
| { | ||
| import core.memory : GC; | ||
| destroy(_p); | ||
| GC.free(cast(void*) _p); |
There was a problem hiding this comment.
I'm not entirely sure if the GC.free should be here. It seems consistent with the previous behavior, and my expectation would be that Unique should free the memory it is holding when it goes out of scope, but opAssign only calls destroy not GC.free.
Current coverage is 88.83% (diff: 100%)@@ master #4763 diff @@
==========================================
Files 124 121 -3
Lines 77300 74283 -3017
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 68495 65988 -2507
+ Misses 8805 8295 -510
Partials 0 0
|
tmccombs
force-pushed
the
unique-cleanup
branch
2 times, most recently
from
a5a7a8c to
ebecfba
Compare
| /** Forwards member access to contents. */ | ||
| auto opDot() inout { return _p; } | ||
| mixin Proxy!_p; |
There was a problem hiding this comment.
Pretty sure Proxy can only be used with lvalues, but p can be an lvalue or an rvalue
There was a problem hiding this comment.
_p is a member variable. In what circumstances would it be an rvalue?
|
@tmccombs this is getting there, could you respond to @JackStouffer and take this to the finish line? Thx! |
tmccombs
force-pushed
the
unique-cleanup
branch
from
ebecfba to
ae8fa4e
Compare
|
Auto-merge toggled on |
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.
Clean up a couple of deprecated features used in Unique.