849420 - Use MaybeRooted in Shape::Range

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

Rooting a Shape::Range currently requires a separate Shape::Range::AutoRooter class, which is both ugly and confuses the static rooting analysis.  It would be better if Range instead wrapped a MaybeRooted values, so that whether it roots its contents depends on a template parameter.

Comment 1

[Steve Fink [:sfink] [:s:]]

Comment on attachment 722979 [details] [diff] [review]
patch

Review of attachment 722979 [details] [diff] [review]:
-----------------------------------------------------------------

Wow, this is way nicer.

::: js/src/jsiter.cpp
@@ +141,5 @@
>  
>      size_t initialLength = props->length();
>  
>      /* Collect all unique properties from this object's scope. */
> +    Shape::Range<NoGC> r(pobj->lastProperty());

This one is CanGC.

Comment 2

[Brian Hackett [Laid off!]]

Actually, the Enumerate call inside that AutoRooter can't GC.

https://hg.mozilla.org/integration/mozilla-inbound/rev/65bb00392010

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/65bb00392010