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

Tracking issue for libcore prelude traits #32110

Closed
alexcrichton opened this issue Mar 8, 2016 · 22 comments
Closed

Tracking issue for libcore prelude traits #32110

alexcrichton opened this issue Mar 8, 2016 · 22 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

These are currently all unstable but part of the libcore prelude.

This is currently intentional, but the fact that they need to be unstable is somewhat unsettling. Arguably all unstable features in libcore should be on the path to deprecation or stabilization at some point.

@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Mar 8, 2016
@Mark-Simulacrum
Copy link
Member

I believe that libcore is now mostly stable, aside from those below. As far as I can tell, none of these are directly in the prelude, though I may be wrong. Perhaps this can be closed?

   1 src/libcore/fmt/mod.rs: fmt_flags_align
   1 src/libcore/hash/mod.rs: sip_hash_13
   1 src/libcore/iter/range.rs: trusted_len
   1 src/libcore/iter/traits.rs: exact_size_is_empty
   1 src/libcore/iter/traits.rs: iter_rfind
   1 src/libcore/macros.rs: concat_idents_macro
   1 src/libcore/marker.rs: unsize
   1 src/libcore/slice/mod.rs: slice_get_slice
   1 src/libcore/sync/atomic.rs: compiler_fences
   1 src/libcore/sync/atomic.rs: future_atomic_orderings
   2 src/libcore/fmt/mod.rs: never_type_impls
   2 src/libcore/hash/mod.rs: i128
   2 src/libcore/iter/sources.rs: trusted_len
   2 src/libcore/iter/traits.rs: fused
   2 src/libcore/iter/traits.rs: trusted_len
   2 src/libcore/ops.rs: question_mark_carrier
   2 src/libcore/option.rs: option_entry
   2 src/libcore/ptr.rs: offset_to
   2 src/libcore/slice/mod.rs: trusted_len
   2 src/libcore/str/mod.rs: str_internals
   3 src/libcore/char.rs: try_from
   3 src/libcore/cmp.rs: reverse_cmp_key
   3 src/libcore/iter/sources.rs: fused
   3 src/libcore/ops.rs: fn_traits
   3 src/libcore/option.rs: fused
   3 src/libcore/option.rs: trusted_len
   3 src/libcore/result.rs: fused
   3 src/libcore/result.rs: trusted_len
   3 src/libcore/slice/mod.rs: sort_unstable
   3 src/libcore/str/mod.rs: str_mut_extras
   4 src/libcore/char.rs: decode_utf8
   4 src/libcore/char.rs: fused
   4 src/libcore/cmp.rs: never_type_impls
   4 src/libcore/convert.rs: try_from
   5 src/libcore/cell.rs: coerce_unsized
   5 src/libcore/char.rs: char_escape_debug
   5 src/libcore/num/mod.rs: try_from
   5 src/libcore/ops.rs: placement_new_protocol
   5 src/libcore/ptr.rs: unique
   6 src/libcore/iter/range.rs: fused
   7 src/libcore/iter/mod.rs: trusted_len
   7 src/libcore/mem.rs: manually_drop
   7 src/libcore/str/mod.rs: fused
   8 src/libcore/hash/sip.rs: sip_hash_13
   9 src/libcore/ptr.rs: shared
  10 src/libcore/ops.rs: coerce_unsized
  10 src/libcore/slice/mod.rs: fused
  10 src/libcore/str/mod.rs: str_checked_slicing
  14 src/libcore/slice/mod.rs: slice_rsplit
  20 src/libcore/iter/mod.rs: fused

@SimonSapin
Copy link
Contributor

@Mark-Simulacrum How did you make this list? It’s missing at least core_slice_ext, core_str_ext, and core_char_ext for the core::slice::SliceExt, core::str::StrExt, and core::char::CharExt traits, which is what this issue is about. They are re-exported from libcore’s prelude, which is different from libstd’s prelude.

@Mark-Simulacrum
Copy link
Member

I can't find the command in my history right now for some reason. I guess I messed up with the ripgrep parameters, though. No need for the list now though, since you've provided it :).

@nagisa
Copy link
Member

nagisa commented May 11, 2017

We certainly should move non-allocating inherent slice methods into libcore as inherent methods, and not the trait mess we’re currently in.

I’m fairly sure we could hack the compiler enough to allow multiple inherent impl blocks across the libcore and libcollections crates with some attribute or something.

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@Razican
Copy link
Contributor

Razican commented Mar 4, 2018

Hello, I'm seeing that the core_float feature redirects here. I found it out when trying to use f32::powi(). Why are float "power" methods not available in stable core? Do they need to allocate? Why?

