Skip to content

Add target_float #31795

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

Closed
wants to merge 2 commits into from
Closed

Add target_float #31795

wants to merge 2 commits into from

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Feb 20, 2016

Adds target_float as suggested in rust-lang/rfcs#1364

@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

pub mod flt2dec;
#[cfg_attr(not(stage0), cfg(target_float))]
pub mod dec2flt;
pub mod bignum;
pub mod diy_float;
Copy link
Contributor

Choose a reason for hiding this comment

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

If flt2dec and dec2flt are cfg'd out, then bignum and diy_float can also be cfg'd out..

@Zoxc
Copy link
Contributor Author

Zoxc commented Feb 20, 2016

@rkruppe Tests require libtest which require librand and libstd which require floats, so that won't be very beneficial until libstd can build without floats.

@hanna-kruppe
Copy link
Contributor

Oh, right, sorry!

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 21, 2016
@alexcrichton
Copy link
Member

The libs team discussed this during triage yesterday, and we reached a few conclusions

  • There... are many overlapping concerns on this PR. It's ended up being unclear that this was even a libs team PR.
  • This PR adds support for a target_float #[cfg] directive defined in custom target specs, but it only goes halfway. The language still defines the types and they're still available for use.
  • In terms of an API, we likely want to consider this in conjunction with architecture-dependent types. For example AtomicU64 might also be an architecture-dependent type. We've long wanted a mechanism for this, but haven't yet designed one.

For now we concluded that this PR may be a bit premature. There are enough concerns here (and this is touching core-enough types) that this likely wants to go through an RFC before a PR. Thanks regardless though for the PR @Zoxc!

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

Successfully merging this pull request may close these issues.

5 participants