866134 - GC: Address reported TokenStream::Position rooting hazards

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

There are currently 9 hazards reported relating to TokenStream::Position.  These are false positives since AutoKeepAtoms is in effect when TokenStream in use, so the atoms contained in Position are not going to be collected.

To get rid of these hazards, I've made Position take a dummy AutoKeepAtoms reference in its constructor to ensure that one of these is present, and told the static analysis to regard it as rooted.  I've also marked Position and AutoKeepAtoms as stack-only classes for good measure.

I'm hoping you agree this is a sensible thing to do :-)

Comment 1

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

Comment on attachment 742397 [details] [diff] [review]
Proposed changes

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

I like it. Requiring the AutoKeep reference makes it way harder to use this incorrectly in the future.

Comment 2

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4f5625652fa

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b4f5625652fa