802347 - GC: Create a new, totally opaque Unrooted<T> type for returns

Status: RESOLVED DUPLICATE of bug 811168

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

In Bug 793577 we added the Return<T> type.  This type is appropriate for returning references to a GCThing which is rooted by some other mechanism such that is makes sense to get the GCThing as a temporary in some cases.  These cast operators mean that the properties are asserted at runtime, not compile time.

In other cases it is *wildly* inappropriate to access the returned GCThing as a temporary: NewGCThing, for example.  In these cases we'd like to force correct rooting statically, since it is never correct to use the dynamic features of Return<T>.

Comment 1

[Terrence Cole [:terrence]]

This may not work as well as I was hoping.  For the allocator functions, we generally have one or more layers of wrapping which augment the allocation, for example:

JS_NewExternalString -> js_NewExternalString -> JSExternalString::new_

These all basically look like:

static Foo *
Foo::new_(JSContext *cx) {
    // Do some assertions.

    // Handle the common case in a way that may GC.

    RawFoo foo = NewGCFoo(cx);

    foo.init(...);
    Probes::etc(cx);
    // And other stuff that does not GC.

    return foo;
}

What we want here is a typey way to make a mid-expression split like:

               \ Can GC
                \
 RawString foo = \  NewGCString(cx);
                  \
             No GC \

And because these particular allocators flow into things like Int32ToString, IdToString, and IndexToString, the majority of the users *also* have this property.  They usually store the returned right to somewhere safe in the heap after checking for an error.

If we do this the naive way, it looks like I would just be slapping .unsafeGet() all over the place for very little real gain.

Comment 2

[Terrence Cole [:terrence]]

To clarify, what I mean is that we can't just use Return<T> here because we can't put the full assignment expression under an AutoAssertNoGC block.

Comment 3

[Terrence Cole [:terrence]]

Bug 811168 ended up solving this in a nice way.