Introduce support for environments where the standard library is not available (#![no_std]) but a global allocator is #59
Conversation
…td features are not enabled
…nality (no_std support) without breaking existing API
There was a problem hiding this comment.
Thank you. Supporting no_std would be useful. Before reviewing details in this branch, let me request some points:
- Would you apply the following patch and revert the code commented out due to warnings?
diff --git a/spec-test/src/main.rs b/spec-test/src/main.rs index ddbd226..d502206 100644 --- a/spec-test/src/main.rs +++ b/spec-test/src/main.rs @@ -1,4 +1,5 @@ #![forbid(unsafe_code)] +#![allow(dead_code)] mod crash; mod error;
- Would you revert the package version bumps in this PR? I will do them in my side after understanding the changes. Since the major versions are zero, patch versions should be bumped if there are no breaking changes.
- Could you run the spec test on this branch with enabling
no_stdand compare the results withmainbranch? If the numbers of passed test cases don't match, it would mean that there are some mistakes in the implementation added in this branch.- On my local machine, the results of the speci tests are:
total: 19757, passed: 19460, failed: 179, skipped: 118
- On my local machine, the results of the speci tests are:
|
Thank you for your reply! I've reverted the version changes and commenting-out of "dead" code, as well as added With regards to the failing tests numbers, I've added a Is there anything else you would like? Thanks again! |
|
Wow, passing the same spec-tests is great. I'll review the details. Since this PR is a bit larger, let me take a look at it on the weekend. |
| pub struct DefaultImporter<R: Read, W: Write> { | ||
| /// With `no_std` feature enabled, there is no `DefaultImporter`, it is up to you to create one suitable for your system. | ||
| #[cfg(not(feature = "no_std"))] | ||
| pub struct DefaultImporter<R, W> |
There was a problem hiding this comment.
Question: Why DefaultImporter cannot be implemented when no_std?
| use alloc::fmt; | ||
| use alloc::{borrow::Cow, vec::Vec}; | ||
| #[cfg(feature = "hashbrown")] | ||
| use hashbrown::HashMap; |
There was a problem hiding this comment.
alloc::collections::BTreeMap is available. Can we use BTreeMap instead of introducing a new dependency hashbrown on no_std? Or should we still use hashbrown?
This PR introduces support for environments where the standard library is not available but a global allocator is (i.e. embedded usecase) to the "subcrates" in this repository -
wain-ast,wain-syntax-text,wain-syntax-binary,wain-validateandwain-exec.wain-ast:0.3.0.wain-exec:no_stdnon-default feature.libm- this is only included with theno_stdfeature to make the claim that "no other dependencies are used" true for the default featureset.0.4.0.executefunction andDefaultImporterare not available when theno_stdfeature is enabled - developers who wish to use this feature need to create their ownImporterand use runtime APIs to create and run aRuntimethat uses it.wain-syntax-binary:0.2.0.wain-syntax-text:no_stdnon-default feature.libmandhashbrown- this is only included with theno_stdfeature to make the claim that "no other dependencies are used" true for the default featureset.0.3.0.wain-validate:no_stdnon-default feature.hashbrown- this is only included with theno_stdfeature to make the claim that "no other dependencies are used" true for the default featureset.0.2.0.Additionally, post-commit hooks were failing due to Clippy identified "dead code" in
spec-test. I did not alter the code inspec-testto introduce#![no_std]support however have commented out the code that turned out to be actually dead and added Clippy ignore directives to the code that was being used in the tests but not detected by Clippy, in order to get these checks to pass and resolve all Clippy warnings.If you decide to approve this PR, can you please deploy the new versions of the crates to
crates.ioso that I can use them more easily in my project?Thank you for your time!