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

Remove trait LengthAtMost32 #74060

Merged
merged 5 commits into from
Jul 26, 2020
Merged

Conversation

kpp
Copy link
Contributor

@kpp kpp commented Jul 5, 2020

This is a continuation of #74026 preserving the original burrbull's commit.

I talked to @burrbull, he suggested me to finish his PR.

@rust-highfive
Copy link
Collaborator

r? @dtolnay

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 5, 2020
@lcnr
Copy link
Contributor

lcnr commented Jul 5, 2020

One thing which concerns me is Default for arrays, which afaik we can't implement using const generics rn
as the current Default impl does not have a T: Default bound for N == 0.

So I fear that we will end up with Default being the only trait which is not implemented for all arrays.

@kpp
Copy link
Contributor Author

kpp commented Jul 5, 2020

I suppose we can have impl<T, const N: usize> ConstDefault for [T; N] with #49147 and https://github.com/Centril/rfcs/blob/rfc/const-default/text/0000-const-default.md

@withoutboats withoutboats added A-macros-2.0 Area: Declarative macros 2.0 (#39412) T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 5, 2020
@withoutboats
Copy link
Contributor

@rfcbot fcp merge

I propose we merge this, making it so that all traits in std implemented for arrays except Default be implemented for every length of array.

  • These traits have been implemented on top of const generics for over a year, demonstrating that the implementation is solid and reliable.
  • We have universal consensus that we want a form of const generics that could support at least all of these impls. There is no concern that we are committing ourselves to a feature we would not be able to support in the long term.
  • Since we know the implementation is solid and that we are going to stabilize this functionality at some point, we should provide this functionality to users sooner, rather than later.

I also think we should have a discussion about an MVP of const generics which is solid enough that we could stabilize it in the near term, but that's a separate discussion.

@rfcbot
Copy link

rfcbot commented Jul 5, 2020

Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 5, 2020
@crlf0710 crlf0710 added A-const-generics Area: const generics (parameters and arguments) F-const_generics `#![feature(const_generics)]` labels Jul 5, 2020
@withoutboats withoutboats removed A-macros-2.0 Area: Declarative macros 2.0 (#39412) F-const_generics `#![feature(const_generics)]` labels Jul 5, 2020
@ollie27
Copy link
Member

ollie27 commented Jul 5, 2020

The docs will also need to be updated, for example:

/// Arrays of sizes from 0 to 32 (inclusive) implement the following traits if
/// the element type allows it:
///
/// - [`Debug`][debug]
/// - [`IntoIterator`][intoiterator] (implemented for `&[T; N]` and `&mut [T; N]`)
/// - [`PartialEq`][partialeq], [`PartialOrd`][partialord], [`Eq`][eq], [`Ord`][ord]
/// - [`Hash`][hash]
/// - [`AsRef`][asref], [`AsMut`][asmut]
/// - [`Borrow`][borrow], [`BorrowMut`][borrowmut]
/// - [`Default`][default]
///
/// This limitation on the size `N` exists because Rust does not yet support
/// code that is generic over the size of an array type. `[Foo; 3]` and `[Bar; 3]`
/// are instances of same generic type `[T; 3]`, but `[Foo; 3]` and `[Foo; 5]` are
/// entirely different types. As a stopgap, trait implementations are
/// statically generated up to size 32.
///
/// Arrays of *any* size are [`Copy`][copy] if the element type is [`Copy`][copy]
/// and [`Clone`][clone] if the element type is [`Clone`][clone]. This works
/// because [`Copy`][copy] and [`Clone`][clone] traits are specially known
/// to the compiler.

@joshtriplett joshtriplett added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 5, 2020
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 6, 2020
@rfcbot
Copy link

rfcbot commented Jul 6, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 6, 2020
@withoutboats withoutboats added the F-const_generics `#![feature(const_generics)]` label Jul 6, 2020
@bors
Copy link
Contributor

bors commented Jul 26, 2020

📌 Commit 8fc7d47 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2020
@bors
Copy link
Contributor

bors commented Jul 26, 2020

⌛ Testing commit 8fc7d47 with merge 461707c...

@bors
Copy link
Contributor

bors commented Jul 26, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: dtolnay
Pushing 461707c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 26, 2020
@bors bors merged commit 461707c into rust-lang:master Jul 26, 2020
@kpp kpp deleted the remove_length_at_most_32 branch July 26, 2020 07:57
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 13, 2020
Pkgsrc changes:
 * Remove patches now integrated upstream, many related to SunOS / Illumos.
 * The LLVM fix for powerpc is also now integrated upstream.
 * Adapt those patches where the source has moved or parts are integrated.
 * The randomness patches no longer applies, and I could not find
   where those files went...
 * Provide a separate bootstrap for NetBSD/powerpc 9.0, since apparently
   the C++ ABI is different from 8.0.  Yes, this appears to be specific to
   the NetBSD powerpc ports.

Upstream changes:

Version 1.47.0 (2020-10-08)
==========================

Language
--------
- [Closures will now warn when not used.][74869]

Compiler
--------
- [Stabilized the `-C control-flow-guard` codegen option][73893], which enables
  [Control Flow Guard][1.47.0-cfg] for Windows platforms, and is ignored on
  other platforms.
- [Upgraded to LLVM 11.][73526]
- [Added tier 3\* support for the `thumbv4t-none-eabi` target.][74419]
- [Upgrade the FreeBSD toolchain to version 11.4][75204]
- [`RUST_BACKTRACE`'s output is now more compact.][75048]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`CStr` now implements `Index<RangeFrom<usize>>`.][74021]
- [Traits in `std`/`core` are now implemented for arrays of any length, not just
  those of length less than 33.][74060]
- [`ops::RangeFull` and `ops::Range` now implement Default.][73197]
- [`panic::Location` now implements `Copy`, `Clone`, `Eq`, `Hash`, `Ord`,
  `PartialEq`, and `PartialOrd`.][73583]

Stabilized APIs
---------------
- [`Ident::new_raw`]
- [`Range::is_empty`]
- [`RangeInclusive::is_empty`]
- [`Result::as_deref`]
- [`Result::as_deref_mut`]
- [`Vec::leak`]
- [`pointer::offset_from`]
- [`f32::TAU`]
- [`f64::TAU`]

The following previously stable APIs have now been made const.

- [The `new` method for all `NonZero` integers.][73858]
- [The `checked_add`,`checked_sub`,`checked_mul`,`checked_neg`, `checked_shl`,
  `checked_shr`, `saturating_add`, `saturating_sub`, and `saturating_mul`
  methods for all integers.][73858]
- [The `checked_abs`, `saturating_abs`, `saturating_neg`, and `signum`  for all
  signed integers.][73858]
- [The `is_ascii_alphabetic`, `is_ascii_uppercase`, `is_ascii_lowercase`,
  `is_ascii_alphanumeric`, `is_ascii_digit`, `is_ascii_hexdigit`,
  `is_ascii_punctuation`, `is_ascii_graphic`, `is_ascii_whitespace`, and
  `is_ascii_control` methods for `char` and `u8`.][73858]

Cargo
-----
- [`build-dependencies` are now built with opt-level 0 by default.][cargo/8500]
  You can override this by setting the following in your `Cargo.toml`.
  ```toml
  [profile.release.build-override]
  opt-level = 3
  ```
- [`cargo-help` will now display man pages for commands rather just the
  `--help` text.][cargo/8456]
- [`cargo-metadata` now emits a `test` field indicating if a target has
  tests enabled.][cargo/8478]
- [`workspace.default-members` now respects `workspace.exclude`.][cargo/8485]
- [`cargo-publish` will now use an alternative registry by default if it's the
  only registry specified in `package.publish`.][cargo/8571]

Misc
----
- [Added a help button beside Rustdoc's searchbar that explains rustdoc's
  type based search.][75366]
- [Added the Ayu theme to rustdoc.][71237]

Compatibility Notes
-------------------
- [Bumped the minimum supported Emscripten version to 1.39.20.][75716]
- [Fixed a regression parsing `{} && false` in tail expressions.][74650]
- [Added changes to how proc-macros are expanded in `macro_rules!` that should
  help to preserve more span information.][73084] These changes may cause
  compiliation errors if your macro was unhygenic or didn't correctly handle
  `Delimiter::None`.
- [Moved support for the CloudABI target to tier 3.][75568]
- [`linux-gnu` targets now require minimum kernel 2.6.32 and glibc 2.11.][74163]

Internal Only
--------
- [Improved default settings for bootstrapping in `x.py`.][73964]
  You can read details about this change in the ["Changes to `x.py`
  defaults"](https://blog.rust-lang.org/inside-rust/2020/08/30/changes-to-x-py-defaults.html)
  post on the Inside Rust blog.

- [Added the `rustc-docs` component.][75560] This allows you to install
  and read the documentation for the compiler internal APIs. (Currently only
  available for `x86_64-unknown-linux-gnu`.)

[1.47.0-cfg]: https://docs.microsoft.com/en-us/windows/win32/secbp/control-flow-guard
[76980]: rust-lang/rust#76980
[75048]: rust-lang/rust#75048
[74163]: rust-lang/rust#74163
[71237]: rust-lang/rust#71237
[74869]: rust-lang/rust#74869
[73858]: rust-lang/rust#73858
[75716]: rust-lang/rust#75716
[75908]: rust-lang/rust#75908
[75516]: rust-lang/rust#75516
[75560]: rust-lang/rust#75560
[75568]: rust-lang/rust#75568
[75366]: rust-lang/rust#75366
[75204]: rust-lang/rust#75204
[74650]: rust-lang/rust#74650
[74419]: rust-lang/rust#74419
[73964]: rust-lang/rust#73964
[74021]: rust-lang/rust#74021
[74060]: rust-lang/rust#74060
[73893]: rust-lang/rust#73893
[73526]: rust-lang/rust#73526
[73583]: rust-lang/rust#73583
[73084]: rust-lang/rust#73084
[73197]: rust-lang/rust#73197
[72488]: rust-lang/rust#72488
[cargo/8456]: rust-lang/cargo#8456
[cargo/8478]: rust-lang/cargo#8478
[cargo/8485]: rust-lang/cargo#8485
[cargo/8500]: rust-lang/cargo#8500
[cargo/8571]: rust-lang/cargo#8571
[`Ident::new_raw`]:  https://doc.rust-lang.org/nightly/proc_macro/struct.Ident.html#method.new_raw
[`Range::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.Range.html#method.is_empty
[`RangeInclusive::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.RangeInclusive.html#method.is_empty
[`Result::as_deref_mut`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref_mut
[`Result::as_deref`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref
[`TypeId::of`]: https://doc.rust-lang.org/nightly/std/any/struct.TypeId.html#method.of
[`Vec::leak`]: https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.leak
[`f32::TAU`]: https://doc.rust-lang.org/nightly/std/f32/consts/constant.TAU.html
[`f64::TAU`]: https://doc.rust-lang.org/nightly/std/f64/consts/constant.TAU.html
[`pointer::offset_from`]: https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.offset_from
Xiretza added a commit to Xiretza/ghdl that referenced this pull request Feb 23, 2021
This bumps the MSRV to 1.47.0 due to rust#74060[0]

[0]: rust-lang/rust#74060
emilio pushed a commit to rust-lang/rust-bindgen that referenced this pull request Jul 16, 2021
@elichai
Copy link
Contributor

elichai commented Sep 14, 2021

I just want to note that now the "stable since" annotation on this traits is confusing,
I tried using <[u8; 64]>::try_from(slice) on rust 1.36, assuming it would work because it says stable since "1.34", but it's not because removing the "at most 32" didn't change the stable annotation.

@SimonSapin
Copy link
Contributor

SimonSapin commented Sep 19, 2021

There isn’t an easy fix for that, unfortunately. There’s now a single impl for all lengths and it can only have a single "stable since" attribute. Just changing it to 1.47 would make it incorrect for lengths up to 32.

@SimonSapin
Copy link
Contributor

Though this all could be explained in a doc-comment. I’m sure a PR adding that would be welcome.

emilio added a commit to emilio/rust that referenced this pull request Apr 19, 2024
…r than 32 elements.

Much like rust-lang#74060 did for every other trait.

This avoids tools like rust-bindgen having to painfully work around this
limitations, sometimes incorrectly like in
rust-lang/rust-bindgen#2803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-const_generics `#![feature(const_generics)]` finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. 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.