Uh oh!
There was an error while loading. Please reload this page.
refactor(NLVObject): use smart pointers for tracking object lifetime - #76
refactor(NLVObject): use smart pointers for tracking object lifetime#76Rush wants to merge 1 commit into
Conversation
| void ClearChildren() { | ||
| for (auto& ptr: children_) { | ||
| if (auto object = ptr.lock()) { |
There was a problem hiding this comment.
@mbroadst this is the reward, ability to see if those weak pointers are stil valid.
mbroadst
commented
Aug 27, 2016
yeah definitely now that we've committed to c++11, smart pointers are of course the way to go :) |
| HandleType handle_; | ||
| NLVObjectBasePtr* parentReference_; | ||
| std::unique_ptr<HandleValue, decltype(&CleanupHandler::cleanup)> handle_; |
mbroadst
commented
Aug 27, 2016
@Rush I left a few comments. I really like the direction this is taking, but I wonder if we can't split the single commit into at least two:
It's a bit hard to review this mono-commit version because of all the Thoughts? |
Rush
commented
Aug 27, 2016
Sure, I'll split it later this evening probably. Thanks for leaving the comments. |
9794cfe to
a8051f8Compare| HandleType handle() const { | ||
| return handle_; | ||
| } | ||
| const HandleType handle() const { |
There was a problem hiding this comment.
as a purely informational refactor, do you think it would be a good idea to rename this to virHandle() so we avoid mental collisions with v8's handle() ?
There was a problem hiding this comment.
otherwise I have no issues with this commit, I'll just cherry-pick it right in
There was a problem hiding this comment.
Good idea. Do you have the time to rename it yourself?
a8051f8 to
925e37eCompare
@mbroadst I had a lazy evening and a crazy idea to use weak pointers instead of
NLVObjectPtr. Note:enable_shared_from_thisis probably not necessary but I wanted you to see this branch earlier rather than later to get some comments.