Stream: git-wasmtime

Topic: wasmtime / PR #2024 Expand doc section about "what about ...

Wasmtime GitHub notifications bot (Jul 15 2020 at 15:49):

alexcrichton opened PR #2024 from no-std to main:

This commit expands the #![no_std] section of the documentation with
an FAQ-style set of words which explains in more detail about why we
don't support #![no_std] at this time, and how we can support it in
the future.

Wasmtime GitHub notifications bot (Jul 15 2020 at 17:01):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 15 2020 at 17:01):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 15 2020 at 17:01):

sunfishcode created PR Review Comment:

When the no-std-compat crate is considered, this paragraph feels overstated. no-std-compat eliminates the need to manually import from alloc, core, and other crates like sync. The only per-source-file clutter it adds is use std::prelude::v1::*;, which is still a burden, but it is a comparatively small one.

Wasmtime GitHub notifications bot (Jul 15 2020 at 17:01):

sunfishcode created PR Review Comment:

This sentence "The ambitious goals [...]" feels unnecessary to the rest of the paragraph.

Wasmtime GitHub notifications bot (Jul 15 2020 at 17:02):

sunfishcode created PR Review Comment:

The way this paragraph is worded, it sounds a little like the second sentence is contradicting the first. Could you reword this to make the reasoning a little clearer, along the lines of: Rust has no stable way to diagnose no_std errors in an otherwise std build => if we don't want it to break all the time we'd need a dedicated no_std CI build => that has costs in terms of CI times, CI maintenance, and developers having to do extra builds if they wish to avoid CI errors.

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:15):

alexcrichton updated PR #2024 from no-std to main:

This commit expands the #![no_std] section of the documentation with
an FAQ-style set of words which explains in more detail about why we
don't support #![no_std] at this time, and how we can support it in
the future.

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:20):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:20):

alexcrichton created PR Review Comment:

I personally feel like no-std-compat is a good example of "if it compiles it works, right?" with a big emphasis on the question mark. It's trying to make the ecosystem better but in a way that fundamentally is not possible. The best example of this is the dependency on the spin crate to implement std::sync::Mutex. A spin lock is basically never what you want unless you're a kernel and can disable interrupts. Otherwise it's attempting to sidestep fundamental questions of "how do I do multithreading or interact with the system scheduler" where you can't really sidestep these questions.

Crates like that also seem to be taking the fundamental stance of "std will never attempt to fix any of these problems, right?" when in fact PRs like https://github.com/rust-lang/rust/pull/74033 will basically fix the issue for us. If all you want is code to compile using std as-is the way it's written today, then we should be putting energy behind support in the standard library itself upstream. There really is no reason that std can't do what something like no-std-compat already does in a way that supports the already existing ecosystem idioms.

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:29):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:29):

iximeow created PR Review Comment:

This paragraph sounds like testing #![no_std] incurs approximately the same CI concerns we'd have in supporting any other additional OS/target - is that a fair understanding?

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:57):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jul 15 2020 at 18:57):

alexcrichton created PR Review Comment:

Yeah the CI testing isn't onerous, it's just not possible to do on stable right now. It's a cost to acknowledge as well but it's not like this is a showstopper or anything.

Wasmtime GitHub notifications bot (Jul 15 2020 at 20:44):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 15 2020 at 20:44):

sunfishcode created PR Review Comment:

I agree, I'm just considering the perspective of a user of Wasmtime asking about no_std support: no-std-compat exists and works today, and doesn't do anything that would get in the way of better solutions.

Wasmtime GitHub notifications bot (Jul 16 2020 at 16:06):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jul 16 2020 at 16:06):

alexcrichton created PR Review Comment:

Do you think it's worth explicitly calling this out in the documentation here? Basically adding my comment as a new "what about no-std-compat" FAQ?

Wasmtime GitHub notifications bot (Jul 16 2020 at 22:28):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 16 2020 at 22:28):

sunfishcode created PR Review Comment:

If rust-lang/rust#74033 means that our options will change in the not too distant future, one option here is to replace the above paragraph with one that links to that, and points out that we'd prefer to avoid the thrash of updating all the files to a temporary solution when a better one is one the way. Then I don't think we'd even need to mention no-std-compat.

Wasmtime GitHub notifications bot (Jul 17 2020 at 15:32):

tschneidereit submitted PR Review.

Wasmtime GitHub notifications bot (Jul 17 2020 at 15:32):

