The default bug view has changed. See this FAQ.

could we provide a terse Handle syntax for NULL?

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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

5 years ago
Created attachment 640022 [details] [diff] [review]
patch

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+
(Assignee)

Comment 3

5 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

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3c416843da0
Target Milestone: --- → mozilla16
Whiteboard: [js:t]
https://hg.mozilla.org/mozilla-central/rev/a3c416843da0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.