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

no_std does not impl sqrt for f32 and f64 #39473

Closed
CasualX opened this issue Feb 2, 2017 · 15 comments
Closed

no_std does not impl sqrt for f32 and f64 #39473

CasualX opened this issue Feb 2, 2017 · 15 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@CasualX
Copy link

CasualX commented Feb 2, 2017

I tried the following code (compiled as a library):

#![no_std]
pub trait Float {
    fn sqrt(self) -> Self;
}
impl Float for f32 {
    fn sqrt(self) -> Self { self.sqrt() }
}
impl Float for f64 {
    fn sqrt(self) -> Self { self.sqrt() }
}

I expected to see this happen: The above code working and calling the correct sqrt function.

The core docs for the f32 primitive type 404s and unlike the integer types does not contain a link to f32's primitive type info page.

Instead this happened:

warning: function cannot return without recurring, #[warn(unconditional_recursion)] on by default
  --> src\unit.rs:58:2
   |
58 |    fn sqrt(self) -> f32 { self.sqrt() }
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: recursive call site
  --> src\unit.rs:58:25
   |
58 |    fn sqrt(self) -> f32 { self.sqrt() }
   |                           ^^^^^^^^^^^
   = help: a `loop` may express intention better if this is on purpose

warning: function cannot return without recurring, #[warn(unconditional_recursion)] on by default
  --> src\unit.rs:61:2
   |
61 |    fn sqrt(self) -> f64 { self.sqrt() }
   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: recursive call site
  --> src\unit.rs:61:25
   |
61 |    fn sqrt(self) -> f64 { self.sqrt() }
   |                           ^^^^^^^^^^^
   = help: a `loop` may express intention better if this is on purpose

Is this intended?

Meta

rustc --version --verbose:

rustc 1.14.0 (e8a012324 2016-12-16)
binary: rustc
commit-hash: e8a0123241f0d397d39cd18fcc4e5e7edde22730
commit-date: 2016-12-16
host: x86_64-pc-windows-msvc
release: 1.14.0
LLVM version: 3.9
@nagisa
Copy link
Member

nagisa commented Feb 2, 2017

Yes this is intended I think. sqrt() depends on libraries in platform in some cases and the whole point of core is to not depend on those.

@durka
Copy link
Contributor

durka commented Feb 2, 2017

@nagisa the implementation of f32::sqrt and f64::sqrt in std seem to just call intrinsics.

@FenrirWolf
Copy link
Contributor

From what I understand, many of those intrinsics get lowered to functions whose definitions usually live in libm.

@nagisa
Copy link
Member

nagisa commented Feb 3, 2017

What @FenrirWolf said is correct.

@CasualX
Copy link
Author

CasualX commented Feb 3, 2017

That's unfortunate but understandable.

@durka
Copy link
Contributor

durka commented Feb 3, 2017 via email

@FenrirWolf
Copy link
Contributor

Better yet would be implementations of those functions in Rust, similar to the compiler-builtins crate for compiler-rt intrinsics. There is a work-in-progress port of openlibm with this goal in mind.

@anatol
Copy link
Contributor

anatol commented Feb 12, 2017

@FenrirWolf big plus from me. math functions like those defined in libm can be fully implemented in Rust. There is no strong reason to link to libm C library.

@nagisa
Copy link
Member

nagisa commented Feb 13, 2017

There’s no strong reason to include these in no_std builds either – you can link to software impls yourselves just fine by depending on a crate like math.rs or the already mentioned m.

@anatol
Copy link
Contributor

anatol commented Feb 13, 2017

@nagisa thanks for the pointer to your math.rs crate. It looks like it is the most complete math functions implementation in Rust.

For me it would make a lot of sense to replace existing Rust libm-based implementation with yours from math.rs. Having math library implemented fully in Rust is a great step towards C-free libstd.

@Mark-Simulacrum Mark-Simulacrum added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 22, 2017
@Mark-Simulacrum
Copy link
Member

I propose that we close this issue since this is something that is intentional for now; should we ever move to Rust implementations of math functions, most likely all functions on f32/f64 should be reevaluated for inclusion in core. cc @rust-lang/libs

@CasualX
Copy link
Author

CasualX commented May 23, 2017

I completely forgot about this issue! I agree with Mark, the subject seems settled that for now this is expected behaviour due to how core works.

@jhpratt
Copy link
Member

jhpratt commented Sep 25, 2019

Any updates on this? Documentation at a minimum would be great, since it's a very odd issue to run into given the present error message.

Some of the functions, like .abs(), .signum(), etc. seem relatively trivial to rewrite in Rust. Is this something that is desired? I'd be up for a PR if so.

@Mark-Simulacrum?

@Mark-Simulacrum
Copy link
Member

This is pretty hard -- I don't have a link handy, but there are a bunch of considerations that need to go into doing this correctly. I believe some folks are working on it though... I recall a PR or two flying by in the past couple weeks.

@lvella
Copy link

lvella commented Jul 6, 2023

The current open issue for this is #50145.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants