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.

Sign up for GitHub

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

Jump to bottom

Implement actual support for no_std #372

Merged
merged 2 commits into from
Nov 25, 2019
Merged

Implement actual support for no_std #372

merged 2 commits into from
Nov 25, 2019

Conversation

CryZe
Copy link
Contributor

@CryZe CryZe commented Oct 4, 2019 •
edited
Loading

With num-traits having recently gained full support for no_std by using the libm crate, this crate can now fully support no_std as well without any sacrifices.

Currently num-traits has not released a new version yet, so this uses a git dependency until then.

Additionally the tests should fail at the moment when using the libm, as there actually was a bug in libm, but I have created a PR for it.

This change is Reviewable

@CryZe
Implement actual support for no_std
021b9bb 
With num-traits having recently gained full support for no_std by using
the libm crate, this crate can now fully support no_std as well without
any sacrifices.

Currently num-traits has not released a new version yet, so this uses a
git dependency until then.

Additionally the tests should fail at the moment when using the libm, as
there actually was a bug in libm that I have created a PR for.
kvark
kvark approved these changes Oct 4, 2019
View reviewed changes
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CryZe CryZe mentioned this pull request Oct 6, 2019
Closed
26 tasks
@CryZe
Copy link
Contributor Author

CryZe commented Nov 18, 2019

So this is done, but the one test fails because sincosf was broken in Rust's libm port. However this has since been fixed, but no new version of libm has been published. rust-lang/libm#229

Not sure how you want me to proceed here.

@nical
Copy link
Contributor

nical commented Nov 18, 2019

Thanks! My preference would be to wait for the the new libm to be published and depend on the fixed revision in this PR.

@nical
Copy link
Contributor

nical commented Nov 18, 2019

I filed rust-lang/libm#231

@CryZe
Copy link
Contributor Author

CryZe commented Nov 21, 2019

libm 0.2.0 is released, I just opened a PR against num-traits: rust-num/num-traits#144

@nical
Copy link
Contributor

nical commented Nov 25, 2019

Looks like your PR to num-traits with the new libm made it to crates.io in 0.2.10.

@CryZe
Use published num-traits crate
7ce307b 
Version 0.2.10 of num-traits now includes the no-std support.
@CryZe
Copy link
Contributor Author

CryZe commented Nov 25, 2019

Alright, I bumped the version and it seems to work now.

@nical
Copy link
Contributor

nical commented Nov 25, 2019

Thanks for your persistence!

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 7ce307b has been approved by nical

bors-servo pushed a commit that referenced this pull request Nov 25, 2019
Auto merge of #372 - CryZe:actual-no-std, r=nical
a53be3d 
Implement actual support for no_std

With num-traits having recently gained full support for no_std by using the libm crate, this crate can now fully support no_std as well without any sacrifices.

Currently num-traits has not released a new version yet, so this uses a git dependency until then.

Additionally the tests should fail at the moment when using the libm, as there actually was a bug in libm, but I have created a PR for it.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/372)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

⌛ Testing commit 7ce307b with merge a53be3d...

@nical nical mentioned this pull request Nov 25, 2019
Closed
@bors-servo
Copy link
Contributor

☀️ Test successful - checks-travis
Approved by: nical
Pushing a53be3d to master...

@bors-servo bors-servo merged commit 7ce307b into servo:master Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvark kvark kvark approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CryZe @nical @bors-servo @kvark