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

Add std::hash::{DefaultHasher, RandomState} exports (needs FCP) #115694

Merged
merged 2 commits into from Nov 11, 2023

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented Sep 9, 2023

This implements rust-lang/libs-team#267 to move the libstd hasher types to std::hash where they belong, instead of std::collections::hash_map.

The below no longer applies, but is kept for clarity. This is a small refactor for #27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API.

I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of std::hash, but for now, they're at least separated out from the collections code in a way that will make moving that around easier.

I decided to simply copy the rustdoc for std::hash from core::hash since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred.

@rustbot
Copy link
Collaborator

rustbot commented Sep 9, 2023

r? @thomcc

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 9, 2023
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@clarfonthey clarfonthey force-pushed the std-hash-private branch 2 times, most recently from 12520ae to 26d14ac Compare September 9, 2023 04:12
@rust-log-analyzer

This comment has been minimized.

@clarfonthey
Copy link
Contributor Author

clarfonthey commented Oct 4, 2023

Since the ACP got accepted, it's up to the reviewer whether we make this have an FCP here to add new aliases or save that for another PR. (I'm fine with either.)

I believe that re-exports can't have separate stability flags, but I could be wrong, in which case I can unstably export them in std::hash and open a tracking issue.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 13, 2023
@rustbot
Copy link
Collaborator

rustbot commented Oct 13, 2023

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/rust-analyzer

cc @rust-lang/rust-analyzer

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

These commits modify compiler targets.
(See the Target Tier Policy.)

@clarfonthey
Copy link
Contributor Author

(Apologies for the @ mentions; I removed the subtree commits.)

@clarfonthey
Copy link
Contributor Author

