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

Crate is not actually no_std #20

Closed
cbiffle opened this issue Feb 2, 2019 · 7 comments · Fixed by #41
Closed

Crate is not actually no_std #20

cbiffle opened this issue Feb 2, 2019 · 7 comments · Fixed by #41

Comments

@cbiffle
Copy link

cbiffle commented Feb 2, 2019

Despite the assertion in the docs, the vek crate at 0.9.6 does not actually build on targets without std available, because of a transitive dependency on std through the approx crate.

Steps to reproduce:

Ensure that the thumbv7em-none-eabihf target is installed:

$ rustup target add thumbv7em-none-eabihf

Clone and build:

$ git clone https://github.com/yoanlcq/vek.git
$ cd vek
$ cargo build --target thumbv7em-none-eabihf

Expected results

Crate builds.

Actual results

    Updating crates.io index
   Compiling semver-parser v0.7.0
   Compiling num-traits v0.2.6
   Compiling num-integer v0.1.39
   Compiling approx v0.1.1
   Compiling static_assertions v0.2.5
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7em-none-eabihf` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `approx`.
warning: build failed, waiting for other jobs to finish...
error: build failed

Likely fix

approx, num-traits, num-integer all need to be set to default-features = false. Unfortunately, the version of num-integer (at least) that's pinned here isn't no_std compatible, so this probably never worked. Upgrading to at least 0.1.36 should fix it.

Also, consider adding a no-std target to your continuous build to catch regressions here. thumbv7em-none-eabihf is a supported target on stable and nightly and would be a reasonable choice, but in general, any target with -none- in the triple instead of e.g. -linux- should do.

@cbiffle
Copy link
Author

cbiffle commented Feb 2, 2019

In case it helps, the minimum versions of vek's dependencies that are no_std compatible appear to be:

  • num-traits: 0.2.0
  • num-integer: 0.1.36
  • approx: 0.2.0

However, on review of the source of the dependencies, a lot of the functionality vek relies on is not available in no_std -- in particular, all of the Float and Real related traits in num-traits. This is likely because a lot of the floating point math routines haven't made it to core.

@yoanlcq
Copy link
Owner

yoanlcq commented Feb 3, 2019

Thanks for the detailed bug report and suggestions! I very much appreciate it. Turns out I was wrong to assume that just putting #![no_std] in my crate was all it takes :)

I'll give it a shot right now.

About this part :

a lot of the functionality vek relies on is not available in no_std -- in particular, all of the Float and Real related traits in num-traits. This is likely because a lot of the floating point math routines haven't made it to core.

This is probably true of Float, OTOH the point of Real was exactly to describe real numbers without assuming any underlying representation, so Real ought to be no_std (if it's not, it should be).
I remember that num-traits's no_std story was a bit complicated (not to blame anyone - there were good reasons for this), but I'll see as I try to fix this.

@yoanlcq
Copy link
Owner

yoanlcq commented Feb 3, 2019

As an aside, it would be great if rustc would generate warnings or errors for this. I wonder how many crates are in a similar situation and if this has been discussed with the Rust devs; but well let's do one thing at a time.

@yoanlcq
Copy link
Owner

yoanlcq commented Feb 3, 2019

The actual issue is here: rust-num/num-traits#75

To me, the true fix is to unconditionally define Real in no_std (the trait itself should not require std (since it doesn't mandate default implementations), nor should the traits it depends on).
The only problem appears to be impl<T: Float> Real for T { ... } because Float does indeed require std.
I'll fiddle with my num-traits fork to get a bigger picture; hopefully this can be fixed.

EDIT: For posterity, Math support in core

@yoanlcq
Copy link
Owner

yoanlcq commented Feb 3, 2019

I've opened a PR on num-traits: rust-num/num-traits#99. If it's accepted, there should be no more obstacles on the way.

That would mean, on no_std operations on Real would be delegated to the libm crate. This will at least allow vek to compile; performance is another matter, but at least you would only need to optimize libm.

@yoanlcq
Copy link
Owner

yoanlcq commented Nov 1, 2019

The PR was merged, but we still need to wait for a new version of num-traits to be pushed with these changes.

@CryZe
Copy link
Contributor

CryZe commented Nov 25, 2019

This is now ready. I have a branch with the changes for vek. I'll open a PR soon.

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

Successfully merging a pull request may close this issue.

3 participants