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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: luke, Assigned: luke)

Details

(Whiteboard: [js:t])

Attachments

(1 file)

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.
Attached patch patchSplinter Review
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.
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #640022 - Flags: review?(wmccloskey)
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+
(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.
Whiteboard: [js:t]
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.

Attachment

General

Created:
Updated:
Size: