Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web-sys] Incorrect codegen for unsigned long long in IDBVersionChangeEvent #1149

Closed
richard-uk1 opened this issue Jan 6, 2019 · 5 comments

Comments

@richard-uk1
Copy link
Contributor

richard-uk1 commented Jan 6, 2019

The webidl signature is

[Constructor(DOMString type, optional IDBVersionChangeEventInit eventInitDict),
 Exposed=(Window,Worker,System)]
interface IDBVersionChangeEvent : Event {
    readonly    attribute unsigned long long  oldVersion;
    readonly    attribute unsigned long long? newVersion;
};

but the code in web-sys is

impl IdbVersionChangeEvent {
    pub fn old_version(&self) -> f64;
    pub fn new_version(&self) -> Option<f64>;
}

The f64 should be u64.

@richard-uk1
Copy link
Contributor Author

I don't know how this works in practice, since u64 isn't supported in ECMAScript. Maybe f64 is the correct type. But then why define something in webidl that can't be represented in js?

@chinedufn
Copy link
Contributor

Yup -> #1088 (comment)

And because WebIDL was never meant to be coupled to JS AFAICT.

@Pauan
Copy link
Contributor

Pauan commented Jan 6, 2019

And because WebIDL was never meant to be coupled to JS AFAICT.

To expand on this: WebIDL was intended to be language-agnostic (so it could be used in other languages, like Java).

But in practice it is used almost exclusively with JS, so more recent WebIDL contains more JSisms.

The f64 is correct, since JS doesn't have u64 support.

@richard-uk1
Copy link
Contributor Author

richard-uk1 commented Jan 6, 2019

Closing as invalid.

I'm going to convert like the following

fn to_u64(x: f64) -> u64 {
    if x < 0.0 || x > (u64::max_value() as f64) {
        panic!("out of bounds")
    }
    // cast rounds towards 0
    (x + 0.5) as u64
}

EDIT This doesn't work. Instead I'm only going to support up to 2^(mantissa bits + 1)

(See SO)

@Pauan
Copy link
Contributor

Pauan commented Jan 6, 2019

@derekdreery As you found, the biggest integer in JS is Number.MAX_SAFE_INTEGER, which is 9007199254740991

It seems to me that a function like that would be a nice addition to wasm-bindgen, since converting from f64 to u64 is rather tricky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants