Rewrite FromJSValConvertible to better support unions

[Ms2ger]

There are three cases to consider when converting a JS value to a Rust value:

1. Successful conversion
2. Impossible to convert the value
3. Exception is thrown from within the JS engine somewhere.

An example of case 3 happens when converting { get [Symbol.iterator]() { throw 7; } } to a sequence type.

Right now, the signature is

unsafe fn from_jsval(cx: *mut JSContext,
                     val: HandleValue,
                     option: Self::Config)
                     -> Result<Self, ()>

and we use throw_type_error to set a pending exception in the second case.

That implies it is impossible for callers to detect whether case 2 or 3 occurred. This is fine for most cases, but unions need to be able to make that distinction (and not set a pending exception for the second case).

Possible solutions

Just what I could come up with right now; feel free to propose other options.

Each section shows any enums used in the proposal, followed by the common conversion case and the union case.

Result<Self, Error>

enum Error {
    Case2(String),
    Case3
}

match T::from_jsval() {
    Ok(x) => x,
    Err(Case2(error)) => {
        throw_type_error(error);
        return Err(());
    }
    Err(Case3) => return Err(()),
}

match T::from_jsval() {
    Ok(x) => return Ok(Union::T(x)),
    Err(Case2(error)) => (),
    Err(Case3) => return Err(()),
}

Result<Result<Self, String>, ()>

match try!(T::from_jsval()) {
    Ok(x) => x,
    Err(error) => {
        throw_type_error(error);
        return Err(());
    }
}

match try!(T::from_jsval()) {
    Ok(x) => return Ok(Union::T(x)),
    Err(error) => ()
}

Result<ConversionResult<Self>, ()>

enum ConversionResult<T> {
    Success(T),
    Failure(String),
}

match try!(T::from_jsval()) {
    ConversionResult::Success(x) => x,
    ConversionResult::Failure(error) => {
        throw_type_error(error);
        return Err(());
    }
}

match try!(T::from_jsval()) {
    ConversionResult::Success(x) => return Ok(Union::T(x)),
    ConversionResult::Failure(error) => ()
}

ConversionResult<Self>

enum ConversionResult<T> {
    Case1(T),
    Case2(String),
    Case3
}

match T::from_jsval() {
    Case1(x) => x,
    Case2(error) => {
        throw_type_error(error);
        return Err(());
    }
    Case3 => return Err(()),
}

match T::from_jsval() {
    Case1(x) => return Ok(Union::T(x)),
    Case2(error) => (),
    Case3 => return Err(()),
}

CC @nox @GuillaumeGomez

[GuillaumeGomez]

Ok, now I understand better what's expected. I'll try to do something based on this piece of information.

[KiChjang]

The Result<Self, Error> approach makes the most sense to me.

[Ms2ger]

I would prefer one of the two variants that allow try!().

C @jdm @Manishearth

[nox]

Result<Result<Self, String>, ()> makes the most sense to me, especially the unit part, because that means caller just need to try! the outer Result to return early in case of pending exception.

[jdm]

I'm in favour of Result<ConversionResult<Self>, ()>. I find results inside results confusing to read, and match try!(..) { Ok(...) => ...} is particularly surprising.

[nox]

After discussing on IRC, @jdm, @Ms2ger and myself decided on

enum ConversionResult<T> {
    Success(T),
    Failure(Cow<'static, str>),
}

fn from_jsval(...) -> Result<ConversionResult<Self>, ()>;

[Ms2ger]

Cow<'static, str>, that is.

[GuillaumeGomez]

Ok, I'll give it a try.

[nox]

@Ms2ger What if the failure case was a ConversionResult::Failure(ConversionError), and the inner error can be promoted to a throw with a unsafe fn throw(self, cx: *mut JSObject) -> Result<ConversionResult<_>, ()> method?

This way we can do:

match try!(T::from_jsval(cx, ...)) {
    ConversionResult::Success(x) => x,
    ConversionResult::Failure(error) => error.throw(cx)
}