document overflow-checks

[ibaryshnikov]

Consider the following code:

#[wasm_bindgen]
pub fn add(a: u8, b: u8) -> u8 {
    a + b
}

The build command is

wasm-pack build --target web

Initialization js is

import init, { add } from './pkg/overflow_check.js';

window.addEventListener('load', async () => {
    await init();

    const result = add(200, 200);
    console.log(`result`, result);
});

Expected result (from JS perspective) is either 400 or error
Actual result is 144
Possible fix is to enable overflow checks in Cargo.toml

[profile.release]
overflow-checks = true

The problem here is that it's a default build, which produces a wrong result silently.

Making debug builds by default was already discussed in #437
I'd like to bring this topic again and collect some feedback here

Environment details:

wasm-pack 0.8.1
wasm-bindgen 0.2.47
rustc 1.36.0

[fitzgen]

I don't think that we want to change whether overflow checks are enabled or not for a given profile by default. If building with the debug profile, we should use the debug profile's defaults for overflow checks, and same thing for release profile. Changing that behavor is veering off too much from standard configurations for my taste.

I don't know if either @alexcrichton or @ashleygwilliams feel that this point changes the calculus of whether the default profile should be release or debug, though. I don't have strong opinions.

[alexcrichton]

I agree that we shouldn't change the default [profile] section by default as it's relatively unidiomatic in Rust. I'm personally still slightly in favor of debug-by-default, but nowadays it would be a big change for wasm-pack so it may no longer be on the table.

[ibaryshnikov]

I'd like to clarify that I don't suggest to change the default [profile]. My point is that I as a user can add to my own Cargo.toml for every project

[profile.release]
overflow-checks = true

[alexcrichton]

Ah good point! I'm personally always down for documentation updates which mention things like that

[ibaryshnikov]

What was the reason to make --release build as default?

[alexcrichton]

IIRC it was because many users coming from JS may not know about a debug/optimized distinction and if rust/wasm is evaluated with the results of wasm-pack build we'd want that to be "best in class". I may be misremembering though

[Pauan]

@alexcrichton Yeah, I vaguely remember something about users complaining about file size/performance (which are quite bad in debug mode).

[ibaryshnikov]

Well, I wanted to gather some feedback and to propose either changind defaults to --dev, or making this flag required (without defaults). My bet is that people who complained about code performance before would start to complain about compiling times later. I think that once people start building big web applications in rust, they will start switching to debug build. And now, before 1.0, it would be easier to make a breaking change.

It doesn't deny the need in a good documentation for everything.