Account for -Z simulate-remapped-rust-src-base when resolving remapped paths
#97786
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
rustbot
added
the
T-compiler
|
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This was referenced Jun 6, 2022
pietroalbini
force-pushed
the
pa-fix-simulate-remap-prefix
branch
from
0d5e545
to
2dbbbd5
Compare
|
Note that this PR will fail CI until #97789 is merged, as |
This comment has been minimized.
This comment has been minimized.
|
r? rust-lang/compiler |
|
☔ The latest upstream changes (presumably #97989) made this pull request unmergeable. Please resolve the merge conflicts. |
pietroalbini
force-pushed
the
pa-fix-simulate-remap-prefix
branch
from
2dbbbd5
to
bcf2962
Compare
|
Rebased, tests should now pass! |
This comment has been minimized.
This comment has been minimized.
pietroalbini
force-pushed
the
pa-fix-simulate-remap-prefix
branch
from
bcf2962
to
1a5e2d8
Compare
|
r? @Mark-Simulacrum since you last reviewed this code |
|
@bors r+ |
|
📌 Commit 1a5e2d8 has been approved by |
bors
added
S-waiting-on-bors
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Jun 26, 2022
…fix, r=Mark-Simulacrum Account for `-Z simulate-remapped-rust-src-base` when resolving remapped paths Discovered in rust-lang#97682, `-Z simulate-remapped-rust-src-base` only partially simulated the behavior of `remap-debuginfo = true`. While the flag successfully simulates the remapping when stdlib's `rmeta` file is loaded, the simulated prefix was not accounted for when the remapped path's local path was being discovered. This caused the flag to not fully simulate the behavior of `remap-debuginfo = true`, leading to inconsistent behaviors. This PR fixes rust-lang#97682 by also accounting for the simulated path.
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Jun 28, 2022
…fix, r=Mark-Simulacrum Account for `-Z simulate-remapped-rust-src-base` when resolving remapped paths Discovered in rust-lang#97682, `-Z simulate-remapped-rust-src-base` only partially simulated the behavior of `remap-debuginfo = true`. While the flag successfully simulates the remapping when stdlib's `rmeta` file is loaded, the simulated prefix was not accounted for when the remapped path's local path was being discovered. This caused the flag to not fully simulate the behavior of `remap-debuginfo = true`, leading to inconsistent behaviors. This PR fixes rust-lang#97682 by also accounting for the simulated path.
This was referenced Jun 28, 2022
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 29, 2022
Rollup of 7 pull requests Successful merges: - rust-lang#97423 (Simplify memory ordering intrinsics) - rust-lang#97542 (Use typed indices in argument mismatch algorithm) - rust-lang#97786 (Account for `-Z simulate-remapped-rust-src-base` when resolving remapped paths) - rust-lang#98277 (Fix trait object reborrow suggestion) - rust-lang#98525 (Add regression test for rust-lang#79224) - rust-lang#98549 (interpret: do not prune requires_caller_location stack frames quite so early) - rust-lang#98603 (Some borrowck diagnostic fixes) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Note: this PR was a major source of the big perf regression seen in rollup #98656 (as the results from a perf run of its revert show). |
|
This PR is responsible for this regression, so I am tagging it as perf-regression. It landed in #98656 (a rollup), which as now been marked as having its perf regression triaged. I think the next step here is to investigate to see if we can avoid the at least to me unexpected impact here -- @pietroalbini can hopefully look into it a little. |
|
I'm... extremely confused how this could cause such a perf regression. I'll try to spend a bit of time loooking into it. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 26, 2022
…sion, r=Mark-Simulacrum Remove new allocations from `imported_source_files` rust-lang#97786 introduced a [large performance regression](rust-lang#97786 (comment)). After some local investigation it turns out the allocations performed by my change were the cause of the perf regression. This PR refactors my change to remove those allocations.
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.
Discovered in #97682,
-Z simulate-remapped-rust-src-baseonly partially simulated the behavior ofremap-debuginfo = true. While the flag successfully simulates the remapping when stdlib'srmetafile is loaded, the simulated prefix was not accounted for when the remapped path's local path was being discovered. This caused the flag to not fully simulate the behavior ofremap-debuginfo = true, leading to inconsistent behaviors.This PR fixes #97682 by also accounting for the simulated path.