I checked, and yes, aliases are insta-stable (you can mark them as unstable, but it won't actually require the feature flag to use them).

I decided I'd just go ahead with making the aliases and we can either do an FCP, or we can remove the second commit. I also tried to replace code in the tree that used the old exports, although as you can see, I accidentally included some subtrees that are now removed.

@clarfonthey clarfonthey changed the title Move RandomState and DefaultHasher into std::hash, but in a sneaky way Add std::hash::{DefaultHasher, RandomState} exports (needs FCP) Oct 13, 2023
@clarfonthey
Copy link
Contributor Author

@rustbot label needs-FCP

@rustbot rustbot added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Oct 13, 2023
@rust-log-analyzer

This comment has been minimized.

@clarfonthey clarfonthey force-pushed the std-hash-private branch 2 times, most recently from eb851d4 to 4bee554 Compare October 13, 2023 06:44
@clarfonthey
Copy link
Contributor Author

Gentle nudge @thomcc (or any libs-team members around) to start FCP for merging these aliases. Unfortunately we can't make aliases have a separate stability attribute from what I've gathered.

@thomcc
Copy link
Member

thomcc commented Oct 26, 2023

Does this need libs-api or libs FCP? I can only start libs, not libs-api, and I thought this needed API.

@clarfonthey
Copy link
Contributor Author

Oh, then yes, it does need libs-api FCP. I'm not sure who would be appropriate for that then.

@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 Nov 10, 2023
@dtolnay dtolnay assigned dtolnay and unassigned Amanieu Nov 10, 2023
@bors
Copy link
Contributor

bors commented Nov 10, 2023

⌛ Testing commit 8337e86 with merge b5545d6...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 10, 2023
Add `std::hash::{DefaultHasher, RandomState}` exports (needs FCP)

This implements rust-lang/libs-team#267 to move the libstd hasher types to `std::hash` where they belong, instead of `std::collections::hash_map`.

<details><summary>The below no longer applies, but is kept for clarity.</summary>
This is a small refactor for rust-lang#27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API.

I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of `std::hash`, but for now, they're at least separated out from the collections code in a way that will make moving that around easier.

I decided to simply copy the rustdoc for `std::hash` from `core::hash` since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred.
</details>
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-mingw failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [rustdoc] tests\rustdoc\intra-doc\pub-use.rs stdout ----

error: rustdoc failed!
status: exit code: 0xc00000fd
command: PATH="C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin;C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0-bootstrap-tools\x86_64-pc-windows-gnu\release\deps;C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0\bin;C:\a\rust\rust\ninja;C:\a\rust\rust\mingw64\bin;C:\hostedtoolcache\windows\Python\3.12.0\x64\Scripts;C:\hostedtoolcache\windows\Python\3.12.0\x64;C:\msys64\usr\bin;C:\a\rust\rust\sccache;C:\PROGRA~1\MongoDB\bin;C:\aliyun-cli;C:\vcpkg;C:\cf-cli;C:\Program Files (x86)\NSIS;C:\tools\zstd;C:\Program Files\Mercurial;C:\hostedtoolcache\windows\stack\2.13.1\x64;C:\cabal\bin;C:\ghcup\bin;C:\mingw64\bin;C:\Program Files\dotnet;C:\Program Files\MySQL\MySQL Server 5.7\bin;C:\Program Files\R\R-4.3.1\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.20.10\x64\bin;C:\hostedtoolcache\windows\Python\3.7.9\x64\Scripts;C:\hostedtoolcache\windows\Python\3.7.9\x64;C:\hostedtoolcache\windows\Ruby\2.5.9\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.392-8\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\Program Files\Eclipse Foundation\jdk-8.0.302.8-hotspot\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\Chocolatey\bin;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\150\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\160\DTS\Binn;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\TortoiseSVN\bin;C:\Program Files\CMake\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.8.7\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\SeleniumWebDrivers\ChromeDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files (x86)\Microsoft BizTalk Server;C:\Program Files\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\bin\\rustdoc.exe" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\lib\\rustlib\\x86_64-pc-windows-gnu\\lib" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\intra-doc\\pub-use\\auxiliary" "-o" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\intra-doc\\pub-use" "--deny" "warnings" "C:\\a\\rust\\rust\\tests\\rustdoc\\intra-doc\\pub-use.rs" "-A" "internal_features"
--- stderr -------------------------------

thread 'main' has overflowed its stack
------------------------------------------

@bors
Copy link
Contributor

bors commented Nov 10, 2023

💔 Test failed - checks-actions

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

@dtolnay looks spurious, maybe retry?

@dtolnay
Copy link
Member

dtolnay commented Nov 11, 2023

@bors retry

@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 Nov 11, 2023
@bors
Copy link
Contributor

bors commented Nov 11, 2023

⌛ Testing commit 8337e86 with merge 2c1b65e...

@bors
Copy link
Contributor

bors commented Nov 11, 2023

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 2c1b65e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 11, 2023
@bors bors merged commit 2c1b65e into rust-lang:master Nov 11, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Nov 11, 2023
@clarfonthey clarfonthey deleted the std-hash-private branch November 11, 2023 23:09
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2c1b65e): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.9% [-1.2%, -0.4%] 6
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.9% [-1.2%, -0.4%] 6

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.3% [0.5%, 4.1%] 10
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.8% [-1.1%, -0.5%] 6
Improvements ✅
(secondary)
-1.0% [-1.0%, -1.0%] 1
All ❌✅ (primary) 0.5% [-1.1%, 4.1%] 16

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.5% [0.5%, 0.5%] 1
Regressions ❌
(secondary)
2.3% [2.2%, 2.5%] 3
Improvements ✅
(primary)
-0.8% [-0.9%, -0.6%] 5
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.5% [-0.9%, 0.5%] 6

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.2% [0.1%, 0.4%] 5
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.2% [-0.6%, -0.0%] 11
Improvements ✅
(secondary)
-0.0% [-0.0%, -0.0%] 3
All ❌✅ (primary) -0.0% [-0.6%, 0.4%] 16

Bootstrap: 673.486s -> 674.671s (0.18%)
Artifact size: 311.11 MiB -> 311.12 MiB (0.00%)

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 14, 2023
andros21 added a commit to andros21/wasmplayer that referenced this pull request Feb 11, 2024
* chore(src/lib.rs): `RandomState` now exported in `std::hash`

see https://github.com/rust-lang/rust/releases/tag/1.76.0
see rust-lang/rust#115694

* chore(ci/cd): handle `rustup update stable`

when there is a version mismatch between stable toolchain
installed on runner (used) and stable toolchain available
from upstream (latest)
andros21 added a commit to andros21/filmbot that referenced this pull request Feb 11, 2024
* chore(src/lib.rs): `{RandomState, DefaultHasher}` now exported in `std::hash`

see https://github.com/rust-lang/rust/releases/tag/1.76.0
see rust-lang/rust#115694

* chore(ci): handle `rustup update stable`

when there is a version mismatch between stable toolchain
installed on runner (used) and stable toolchain available
from upstream (latest)
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Feb 18, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.76.0 (2024-02-08)
==========================

