Closed
Bug 770795
Opened 12 years ago
Closed 12 years ago
could we provide a terse Handle syntax for NULL?
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: luke, Assigned: luke)
Details
(Whiteboard: [js:t])
Attachments
(1 file)
51.78 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
It would be nice if, to pass a NULL pointer to a Handle-taking function I didn't have to write: Rooted<JSObject*> null(cx); foo(null); One idea is to make some sentinel type to do something like: struct NullPtr {}; template <typename T> class Handle { Handle(NullPtr); } The actual Handle::ptr could point to some global constant variable (so hopefully it'd be placed in BSS and seg-fault if anyone tried to mutate the Handle's value via .address().) This NullPtr-constructed Handles would also provide a slight speed advantage over Rooted once Rooted does actual pushing/popping.
Assignee | ||
Comment 1•12 years ago
|
||
This patch adds NullPtr and tries to use it everywhere that it can. Since I was looking at all 326 matches of /Rooted.*(cx);/, I also converted the two-line: Rooted<JSObject*> obj(cx); obj = NewBlah(); into Rooted<JSObject*> obj(cx, NewBlah()); since I think this is the style I've seen more.
Comment on attachment 640022 [details] [diff] [review] patch Review of attachment 640022 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Root.h @@ +96,5 @@ > } > > + /* Create a handle for a NULL pointer. */ > + Handle(NullPtr) > + { Brace on same line as definition. @@ +97,5 @@ > > + /* Create a handle for a NULL pointer. */ > + Handle(NullPtr) > + { > + ptr = reinterpret_cast<const T *>(&NullPtr::constNullValue); This seems to allow us to pass a NullPtr in for a HandleValue. Maybe you could do one of those dummy assignments here to force a compiler check?
Attachment #640022 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #2) > This seems to allow us to pass a NullPtr in for a HandleValue. Maybe you > could do one of those dummy assignments here to force a compiler check? Good point; I'll do that in this patch. In the longer term, it has come up a few times now for me that Handle<Value> feels rather unnatural. What do you think about giving Handle<Value> a completely different interface (via a template specialization) that more resembles EncapsulatedValue? I think this makes sense because the idea of Handle's interface seems to be "hide the level of indirection introduced by the Handle by giving Handle the interface of the thing it points to" and the interface of a Value is quite different than that of a GC-thing pointer.
Assignee | ||
Comment 4•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3c416843da0
Target Milestone: --- → mozilla16
Updated•12 years ago
|
Whiteboard: [js:t]
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a3c416843da0
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•