@SimonSapin
Copy link
Contributor

@alexcrichton Should core::num::Float be added to core::prelude::v1, following the same pattern as CharExt for example? This would make methods like powi available to no_std crates.

@alexcrichton
Copy link
Member Author

In general yes, functionality stable in libstd is fine to ensure is available in libcore as well. Looking at the trait it also has a stable Bits associated constant though, which means the entire trait is probably not ready for stable reexportation.

@Razican
Copy link
Contributor

Razican commented Mar 5, 2018

I also see that for the cos(), sin(), sin_cos() and sqrt() functions in floats only use intrinsics readily available in core_intrinsics. Maybe those could be stabilized as well, right?

@alexcrichton
Copy link
Member Author

Those are ones we'd specifically actually not want to stabilize as well because they require runtime support. Their definitions in libcore should likely be removed.

@michael-p
Copy link

So if one needs to use sin, cos etc on an embedded device (with implementations provided by libc/libm) one has to use core::intrinsics::sinf32 and friends, right? Or is there another (better) way of doing this, since core_intrinsics is probably never going to be stable? (meaning using trigonometric functions in the embedded world will never be stable)

@Razican
Copy link
Contributor

Razican commented Mar 6, 2018

I was thinking on porting parts of libm to Rust using corrode or something similar, since I also would like to have atan(), asin() and acos() too, and they use cmath in the standard library. I still don't know if I could make them work without heap allocation, though.

I ported most of some code to use core::intrinsics::sinf32 and friends, but @alexcrichton said they should likely be removed, so it's probably not a good idea to depend on them. I didn't try to compile them to bare metal, even though they compiled fine using #[no_std].

I haven't started any work on sqrt().

@alexcrichton
Copy link
Member Author

@Razican for sure yeah it's useful to have in libcore but it should be explicitly improted. Something like core::intrinsics::sinf32 just calls sin, so that's why it's not exported.

@SimonSapin
Copy link
Contributor

not want to stabilize as well because they require runtime support

@alexcrichton Does this also apply to the abs, signum, and powi methods of core::num::Float? Their impls are based on intrinsics.

@alexcrichton
Copy link
Member Author

@SimonSapin yes ideally anything requiring runtime support is only stabilized in libstd, although that ship may have already sailed.

@SimonSapin
Copy link
Contributor

The Float trait is unstable and not in the prelude, so… maybe not?

To be clear, what does "runtime support" mean here exactly? You seem to be suggesting it includes anything that calls into an LLVM intrinsic but as far as I can tell plenty of things in libcore do that.

@alexcrichton
Copy link
Member Author

To me "runtime support" is "on any platform this lowers to a call to libm"

@SimonSapin
Copy link
Contributor

Sounds good. Do you know how I can find out which intrinsics can lower to calls to libm?

@alexcrichton
Copy link
Member Author

Unfortunately I'm not sure there's a great way other than "compile it for a bunch of platforms and see what LLVM does"

@SimonSapin
Copy link
Contributor

The relevant intrinsics are:

  • llvm.fabs.f32
  • llvm.fabs.f64
  • llvm.copysign.f32
  • llvm.copysign.f64
  • llvm.powi.f32
  • llvm.powi.f32

The compiler-builtins crate has Rust implementations of __powisf2 and __powidf2, but in LLVM code those are only mentioned in lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp.

@SimonSapin
Copy link
Contributor

#27823 moved a number of float methods (back) from libcore to libstd, but specifically left abs, signum and powi in libcore but doesn’t say why. (And the author hasn’t been active on github for several years.)

@SimonSapin
Copy link
Contributor

#49896 replaces these traits with inherent methods.

SimonSapin added a commit to SimonSapin/rust that referenced this issue Apr 13, 2018
… previously in the unstable core::num::Float trait.

Per rust-lang#32110 (comment),
the `abs`, `signum`, and `powi` methods are *not* included for now
since they rely on LLVM intrinsics and we haven’t determined yet whether
those instrinsics lower to calls to libm functions on any platform.
SimonSapin added a commit to SimonSapin/rust that referenced this issue Apr 13, 2018
bors added a commit that referenced this issue Apr 14, 2018
Add inherent methods in libcore for [T], [u8], str, f32, and f64

# Background

