Unions containing objects and primitive types are broken when converting from a JS value

[jdm]

The FromJSValConvertible implementation for StringOrObject (from FileReader.webidl) has this:

        if value.get().is_object() {

        }

        match StringOrObject::TryConvertToString(cx, value) {
            Err(_) => return Err(()),
            Ok(Some(value)) => return Ok(ConversionResult::Success(StringOrObject::String(value))),
            Ok(None) => (),
        }


        throw_not_in_union(cx, "Object");
        Err(())

[jdm]

This is the hasObjectTypes branch in CGUnionStructs.

[Xanewok]

From recent conversation on IRC with @jdm:

xanewok: yeah, I think we need to make a choice about whether to always generate a single type that only contains values that root themselves, or a single type that must always itself be rooted (and contains values that should only be contained inside of a rooted value), or multiple types to allow both

while I was trying to write out an example of the first option, I realized that I couldn't even figure out how to do that for "only contains values that root themselves" when that includes any or object values

so we probably want something like: #[must_root] enum Something { A(JS), B(Box<Heap>) } for "a single type that must always itself be rooted"

which would then be used with a CustomAutoRooter

same with dictionary values
...
yeah, going for unrooted members and marking the outermost type as must_root is the most workable strategy, I think

This will probably require similar work as in #19644.

@jdm if in #16678 (comment) we can hold only raw gc pointers in sequence case, if the type is used with CustomAutoRooter, can't we store raw gc thing pointers inside the union enum as well?

[jdm]

The answer is both yes and no. It would be safe to have code like:

enum FooUnion {
    A(JSVal),
    B(Vec<JSVal>),
}

if the value is always used inside CustomAutoRooter<FooUnion>.

Interestingly, adding #[must_root] would provide a false sense of security here, because if we stored the FooUnion value directly inside of a heap-allocated structure (for example, in a DOM object member) then we should be storing the JS values in Heap<JSVal> in that case.