tschneidereit created PR Review Comment:

I think we should at least link to build-std. However, IIUC it'll be quite some time until that reaches stable, so it'll probably not be a "just do this" answer anytime soon. Given that, perhaps we could also link to no-std-compat and point out that people can use it if it works for their use case, but that we don't include it for the reasons @alexcrichton stated above?

Wasmtime GitHub notifications bot (Jul 17 2020 at 16:12):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 17 2020 at 16:12):

sunfishcode created PR Review Comment:

no-std-compat by itself isn't sufficient to make Wasmtime run in no_std, because at least mmap. We may make that avoidable at some point, but that'll take time too. So I suggest just linking to build-std, and not no-std-compat, and just say that we'll wait until the better solutions arrive.

Also, I think it's reasonable to ask people who want to do no_std things and don't want to wait for build-std features to stabilize to use nightly Rust for a Rust release cycle or two.

Wasmtime GitHub notifications bot (Jul 17 2020 at 19:00):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jul 17 2020 at 19:00):

alexcrichton created PR Review Comment:

One thing I don't really understand though is in what context we're writing this information down. This section is under the question "the patch is small, why not?" and this sub-point is "the idioms are different enough that it's nontrivial to do so". I don't get the impression that people are frequently sending patches with no-std-compat being used. Apart from that I'm not really sure how to tweak the words already there.

If y'all want I can write an explicit bullet saying "no-std-compat doesn't work", or I can add more words under "my target doesn't have std" pointing to -Zbuild-std and the PR I mentioned, but neither of those really changes the point that the idioms of #![no_std] are different than that of std in nontrivial ways.

Also, unrelated to this PR itself, but do y'all think that we should be moving to #![no_std] today? I can't quite get a feeling for whether y'all are playing devil's advocate or are instead along the lines of "I think we should do this and these words don't convince me we shouldn't"

Wasmtime GitHub notifications bot (Jul 17 2020 at 20:46):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 17 2020 at 20:46):

sunfishcode created PR Review Comment:

I agree we shouldn't do #![no_std] today. As you mention, until we have a way to avoid calling mmap, it wouldn't matter.

The -Zbuild-std thing sounds cool. I think we should mention/link to that, and say that it doesn't make sense to take on no_std changes with the current tools when better tools are on the horizon.

Wasmtime GitHub notifications bot (Jul 20 2020 at 10:06):

tschneidereit submitted PR Review.

Wasmtime GitHub notifications bot (Jul 20 2020 at 10:06):

tschneidereit created PR Review Comment:

This argument doesn't entirely cover other crates than Wasmtime in this repository, or in other repositories, such as wasm-tools. Would it make sense to include another question below, along the lines of "The crate I want to use without std works with small changes, why not accept those"?

After that one, yet another question could be "But I can't use std; what are my options?", where the answer could link to build-std, pointing out that it's Nightly-only for now, but progressing.

Wasmtime GitHub notifications bot (Jul 20 2020 at 10:07):

tschneidereit submitted PR Review.

Wasmtime GitHub notifications bot (Jul 20 2020 at 10:07):

tschneidereit created PR Review Comment:

I just left another comment on the first question with a suggestion for how this might be structured. Also agreed that mentioning no-std-compat might not be needed.

Wasmtime GitHub notifications bot (Jul 22 2020 at 14:25):

alexcrichton updated PR #2024 from no-std to main:

This commit expands the #![no_std] section of the documentation with
an FAQ-style set of words which explains in more detail about why we
don't support #![no_std] at this time, and how we can support it in
the future.

Wasmtime GitHub notifications bot (Jul 22 2020 at 14:29):

tschneidereit submitted PR Review.

Wasmtime GitHub notifications bot (Jul 22 2020 at 14:29):

tschneidereit submitted PR Review.

Wasmtime GitHub notifications bot (Jul 22 2020 at 14:29):

tschneidereit created PR Review Comment:

nit: s/means/mean/

Wasmtime GitHub notifications bot (Jul 22 2020 at 16:28):

alexcrichton updated PR #2024 from no-std to main:

This commit expands the #![no_std] section of the documentation with
an FAQ-style set of words which explains in more detail about why we
don't support #![no_std] at this time, and how we can support it in
the future.

Wasmtime GitHub notifications bot (Jul 29 2020 at 17:53):

alexcrichton merged PR #2024.

Last updated: Nov 22 2024 at 16:03 UTC