Primitive types are defined by the language, they don’t have a type definition like `pub struct Foo { … }` in any crate. So they don’t “belong” to any crate as far as `impl` coherence is concerned, and on principle no crate would be able to define inherent methods for them, without a trait. Since we want these types to have inherent methods anyway, the standard library (with cooperation from the compiler) bends this rule with code like [`#[lang = "u8"] impl u8 { /*…*/ }`](https://github.com/rust-lang/rust/blob/1.25.0/src/libcore/num/mod.rs#L2244-L2245). The `#[lang]` attribute is permanently-unstable and never intended to be used outside of the standard library.

Each lang item can only be defined once. Before this PR there is one impl-coherence-rule-bending lang item per primitive type (plus one for `[u8]`, which overlaps with `[T]`). And so one `impl` block each. These blocks for `str`, `[T]` and `[u8]` are in liballoc rather than libcore because *some* of the methods (like `<[T]>::to_vec(&self) -> Vec<T> where T: Clone`) need a global memory allocator which we don’t want to make a requirement in libcore. Similarly, `impl f32` and `impl f64` are in libstd because some of the methods are based on FFI calls to C’s `libm` and we want, as much as possible, libcore not to require “runtime support”.

In libcore, the methods of `str` and `[T]` that don’t allocate are made available through two **unstable traits** `StrExt` and `SliceExt` (so the traits can’t be *named* by programs on the Stable release channel) that have **stable methods** and are re-exported in the libcore prelude (so that programs on Stable can *call* these methods anyway). Non-allocating `[u8]` methods are not available in libcore: #45803. Some `f32` and `f64` methods are in an unstable `core::num::Float` trait with stable methods, but that one is **not in the libcore prelude**. (So as far as Stable programs are concerns it doesn’t exist, and I don’t know what the point was to mark these methods `#[stable]`.)

#32110 is the tracking issue for these unstable traits.

# High-level proposal

Since the standard library is already bending the rules, why not bend them *a little more*? By defining a few additional lang items, the compiler can allow the standard library to have *two* `impl` blocks (in different crates) for some primitive types.

The `StrExt` and `SliceExt` traits still exist for now so that we can bootstrap from a previous-version compiler that doesn’t have these lang items yet, but they can be removed in next release cycle. (`Float` is used internally and needs to be public for libcore unit tests, but was already `#[doc(hidden)]`.) I don’t know if #32110 should be closed by this PR, or only when the traits are entirely removed after we make a new bootstrap compiler.

# Float methods

Among the methods of the `core::num::Float` trait, three are based on LLVM intrinsics: `abs`, `signum`, and `powi`. PR #27823 “Remove dependencies on libm functions from libcore” moved a bunch of `core::num::Float` methods back to libstd, but left these three behind. However they aren’t specifically discussed in the PR thread. The `compiler_builtins` crate defines `__powisf2` and `__powidf2` functions that look like implementations of `powi`, but I couldn’t find a connection with the `llvm.powi.f32` and `llvm.powi.f32` intrinsics by grepping through LLVM’s code.

In discussion starting at #32110 (comment) Alex says that we do not want methods in libcore that require “runtime support”, but it’s not clear whether that applies to these `abs`, `signum`, or `powi`. In doubt, I’ve **removed** them for the trait and moved them to inherent methods in libstd for now. We can move them back later (or in this PR) if we decide that’s appropriate.

# Change details

For users on the Stable release channel:

* I believe this PR does not make any breaking change
* Some methods for `[u8]`, `f32`, and `f64` are newly available to `#![no_std]` users (fixes #45803)
* There should be no visible change for `std` users in terms of what programs compile or what their behavior is. (Only in compiler error messages, possibly.)

For Nightly users, additionally:

* The unstable `StrExt` and `SliceExt` traits are gone
* Their methods are now inherent methods of `str` and `[T]` (so only code that explicitly named the traits should be affected, not "normal" method calls)
* The `abs`, `signum` and `powi` methods of the `Float` trait are gone
* The `Float` trait’s unstable feature name changed to `float_internals` with no associated tracking issue, to reflect it being a permanently unstable implementation detail rather than a public API on a path to stabilization.
* Its remaining methods are now inherent methods of `f32` and `f64`.

-----

CC @rust-lang/libs for the API changes, @rust-lang/compiler for the new lang items
SimonSapin added a commit to SimonSapin/rust that referenced this issue Apr 21, 2018
… previously in the unstable core::num::Float trait.

Per rust-lang#32110 (comment),
the `abs`, `signum`, and `powi` methods are *not* included for now
since they rely on LLVM intrinsics and we haven’t determined yet whether
those instrinsics lower to calls to libm functions on any platform.
SimonSapin added a commit to SimonSapin/rust that referenced this issue Apr 21, 2018
bors added a commit that referenced this issue Apr 22, 2018
Add inherent methods in libcore for [T], [u8], str, f32, and f64

# Background

Primitive types are defined by the language, they don’t have a type definition like `pub struct Foo { … }` in any crate. So they don’t “belong” to any crate as far as `impl` coherence is concerned, and on principle no crate would be able to define inherent methods for them, without a trait. Since we want these types to have inherent methods anyway, the standard library (with cooperation from the compiler) bends this rule with code like [`#[lang = "u8"] impl u8 { /*…*/ }`](https://github.com/rust-lang/rust/blob/1.25.0/src/libcore/num/mod.rs#L2244-L2245). The `#[lang]` attribute is permanently-unstable and never intended to be used outside of the standard library.

Each lang item can only be defined once. Before this PR there is one impl-coherence-rule-bending lang item per primitive type (plus one for `[u8]`, which overlaps with `[T]`). And so one `impl` block each. These blocks for `str`, `[T]` and `[u8]` are in liballoc rather than libcore because *some* of the methods (like `<[T]>::to_vec(&self) -> Vec<T> where T: Clone`) need a global memory allocator which we don’t want to make a requirement in libcore. Similarly, `impl f32` and `impl f64` are in libstd because some of the methods are based on FFI calls to C’s `libm` and we want, as much as possible, libcore not to require “runtime support”.

In libcore, the methods of `str` and `[T]` that don’t allocate are made available through two **unstable traits** `StrExt` and `SliceExt` (so the traits can’t be *named* by programs on the Stable release channel) that have **stable methods** and are re-exported in the libcore prelude (so that programs on Stable can *call* these methods anyway). Non-allocating `[u8]` methods are not available in libcore: #45803. Some `f32` and `f64` methods are in an unstable `core::num::Float` trait with stable methods, but that one is **not in the libcore prelude**. (So as far as Stable programs are concerns it doesn’t exist, and I don’t know what the point was to mark these methods `#[stable]`.)

#32110 is the tracking issue for these unstable traits.

# High-level proposal

Since the standard library is already bending the rules, why not bend them *a little more*? By defining a few additional lang items, the compiler can allow the standard library to have *two* `impl` blocks (in different crates) for some primitive types.

The `StrExt` and `SliceExt` traits still exist for now so that we can bootstrap from a previous-version compiler that doesn’t have these lang items yet, but they can be removed in next release cycle. (`Float` is used internally and needs to be public for libcore unit tests, but was already `#[doc(hidden)]`.) I don’t know if #32110 should be closed by this PR, or only when the traits are entirely removed after we make a new bootstrap compiler.

# Float methods

Among the methods of the `core::num::Float` trait, three are based on LLVM intrinsics: `abs`, `signum`, and `powi`. PR #27823 “Remove dependencies on libm functions from libcore” moved a bunch of `core::num::Float` methods back to libstd, but left these three behind. However they aren’t specifically discussed in the PR thread. The `compiler_builtins` crate defines `__powisf2` and `__powidf2` functions that look like implementations of `powi`, but I couldn’t find a connection with the `llvm.powi.f32` and `llvm.powi.f32` intrinsics by grepping through LLVM’s code.

In discussion starting at #32110 (comment) Alex says that we do not want methods in libcore that require “runtime support”, but it’s not clear whether that applies to these `abs`, `signum`, or `powi`. In doubt, I’ve **removed** them for the trait and moved them to inherent methods in libstd for now. We can move them back later (or in this PR) if we decide that’s appropriate.

# Change details

For users on the Stable release channel:

* I believe this PR does not make any breaking change
* Some methods for `[u8]`, `f32`, and `f64` are newly available to `#![no_std]` users (fixes #45803)
* There should be no visible change for `std` users in terms of what programs compile or what their behavior is. (Only in compiler error messages, possibly.)

For Nightly users, additionally:

* The unstable `StrExt` and `SliceExt` traits are gone
* Their methods are now inherent methods of `str` and `[T]` (so only code that explicitly named the traits should be affected, not "normal" method calls)
* The `abs`, `signum` and `powi` methods of the `Float` trait are gone
* The `Float` trait’s unstable feature name changed to `float_internals` with no associated tracking issue, to reflect it being a permanently unstable implementation detail rather than a public API on a path to stabilization.
* Its remaining methods are now inherent methods of `f32` and `f64`.

-----

CC @rust-lang/libs for the API changes, @rust-lang/compiler for the new lang items
@SimonSapin
Copy link
Contributor

I’ve filed #50145 specifically for f32 and f64 methods in libcore.

SimonSapin added a commit to SimonSapin/rust that referenced this issue May 22, 2018
bors added a commit that referenced this issue May 23, 2018
Remove the unstable Float trait

Following up to #49896 and #50629. Fixes #32110.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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

6 participants