On 08/12/2012 09:23 PM, Nicholas Nethercote wrote:
> Hi,
>
> Sharps were removed from SpiderMonkey a while ago;  see
> http://whereswalden.com/2012/01/25/spidermonkey-no-longer-supports-sharp-variables/
> and https://bugzilla.mozilla.org/show_bug.cgi?id=566700.
>
> But there's still some sharps-related code in SpiderMonkey, e.g.
> JSContext::sharpObjectMap

I can't speak for the behavioural side of things, but I think
sharpObjectMap can go now.  It was not clear that it could go before (I
cleaned up Object::toSource recently) because the comment explaining
what it is doing is not terribly clear about what the possible rooting
scenarios are and the code was (and still largely is) a forest of
confusion and despair.

After more thought, I can only see one situation it could be there to
protect us from: a getter prop (running in JS code) which removes two
very specific elements from the object being to-sourced, then GC's at
exactly the wrong moment.  If the target of the sharp was on a different
branch recursion and was therefore not held alive by the stack (and not
by tracing, since it was removed), but the referring element was kept
alive because it is on the stack and we are about to trace it (if it
were alive from tracing we would have cleaned the sharp ref), then we
could try to toSource a dead object.  At least, I think this is the gist
of the comment in the source: at least its the only feasible situation
that I see that would require the sharpObjectMap in any situation.

Now that toSource is purely recursive, I think just using RootedObject
should be fine.  I'll craft a patch and get the fuzzers on it as soon as
I finish up some more critical work.

> and js_{Enter,Leave}SharpObject, which are
> used in obj_toSource().  Bug 566700 had some discussion about
> toSource(), and how sharps syntax shouldn't be used in it, but I
> didn't understand the discussion.
>
> Also, this doesn't look right:
>
>   js> let x = {};
>   js> x.a = x;
>   ({a:{a:{}}})
>   js>
>   js> let y = []
>   js> y[0] = y;
>   too much recursion
>
> Can anyone clarify if things are as they are supposed to be?  Thanks.
>
> Nick
> _______________________________________________
> dev-tech-js-engine-internals mailing list
> [email protected]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals

_______________________________________________
dev-tech-js-engine-internals mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals

Reply via email to