Distinguish crates with the same name in type errors #117609
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @davidtwco (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
Noratrieb
reviewed
Nov 5, 2023
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
sysroot-errors
branch
from
3ac20da to
407099e
Compare
|
@rustbot label +A-metadata |
This comment has been minimized.
This comment has been minimized.
|
i ... don't understand how that error is possible? hashbrown.rs is only 1 line long lol |
jyn514
force-pushed
the
sysroot-errors
branch
from
407099e to
c25af3d
Compare
|
oh i forgot to push a commit lol |
davidtwco
approved these changes
Nov 6, 2023
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 6, 2023
Distinguish crates with the same name in type errors Previously, errors for crates with the same name would only distinguish them by the span of the source: ``` note: `HashMap<_, _, _, _>` is defined in crate `hashbrown` --> /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 note: `HashMap<u32, u32>` is defined in crate `hashbrown` --> /Users/jyn/.local/lib/cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 ``` When the same version of the crate is loaded twice, this isn't particularly helpful. Additionally show where the .rlib was loaded from (in this case one was loaded from the sysroot and the other from a cargo target dir). Fixes rust-lang#110055.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 6, 2023
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#117578 (Derive `TyEncodable`/`TyDecodable` in `rustc_type_ir`) - rust-lang#117592 (Use the correct span when emitting the `env!` result) - rust-lang#117609 (Distinguish crates with the same name in type errors) - rust-lang#117613 (Remove from vacation and compiler review group) - rust-lang#117615 (Couple of small changes) r? `@ghost` `@rustbot` modify labels: rollup
bors
added
S-waiting-on-author
|
lol just fyi literally every pr i make should not be marked as rollup=always |
|
i can't replicate this locally with |
|
oh it's because the source for the cargo registry is available lmao blocked on #113611 |
|
yes ok |
This was referenced Nov 12, 2023
jyn514
force-pushed
the
sysroot-errors
branch
from
c25af3d to
7f4072f
Compare
rustbot
added
the
T-bootstrap
rustbot
added
S-waiting-on-review
Previously, errors for crates with the same name would only distinguish them by the span of the source: ``` note: `HashMap<_, _, _, _>` is defined in crate `hashbrown` --> /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 note: `HashMap<u32, u32>` is defined in crate `hashbrown` --> /Users/jyn/.local/lib/cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 ``` When the same version of the crate is loaded twice, this isn't particularly helpful. Additionally show where the .rlib was loaded from (in this case one was loaded from the sysroot and the other from a cargo target dir). --- This also remaps cargo paths more eagerly; particularly, it unconditionally remaps cargo paths for libstd unconditionally, even if `remap-debuginfo = false`. This is the first time that cargo paths have shown up in UI tests; if the paths aren't remapped, they will differ depending on the value of `remap-debuginfo`, which means tests will fail either locally or in CI. This can't be worked around with adhoc normalization in the test makefile because it impacts the column width used for line numbers (when paths are remapped, we don't show the cargo sources). I could be convinced this is not the best solution. It's possible we could take some other approach, like how `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` opportunistically reverses the mapping for `compiler/`, which doesn't impact the error messages standard library developers see. That would be a bit more involved to get working, though.
jyn514
force-pushed
the
sysroot-errors
branch
from
7f4072f to
c790313
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
|
you probably want to turn off rollup=always |
|
@bors rollup=never |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 30, 2024
Distinguish crates with the same name in type errors Previously, errors for crates with the same name would only distinguish them by the span of the source: ``` note: `HashMap<_, _, _, _>` is defined in crate `hashbrown` --> /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 note: `HashMap<u32, u32>` is defined in crate `hashbrown` --> /Users/jyn/.local/lib/cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 ``` When the same version of the crate is loaded twice, this isn't particularly helpful. Additionally show where the .rlib was loaded from (in this case one was loaded from the sysroot and the other from a cargo target dir). --- This also remaps cargo paths more eagerly; particularly, it unconditionally remaps cargo paths for libstd unconditionally, even if `remap-debuginfo = false`. This is the first time that cargo paths have shown up in UI tests; if the paths aren't remapped, they will differ depending on the value of `remap-debuginfo`, which means tests will fail either locally or in CI. This can't be worked around with adhoc normalization in the test makefile because it impacts the column width used for line numbers (when paths are remapped, we don't show the cargo sources). I could be convinced this is not the best solution. It's possible we could take some other approach, like how `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` opportunistically reverses the mapping for `compiler/`, which doesn't impact the error messages standard library developers see. That would be a bit more involved to get working, though. --- Fixes rust-lang#110055.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
@rustbot author needs blessing i think? |
rustbot
added
S-waiting-on-author
no, this is still #117609 (comment). i don't know why remapping the cargo registry didn't help. |
|
@bors r- Probably got back into the queue to a synchronize. |
|
don't want to spend time debugging this |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, errors for crates with the same name would only distinguish them by the span of the source:
When the same version of the crate is loaded twice, this isn't particularly helpful. Additionally
show where the .rlib was loaded from (in this case one was loaded from the sysroot and the other
from a cargo target dir).
This also remaps cargo paths more eagerly; particularly, it unconditionally remaps cargo paths for
libstd unconditionally, even if
remap-debuginfo = false. This is the first time that cargo pathshave shown up in UI tests; if the paths aren't remapped, they will differ depending on the value of
remap-debuginfo, which means tests will fail either locally or in CI. This can't be worked aroundwith adhoc normalization in the test makefile because it impacts the column width used for line
numbers (when paths are remapped, we don't show the cargo sources).
I could be convinced this is not the best solution. It's possible we could take some other approach,
like how
CFG_VIRTUAL_RUST_SOURCE_BASE_DIRopportunistically reverses the mapping forcompiler/,which doesn't impact the error messages standard library developers see. That would be a bit more
involved to get working, though.
Fixes #110055.