808181 - GC: don't allow implicit conversion of Return<T> to T

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: v0

Jason thought of a brilliant way for us to get type safety for Return<T> without having to re-root in NoGC scopes: make the getter take an unused ref to the AutoAssertNoGC guard.  Between this and the SafeBool idiom that Jeff showed me this week, Return<T> should be much safer and easier to use, to the point where I don't think we need Unrooted<T> anymore.

The attached patch implements this and fixes the ~60 places /that the compiler found/ that needed to be updated.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 677880 [details] [diff] [review]
v0

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

::: js/src/gc/Root.h
@@ +490,5 @@
> +     * This is the SafeBool idiom for allowing use of Return<T> as a boolean
> +     * without allowing implicit conversions to any arbitrary integer.
> +     */
> +    operator ConvertibleToBool() const {
> +      return ptr_ ? 1 : NULL;

Might as well do ? 1 : 0;

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 677880 [details] [diff] [review]
v0

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

Looks good.

Comment 3

[Terrence Cole [:terrence]]

jit-test pass locally:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0d7c7806a90

Comment 4

[Terrence Cole [:terrence]]

And backed out in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f55a7f84de7a

So... success! :-(

Comment 5

[Terrence Cole [:terrence]]

Green try run at:
https://tbpl.mozilla.org/?tree=Try&rev=ecf803ac85cb

Re-pushed at:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cee05d5b1ce

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4cee05d5b1ce