Language
--------
- [Document Rust ABI compatibility between various types]
  (rust-lang/rust#115476)
- [Also: guarantee that char and u32 are ABI-compatible]
  (rust-lang/rust#118032)
- [Warn against ambiguous wide pointer comparisons]
  (rust-lang/rust#117758)

Compiler
--------
- [Lint pinned `#[must_use]` pointers (in particular, `Box<T>`
  where `T` is `#[must_use]`) in `unused_must_use`.]
  (rust-lang/rust#118054)
- [Soundness fix: fix computing the offset of an unsized field in
  a packed struct]
  (rust-lang/rust#118540)
- [Soundness fix: fix dynamic size/align computation logic for
  packed types with dyn Trait tail]
  (rust-lang/rust#118538)
- [Add `$message_type` field to distinguish json diagnostic outputs]
  (rust-lang/rust#115691)
- [Enable Rust to use the EHCont security feature of Windows]
  (rust-lang/rust#118013)
- [Add tier 3 {x86_64,i686}-win7-windows-msvc targets]
  (rust-lang/rust#118150)
- [Add tier 3 aarch64-apple-watchos target]
  (rust-lang/rust#119074)
- [Add tier 3 arm64e-apple-ios & arm64e-apple-darwin targets]
  (rust-lang/rust#115526)

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

Libraries
---------
- [Add a column number to `dbg!()`]
  (rust-lang/rust#114962)
- [Add `std::hash::{DefaultHasher, RandomState}` exports]
  (rust-lang/rust#115694)
- [Fix rounding issue with exponents in fmt]
  (rust-lang/rust#116301)
- [Add T: ?Sized to `RwLockReadGuard` and `RwLockWriteGuard`'s Debug impls.]
  (rust-lang/rust#117138)
- [Windows: Allow `File::create` to work on hidden files]
  (rust-lang/rust#116438)

Stabilized APIs
---------------
- [`Arc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.unwrap_or_clone)
- [`Rc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.unwrap_or_clone)
- [`Result::inspect`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect)
- [`Result::inspect_err`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect_err)
- [`Option::inspect`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.inspect)
- [`type_name_of_val`]
  (https://doc.rust-lang.org/stable/std/any/fn.type_name_of_val.html)
- [`std::hash::{DefaultHasher, RandomState}`]
  (https://doc.rust-lang.org/stable/std/hash/index.html#structs)
  These were previously available only through `std::collections::hash_map`.
- [`ptr::{from_ref, from_mut}`]
  (https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
- [`ptr::addr_eq`](https://doc.rust-lang.org/stable/std/ptr/fn.addr_eq.html)

Cargo
-----

See [Cargo release notes]
(https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-176-2024-02-08).

Rustdoc
-------
- [Don't merge cfg and doc(cfg) attributes for re-exports]
  (rust-lang/rust#113091)
- [rustdoc: allow resizing the sidebar / hiding the top bar]
  (rust-lang/rust#115660)
- [rustdoc-search: add support for traits and associated types]
  (rust-lang/rust#116085)
- [rustdoc: Add highlighting for comments in items declaration]
  (rust-lang/rust#117869)

Compatibility Notes
-------------------
- [Add allow-by-default lint for unit bindings]
  (rust-lang/rust#112380)
  This is expected to be upgraded to a warning by default in a future Rust
  release. Some macros emit bindings with type `()` with user-provided spans,
  which means that this lint will warn for user code.
- [Remove x86_64-sun-solaris target.]
  (rust-lang/rust#118091)
- [Remove asmjs-unknown-emscripten target]
  (rust-lang/rust#117338)
- [Report errors in jobserver inherited through environment variables]
  (rust-lang/rust#113730)
  This [may warn](rust-lang/rust#120515)
  on benign problems too.
- [Update the minimum external LLVM to 16.]
  (rust-lang/rust#117947)
- [Improve `print_tts`](rust-lang/rust#114571)
  This change can break some naive manual parsing of token trees
  in proc macro code which expect a particular structure after
  `.to_string()`, rather than just arbitrary Rust code.
- [Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint]
  (rust-lang/rust#117984)
- [Vec's allocation behavior was changed when collecting some iterators]
  (rust-lang/rust#110353)
  Allocation behavior is currently not specified, nevertheless
  changes can be surprising.
  See [`impl FromIterator for Vec`]
  (https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#impl-FromIterator%3CT%3E-for-Vec%3CT%3E)
  for more details.
- [Properly reject `default` on free const items]
  (rust-lang/rust#117818)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

None yet