bytecodealliance / wasi-rs

Experimental WASI API bindings for Rust
Apache License 2.0
271 stars 48 forks source link

`Into<NonZeroU16>` for `wasi::Error` #61

Closed Cyborus04 closed 8 months ago

Cyborus04 commented 3 years ago

In rust-random/getrandom#233 an optimization depending on wasi::Error::raw_error never returning 0 was added, which AFAIK is guaranteed but not as part of public API. This PR was accepted but would rather have direct conversion to NonZeroU16 to ensure this guarantee. Would that be possible? I looked into directly opening a PR for this but I'm not sure how that would be added

sunfishcode commented 3 years ago

This makes sense to me. I think would look like a function named raw_nonzero_error() or so, which would live next to raw_error(). Does that sound reasonable?

Cyborus04 commented 3 years ago

Sounds good, doesn't have to be an Into impl, any public access to the inner NonZeroU16 is good

Cyborus04 commented 3 years ago

Where does the Error type live exactly? docs.rs says it's in src/error.rs but I don't see that here

sunfishcode commented 3 years ago

The Error type lives here: https://docs.rs/wasi/0.10.2+wasi-snapshot-preview1/src/wasi/error.rs.html#8-10

Cyborus04 commented 3 years ago

I found that, but I don't see src/error.rs in the repo

sunfishcode commented 3 years ago

Ah, that's right. It was replaced by Errno in #59. Which doesn't use NonZeroU16, because it's auto-generated from the interface description, which doesn't record the non-zero property. So unfortunately, this change may indeed not be straightforward to do for a while.

Cyborus04 commented 3 years ago

What about a documented guarantee that none of the functions will ever return Err(ERRNO_SUCCESS)?

sunfishcode commented 3 years ago

For the WASI API bindings currently in this crate, it's safe to say that errors will always be non-zero.

WASI is in the process of transitioning to interface types, at which point it will use expected<T, Error> return types, which in this crate will map directly to Rust Result<T, Error> types. We're still figuring out the details, but I think it'll make sense to say that Error types likely don't map directly to errno values, so I expect they won't make NonZero guarantees.

For compatibility with C's errno and Rust's last_os_error and similar, wasi-libc and Rust's std will likely use an errno variable, with the ability to map WASI errors into errno values that it defines, and I expect it'll always have the invariant that 0 means success and all actual errors are non-zero. However, it won't be a NonZero type, since users can store 0 into it.

alexcrichton commented 8 months ago

This crate has now updated to WASI 0.2.0 and so this issue I believe is no longer applicable as the underlying APIs have chagned in the meantime. I'm going to close this because of that, but API suggestions for the currently generated APIs are definitely welcome!