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

rustdoc-json: switch from HashMap to FxHashMap to fix non-determinism #108626

Merged
merged 1 commit into from
Mar 5, 2023

Conversation

onur-ozkan
Copy link
Member

Using HashMap in rustdoc_json_types::Crate were causing creating randomly ordered objects in the json doc files. Which might cause problems to people who are doing comparison on those files specially in CI pipelines. See #103785 (comment)

This PR fixes that issue and extends the coverage of tests/run-make/rustdoc-verify-output-files testing ability.

@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2023

r? @jsha

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

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 1, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2023

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi

@onur-ozkan onur-ozkan force-pushed the consistent-json-docs branch 2 times, most recently from e921fe9 to 02b32e6 Compare March 1, 2023 22:17
@aDotInTheVoid
Copy link
Member

r? @aDotInTheVoid

I'm a bit nervous about the performance impact of this, as it changes the core data structure of rustdoc-json. Unfortunately we don't have any automated way to see if this actually matters (cc rust-lang/rustc-perf#1512)

Could you use a FxHashMap instead, as that should get us deterministic results, even if the IDs arn't sorted, but I don't think that matters since users shouldn't be relying on content (and therefor ordering) of IDs.

@rustbot rustbot assigned aDotInTheVoid and unassigned jsha Mar 2, 2023
@aDotInTheVoid aDotInTheVoid changed the title fix inconsistent json outputs from rustdoc rustdoc-json: Don't have non-deterministic output due to HashMap Mar 2, 2023
@jyn514
Copy link
Member

jyn514 commented Mar 2, 2023

Could you use a FxHashMap instead, as that should get us deterministic results

I don't think this is true? FxHashMap is just changing the default hasher for performance, it doesn't affect the order. I vaguely recall there's a StableHasher type of something but it's not the same as the FxHasher.

@aDotInTheVoid
Copy link
Member

FxHashMap is just changing the default hasher for performance, it doesn't affect the order.

The relevant difference is that two FxHashMaps with the same order of inserts will always iterate in the same order, as they don't randomize the hash seed (unlike std::collections::HashMap). StableHasher provides additional garenttes about the archetecure we are compilling from, that I don't think we need to make

@obi1kenobi
Copy link
Member

Unfortunately we don't have any automated way to see if this actually matters

As part of my performance work on cargo-semver-checks that led to my "Speeding up Rust semver-checking by over 2000x" blog post, I did some benchmarking of HashMap vs BTreeMap as secondary indexes over rustdoc (code here). To a rounding error, the maps there are of the same size as the rustdoc index and paths maps, and one of them contains Id as part of the key. I also didn't do a ton of quiescing my system (disable hyperthreading and turbo boost, fix fan speed, do thermal warmup, etc.). So treat my findings as anecdotal + directional rather than authoritative + exact. They are not a replacement for proper benchmark tests of rustdoc itself.

I found that using HashMap was consistently noticeably faster than BTreeMap over a gigantic crate (aws-sdk-ec2, 240k items), by about 15-20% i.e. ~1-2s per run.

This doesn't say that we shouldn't replace HashMap with BTreeMap. It only says that there's good reason for performance concern, and we should benchmark more thoroughly before making a substantive change like that.

If anyone would like to repeat my experiments, I'd be happy to help you get set up. And if cargo-semver-checks would be useful as a part of a perf benchmark suite, I'd be happy to assist in that process too.

@onur-ozkan
Copy link
Member Author

onur-ozkan commented Mar 2, 2023

Could you use a FxHashMap instead, as that should get us deterministic results, even if the IDs arn't sorted, but I don't think that matters since users shouldn't be relying on content (and therefor ordering) of IDs.

I didn't want to involve rustc_private into rustdoc-json-types for this. I can do it if you want so.

For the performance concerns, here is a basic demonstration from my desktop machine.

using `BTreeMap`
Build completed successfully in 0:00:15

 Performance counter stats for '../../rust/x doc --stage 1 std --json':

         15,873.45 msec task-clock:u              #    1.000 CPUs utilized
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
           419,374      page-faults:u             #    0.026 M/sec
    62,660,633,799      cycles:u                  #    3.948 GHz                      (83.38%)
       512,794,289      stalled-cycles-frontend:u #    0.82% frontend cycles idle     (83.39%)
        80,806,547      stalled-cycles-backend:u  #    0.13% backend cycles idle      (83.38%)
    82,024,064,071      instructions:u            #    1.31  insn per cycle
                                                  #    0.01  stalled cycles per insn  (83.36%)
    16,035,947,363      branches:u                # 1010.237 M/sec                    (83.38%)
       201,578,639      branch-misses:u           #    1.26% of all branches          (83.32%)

      15.867989639 seconds time elapsed

      14.909949000 seconds user
       0.826553000 seconds sys
using `FxHashMap`
Build completed successfully in 0:00:15

 Performance counter stats for '../../rust/x doc --stage 1 std --json':

         15,548.93 msec task-clock:u              #    1.000 CPUs utilized
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
           423,520      page-faults:u             #    0.027 M/sec
    61,458,365,022      cycles:u                  #    3.953 GHz                      (83.37%)
       505,423,215      stalled-cycles-frontend:u #    0.82% frontend cycles idle     (83.37%)
       710,815,053      stalled-cycles-backend:u  #    1.16% backend cycles idle      (83.40%)
    81,826,900,373      instructions:u            #    1.33  insn per cycle
                                                  #    0.01  stalled cycles per insn  (83.31%)
    15,968,492,682      branches:u                # 1026.984 M/sec                    (83.35%)
       201,022,039      branch-misses:u           #    1.26% of all branches          (83.40%)

      15.542970326 seconds time elapsed

      14.672585000 seconds user
       0.740135000 seconds sys

HashMap benchmark is just for the comparison, I think we should avoid using it. It produces the problem mentioned in the PR description.

using `HashMap`
Build completed successfully in 0:00:15

 Performance counter stats for '../../rust/x doc --stage 1 std --json':

         15,617.23 msec task-clock:u              #    1.000 CPUs utilized
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
           425,083      page-faults:u             #    0.027 M/sec
    61,592,835,803      cycles:u                  #    3.944 GHz                      (83.39%)
       490,297,782      stalled-cycles-frontend:u #    0.80% frontend cycles idle     (83.37%)
       727,341,887      stalled-cycles-backend:u  #    1.18% backend cycles idle      (83.38%)
    81,851,581,044      instructions:u            #    1.33  insn per cycle
                                                  #    0.01  stalled cycles per insn  (83.37%)
    15,975,646,762      branches:u                # 1022.950 M/sec                    (83.38%)
       202,179,644      branch-misses:u           #    1.27% of all branches          (83.31%)

      15.609654475 seconds time elapsed

      14.611848000 seconds user
       0.869682000 seconds sys

Those runs were on the precompiled bootstrap/compiler/library. All went directly to generating the json files.

Machine specs
CPU: AMD Ryzen 9 5900X (24) @ 4.200GHz
MEMORY: 32 GB DDR4 2666 MHz
OS: Void Linux @ Kernel: 6.1.14_1

Regarding to the results, FxHashMap seems fixing the problem with less performance impact compare to BTreeMap.

@aDotInTheVoid
Copy link
Member

I didn't want to involve rustc_private into rustdoc-json-types for this.

It's fine, as this is only ment for use in the rust-lang/rust repo. The reexport I maintain for crates.io already handles this. While it's important that this library can be used on crates.io and stable, adding rustc_data_structures as a dep won't change that

@onur-ozkan
Copy link
Member Author

onur-ozkan commented Mar 2, 2023

I didn't want to involve rustc_private into rustdoc-json-types for this.

It's fine, as this is only ment for use in the rust-lang/rust repo. The reexport I maintain for crates.io already handles this. While it's important that this library can be used on crates.io and stable, adding rustc_data_structures as a dep won't change that

rustc_data_structures will fail jsondoclint because of the rustc_private feature. Is it okay if I use rustc_hash instead of rustc_data_structures?

I will push it using rustc_hash, if it's not okay, you can share your concerns with on the review

@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2023

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

@onur-ozkan onur-ozkan force-pushed the consistent-json-docs branch 2 times, most recently from 278535c to df061c8 Compare March 2, 2023 22:49
@aDotInTheVoid
Copy link
Member

Is it okay if I use rustc_hash instead of rustc_data_structures?

I'm not sure what the policy on this is. I've asked on zulip

@aDotInTheVoid
Copy link
Member

Turns out it's fine. Thanks for fixing this.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2023

📌 Commit df061c8 has been approved by aDotInTheVoid

It is now in the queue for this repository.

@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 Mar 4, 2023
@aDotInTheVoid aDotInTheVoid changed the title rustdoc-json: Don't have non-deterministic output due to HashMap rustdoc-json: switch from HashMap to FxHashMap to fix non-determinism Mar 4, 2023
Signed-off-by: ozkanonur <work@onurozkan.dev>
@aDotInTheVoid
Copy link
Member

r=me when CI's green

@onur-ozkan
Copy link
Member Author

r? @aDotInTheVoid

@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2023

Could not assign reviewer from: aDotInTheVoid.
User(s) aDotInTheVoid are either the PR author or are already assigned, and there are no other candidates.
Use r? to specify someone else to assign.

@aDotInTheVoid
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2023

📌 Commit 52c71e6 has been approved by aDotInTheVoid

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 4, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#106440 (Ignore files in .gitignore in tidy)
 - rust-lang#108613 (Remove `llvm.skip-rebuild` option)
 - rust-lang#108616 (Sync codegen defaults with compiler defaults and add a ping message so they stay in sync)
 - rust-lang#108618 (Rename `src/etc/vscode_settings.json` to `rust_analyzer_settings.json`)
 - rust-lang#108626 (rustdoc-json: switch from HashMap to FxHashMap to fix non-determinism)
 - rust-lang#108744 (Don't ICE when encountering bound var in builtin copy/clone bounds)
 - rust-lang#108749 (Clean up rustdoc-js tester.js file)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 03c1e4d into rust-lang:master Mar 5, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 5, 2023
aDotInTheVoid added a commit to aDotInTheVoid/rustdoc-types that referenced this pull request Mar 6, 2023
aDotInTheVoid added a commit to aDotInTheVoid/rust that referenced this pull request Apr 7, 2023
The original motivation was me trying to remove the
`#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using
`FxHashMap` here. I then realized I should also be able to remove the
`.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot
`rustc-data-strucures`, whereas `rustdoc-json-types` got it via
`rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as
`rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which
means need needs to be buildable without nightly features.
notriddle added a commit to notriddle/rust that referenced this pull request Apr 8, 2023
Use same `FxHashMap` in `rustdoc-json-types` and `librustdoc`.

The original motivation was me trying to remove the `#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using `FxHashMap` here. I then realized I should also be able to remove the `.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot `rustc-data-strucures`, whereas `rustdoc-json-types` got it via `rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as `rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which means need needs to be buildable without nightly features.

r? `@jyn514`
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Apr 14, 2023
The original motivation was me trying to remove the
`#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using
`FxHashMap` here. I then realized I should also be able to remove the
`.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot
`rustc-data-strucures`, whereas `rustdoc-json-types` got it via
`rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as
`rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which
means need needs to be buildable without nightly features.
aliemjay added a commit to aliemjay/rust that referenced this pull request Apr 15, 2023
Use same `FxHashMap` in `rustdoc-json-types` and `librustdoc`.

The original motivation was me trying to remove the `#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using `FxHashMap` here. I then realized I should also be able to remove the `.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot `rustc-data-strucures`, whereas `rustdoc-json-types` got it via `rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as `rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which means need needs to be buildable without nightly features.

r? `@jyn514`
aliemjay added a commit to aliemjay/rust that referenced this pull request Apr 15, 2023
Use same `FxHashMap` in `rustdoc-json-types` and `librustdoc`.

The original motivation was me trying to remove the `#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using `FxHashMap` here. I then realized I should also be able to remove the `.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot `rustc-data-strucures`, whereas `rustdoc-json-types` got it via `rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as `rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which means need needs to be buildable without nightly features.

r? ``@jyn514``
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Jul 14, 2023
The original motivation was me trying to remove the
`#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using
`FxHashMap` here. I then realized I should also be able to remove the
`.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot
`rustc-data-strucures`, whereas `rustdoc-json-types` got it via
`rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as
`rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which
means need needs to be buildable without nightly features.
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Dec 18, 2023
The original motivation was me trying to remove the
`#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using
`FxHashMap` here. I then realized I should also be able to remove the
`.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot
`rustc-data-strucures`, whereas `rustdoc-json-types` got it via
`rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as
`rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which
means need needs to be buildable without nightly features.
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Jul 6, 2024
The original motivation was me trying to remove the
`#![allow(rustc::default_hash_types)]`, as after rust-lang#108626, we should be using
`FxHashMap` here. I then realized I should also be able to remove the
`.into_iter().collect()`, as we no longer need to convert from `FxHashMap` to `std::HashMap`.

However, this didn't work, and I got the following error

```
error[E0308]: mismatched types
   --> src/librustdoc/json/mod.rs:235:13
    |
235 |             index,
    |             ^^^^^ expected `rustc_hash::FxHasher`, found `FxHasher`
    |
    = note: `FxHasher` and `rustc_hash::FxHasher` have similar names, but are actually distinct types
note: `FxHasher` is defined in crate `rustc_hash`
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
note: `rustc_hash::FxHasher` is defined in crate `rustc_hash`
   --> /home/alona/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-hash-1.1.0/src/lib.rs:60:1
    |
60  | pub struct FxHasher {
    | ^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `rustc_hash` are being used?
```

This is because `librustdoc` got it's `FxHashMap` via the sysroot
`rustc-data-strucures`, whereas `rustdoc-json-types` got it via
`rustc-hash` on crates.io.

To avoid this, `rustdoc-json-types` now uses
`#![feature(rustc_private)]` to load the same version as `librustdoc`.

However, this needs to be placed behind a feature, as
`rustdoc-json-types` is also dependency of `src/tools/jsondocck`, which
means need needs to be buildable without nightly features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants