Closed Bug 1037106 Opened 5 years ago Closed 5 years ago
Ptr to manage ownership of the Debugger* for a Debugger object, prior to being stored in a private slot
It's very slightly sad we have to release into a raw pointer, then store into the private slot, rather than do both at once. But we need the pointer for the subsequent calls, and UniquePtr::release() nulls out the UniquePtr. So this seems the cleanest we can do.
Comment on attachment 8453980 [details] [diff] [review] Patch Review of attachment 8453980 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the Debugger.cpp bits. I'd like to see what luke thinks about the MAKE stuff. ::: js/public/Utility.h @@ +170,5 @@ > #define JS_NEW_BODY(allocator, t, parms) \ > void *memory = allocator(sizeof(t)); \ > return memory ? new(memory) t parms : nullptr; > +#define JS_MAKE_BODY(newname, T, parms) \ > + T *ptr = newname<T> parms; \ It looks like your backslashes are not lined up here.
This is indeed cool. My only nit to pick would be the name "make_". For one thing, the trailing underscore seems unnecessary. If all the sibling names had trailing _ then it'd make sense, but there are plenty that don't (pod_malloc, et al). For another thing, what "make" does isn't obvious from its name, nor will it be from local syntax (viz., in "auto x = cx->make_<T>();"). It'll take a bit of grepping (frustrated by macros) to find the source. I'd rather, at the expense of verbosity, go with new_unique. One benefit is that having x_unique mean "like x, but return a UniquePtr instead" extends to the other helpers, e.g., pod_malloc_unique.
The "make" name is a nod to std::make_unique, mozilla::MakeUnique. Maybe make_unique then? (Or do away with this nonsense, out-of-place underscore naming and do makeUnique?)
Comment on attachment 8453980 [details] [diff] [review] Patch Ohhhh. How far I have fallen behind the times :) makeUnique or make_unique seems quite reasonable; make_unique would be a bit more symmetric with pod_malloc et al.
Attachment #8453980 - Flags: review?(luke) → review+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.