Skip to content

Conversation

@Urgau
Copy link
Member

@Urgau Urgau commented Dec 6, 2025

This PR overhauls the way we handle filenames in the compiler and rmeta in order to achieve achieve cross-compiler consistency (ie. having the same path no matter if the filename was created in the current compiler session or is coming from rmeta).

This is required as some parts of the compiler rely on consistent paths for the soundness of generated code (see #148328).

In order to achieved consistency multiple steps are being taken by this PR:

  • by making RealFileName immutable
  • by only having SourceMap::to_real_filename create RealFileName
    • currently RealFileName can be created from any Path and are remapped afterwards, which creates consistency issue
  • by also making RealFileName holds it's working directory, embeddable name and the remapped scopes
    • this removes the need for a Session, to know the current(!) scopes and cwd, which is invalid as they may not be equal to the scopes used when creating the filename

In order for SourceMap::to_real_filename to know which scopes to apply FilePathMapping now takes the current remapping scopes to apply, which makes FileNameDisplayPreference and company useless and are removed.

This PR is split-up in multiple commits (unfortunately not atomic), but should help review the changes.

Unblocks #147611
Fixes #148328

cc @RalfJung @oli-obk
r? @davidtwco

Urgau added 2 commits December 5, 2025 23:32
This is preparation for the filename handling overhaul where having
access to the working directory is mandatory.
This is done in order to simplify the filename overhaul and those places
don't need to the embeddable path anyway.
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2025

Some changes occurred in coverage instrumentation.

cc @Zalathar

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Dec 6, 2025
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the overhaul-filenames branch from 268f9ac to 2dc3531 Compare December 6, 2025 12:02
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2025

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@rustbot rustbot added the T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. label Dec 6, 2025
@RalfJung
Copy link
Member

RalfJung commented Dec 6, 2025

@bors try
@rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Dec 6, 2025
Overhaul filename handling for cross-compiler consistency
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 6, 2025
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the overhaul-filenames branch from 2dc3531 to d5be1b8 Compare December 6, 2025 13:17
@RalfJung

This comment was marked as outdated.

@rust-bors

This comment was marked as outdated.

@Urgau Urgau force-pushed the overhaul-filenames branch from d5be1b8 to 21ea4d2 Compare December 6, 2025 13:47
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the overhaul-filenames branch from 21ea4d2 to 7ab8c59 Compare December 6, 2025 14:56
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the overhaul-filenames branch from 7ab8c59 to b91c3d0 Compare December 6, 2025 16:20
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the overhaul-filenames branch from b91c3d0 to b954e1f Compare December 6, 2025 17:50
@rust-log-analyzer

This comment has been minimized.

Urgau added 9 commits December 6, 2025 20:19
This commit refactors `SourceMap` and most importantly `RealFileName` to
make it self-contained in order to achieve cross-compiler consistency.

This is achieved:
 - by making `RealFileName` immutable
 - by only having `SourceMap::to_real_filename` create `RealFileName`
 - by also making `RealFileName` holds it's working directory,
   it's embeddable name and the remapped scopes
 - by making most `FileName` and `RealFileName` methods take a scope as
   an argument

In order for `SourceMap::to_real_filename` to know which scopes to apply
`FilePathMapping` now takes the current remapping scopes to apply, which
makes `FileNameDisplayPreference` and company useless and are removed.

The scopes type `RemapPathScopeComponents` was moved from
`rustc_session::config` to `rustc_span`.

The previous system for scoping the local/remapped filenames
`RemapFileNameExt::for_scope` is no longer useful as it's replaced by
methods on `FileName` and `RealFileName`.
And use the diagnostic method for diagnostic paths.
We can't completely remove it as it's needed for incr comp but direct
consumers towards `SourceMap::working_dir` instead.
@Urgau Urgau force-pushed the overhaul-filenames branch from b954e1f to 62464ef Compare December 6, 2025 19:19
@Urgau

This comment was marked as outdated.

rust-bors bot added a commit that referenced this pull request Dec 6, 2025
Overhaul filename handling for cross-compiler consistency
@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Dec 6, 2025

☀️ Try build successful (CI)
Build commit: 05b3365 (05b33656fd9ded2829b42521378e6fa37f788cc6, parent: ba86c0460b0233319e01fd789a42a7276eade805)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (05b3365): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.7% [0.2%, 1.1%] 7
Regressions ❌
(secondary)
0.6% [0.0%, 2.2%] 32
Improvements ✅
(primary)
-0.6% [-1.6%, -0.2%] 12
Improvements ✅
(secondary)
-0.3% [-0.7%, -0.2%] 4
All ❌✅ (primary) -0.2% [-1.6%, 1.1%] 19

Max RSS (memory usage)

Results (primary 0.7%, secondary 1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)
3.2% [2.3%, 6.1%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.8% [-4.4%, -3.1%] 2
All ❌✅ (primary) 0.7% [0.7%, 0.7%] 1

Cycles

Results (primary -2.6%, secondary -0.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.0% [4.0%, 4.0%] 1
Improvements ✅
(primary)
-2.6% [-2.6%, -2.6%] 1
Improvements ✅
(secondary)
-2.4% [-2.7%, -2.0%] 3
All ❌✅ (primary) -2.6% [-2.6%, -2.6%] 1

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.3%] 96
Regressions ❌
(secondary)
0.1% [0.0%, 0.4%] 54
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 4
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 2
All ❌✅ (primary) 0.1% [-0.0%, 0.3%] 100

Bootstrap: 470.7s -> 471.092s (0.08%)
Artifact size: 388.87 MiB -> 388.90 MiB (0.01%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 7, 2025
@Urgau
Copy link
Member Author

Urgau commented Dec 7, 2025

Those perf results seems to be mostly noise.

Instruction count saw a lot of variability but is overall positive for primary, but more interestingly Max RSS, Cycles and Wall Time are within noise level.

The only metric that shows a really change is Binary size since we now encode more metadata. Before we encoded two PathBuf max, now we encode six PathBuf max (two * (name, working dir, embeddable name)).

We could potentially de-duplicate the encoding of the working directory but that seems like a lot of work for not much gain.

@Urgau Urgau added the perf-regression-triaged The performance regression has been triaged. label Dec 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

std::panic::Location::caller().file() uses absolute path when monomorphized in a different crate.

7 participants