Skip to content

Commit 5fd2f06

Browse files
committed
Auto merge of #72767 - pnkfelix:track-devirtualized-filenames-issue-70924, r=eddyb
Track devirtualized filenames Split payload of FileName::Real to track both real and virtualized paths. (Such splits arise from metadata refs into libstd; the virtualized paths look like `/rustc/1.45.0/src/libstd/io/cursor.rs` rather than `/Users/felixklock/Dev/Mozilla/rust.git/src/libstd/io/cursor.rs`) This way, we can emit the virtual name into things like the like the StableSourceFileId (as was done back before PR #70642) that ends up in incremental build artifacts, while still using the devirtualized file path when we want to access the file. Fix #70924
2 parents f6072ca + a8e4236 commit 5fd2f06

File tree

14 files changed

+130
-41
lines changed

14 files changed

+130
-41
lines changed

src/librustc_expand/base.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ impl<'a> ExtCtxt<'a> {
10951095
if !path.is_absolute() {
10961096
let callsite = span.source_callsite();
10971097
let mut result = match self.source_map().span_to_unmapped_path(callsite) {
1098-
FileName::Real(path) => path,
1098+
FileName::Real(name) => name.into_local_path(),
10991099
FileName::DocTest(path, _) => path,
11001100
other => {
11011101
return Err(self.struct_span_err(

src/librustc_expand/expand.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
341341
let mut module = ModuleData {
342342
mod_path: vec![Ident::from_str(&self.cx.ecfg.crate_name)],
343343
directory: match self.cx.source_map().span_to_unmapped_path(krate.span) {
344-
FileName::Real(path) => path,
344+
FileName::Real(name) => name.into_local_path(),
345345
other => PathBuf::from(other.to_string()),
346346
},
347347
};

src/librustc_expand/module.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ crate fn parse_external_mod(
7171
// Extract the directory path for submodules of `module`.
7272
let path = sess.source_map().span_to_unmapped_path(module.inner);
7373
let mut path = match path {
74-
FileName::Real(path) => path,
74+
FileName::Real(name) => name.into_local_path(),
7575
other => PathBuf::from(other.to_string()),
7676
};
7777
path.pop();
@@ -189,7 +189,8 @@ fn error_cannot_declare_mod_here<'a, T>(
189189
let mut err =
190190
sess.span_diagnostic.struct_span_err(span, "cannot declare a new module at this location");
191191
if !span.is_dummy() {
192-
if let FileName::Real(src_path) = sess.source_map().span_to_filename(span) {
192+
if let FileName::Real(src_name) = sess.source_map().span_to_filename(span) {
193+
let src_path = src_name.into_local_path();
193194
if let Some(stem) = src_path.file_stem() {
194195
let mut dest_path = src_path.clone();
195196
dest_path.set_file_name(stem);

src/librustc_expand/proc_macro_server.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,8 @@ impl server::SourceFile for Rustc<'_> {
602602
}
603603
fn path(&mut self, file: &Self::SourceFile) -> String {
604604
match file.name {
605-
FileName::Real(ref path) => path
605+
FileName::Real(ref name) => name
606+
.local_path()
606607
.to_str()
607608
.expect("non-UTF8 file path in `proc_macro::SourceFile::path`")
608609
.to_string(),

src/librustc_interface/passes.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use rustc_session::output::{filename_for_input, filename_for_metadata};
3333
use rustc_session::search_paths::PathKind;
3434
use rustc_session::Session;
3535
use rustc_span::symbol::Symbol;
36-
use rustc_span::FileName;
36+
use rustc_span::{FileName, RealFileName};
3737
use rustc_trait_selection::traits;
3838
use rustc_typeck as typeck;
3939

@@ -569,13 +569,16 @@ fn write_out_deps(
569569
for cnum in resolver.cstore().crates_untracked() {
570570
let source = resolver.cstore().crate_source_untracked(cnum);
571571
if let Some((path, _)) = source.dylib {
572-
files.push(escape_dep_filename(&FileName::Real(path)));
572+
let file_name = FileName::Real(RealFileName::Named(path));
573+
files.push(escape_dep_filename(&file_name));
573574
}
574575
if let Some((path, _)) = source.rlib {
575-
files.push(escape_dep_filename(&FileName::Real(path)));
576+
let file_name = FileName::Real(RealFileName::Named(path));
577+
files.push(escape_dep_filename(&file_name));
576578
}
577579
if let Some((path, _)) = source.rmeta {
578-
files.push(escape_dep_filename(&FileName::Real(path)));
580+
let file_name = FileName::Real(RealFileName::Named(path));
581+
files.push(escape_dep_filename(&file_name));
579582
}
580583
}
581584
});

src/librustc_metadata/rmeta/decoder.rs

+16-9
Original file line numberDiff line numberDiff line change
@@ -1471,15 +1471,22 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
14711471

14721472
if let Some(virtual_dir) = virtual_rust_source_base_dir {
14731473
if let Some(real_dir) = &sess.real_rust_source_base_dir {
1474-
if let rustc_span::FileName::Real(path) = name {
1475-
if let Ok(rest) = path.strip_prefix(virtual_dir) {
1476-
let new_path = real_dir.join(rest);
1477-
debug!(
1478-
"try_to_translate_virtual_to_real: `{}` -> `{}`",
1479-
path.display(),
1480-
new_path.display(),
1481-
);
1482-
*path = new_path;
1474+
if let rustc_span::FileName::Real(old_name) = name {
1475+
if let rustc_span::RealFileName::Named(one_path) = old_name {
1476+
if let Ok(rest) = one_path.strip_prefix(virtual_dir) {
1477+
let virtual_name = one_path.clone();
1478+
let new_path = real_dir.join(rest);
1479+
debug!(
1480+
"try_to_translate_virtual_to_real: `{}` -> `{}`",
1481+
virtual_name.display(),
1482+
new_path.display(),
1483+
);
1484+
let new_name = rustc_span::RealFileName::Devirtualized {
1485+
local_path: new_path,
1486+
virtual_name,
1487+
};
1488+
*old_name = new_name;
1489+
}
14831490
}
14841491
}
14851492
}

src/librustc_metadata/rmeta/encoder.rs

+1
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@ impl<'tcx> EncodeContext<'tcx> {
396396
// any relative paths are potentially relative to a
397397
// wrong directory.
398398
FileName::Real(ref name) => {
399+
let name = name.stable_name();
399400
let mut adapted = (**source_file).clone();
400401
adapted.name = Path::new(&working_dir).join(name).into();
401402
adapted.name_hash = {

src/librustc_save_analysis/span_utils.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ impl<'a> SpanUtils<'a> {
1616

1717
pub fn make_filename_string(&self, file: &SourceFile) -> String {
1818
match &file.name {
19-
FileName::Real(path) if !file.name_was_remapped => {
19+
FileName::Real(name) if !file.name_was_remapped => {
20+
let path = name.local_path();
2021
if path.is_absolute() {
2122
self.sess
2223
.source_map()
2324
.path_mapping()
24-
.map_prefix(path.clone())
25+
.map_prefix(path.into())
2526
.0
2627
.display()
2728
.to_string()

src/librustc_span/lib.rs

+60-4
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ use std::cmp::{self, Ordering};
5353
use std::fmt;
5454
use std::hash::Hash;
5555
use std::ops::{Add, Sub};
56-
use std::path::PathBuf;
56+
use std::path::{Path, PathBuf};
5757
use std::str::FromStr;
5858

5959
use md5::Md5;
@@ -81,11 +81,61 @@ impl Globals {
8181

8282
scoped_tls::scoped_thread_local!(pub static GLOBALS: Globals);
8383

84+
// FIXME: Perhaps this should not implement Rustc{Decodable, Encodable}
85+
//
86+
// FIXME: We should use this enum or something like it to get rid of the
87+
// use of magic `/rust/1.x/...` paths across the board.
88+
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, RustcDecodable, RustcEncodable)]
89+
#[derive(HashStable_Generic)]
90+
pub enum RealFileName {
91+
Named(PathBuf),
92+
/// For de-virtualized paths (namely paths into libstd that have been mapped
93+
/// to the appropriate spot on the local host's file system),
94+
Devirtualized {
95+
/// `local_path` is the (host-dependent) local path to the file.
96+
local_path: PathBuf,
97+
/// `virtual_name` is the stable path rustc will store internally within
98+
/// build artifacts.
99+
virtual_name: PathBuf,
100+
},
101+
}
102+
103+
impl RealFileName {
104+
/// Returns the path suitable for reading from the file system on the local host.
105+
/// Avoid embedding this in build artifacts; see `stable_name` for that.
106+
pub fn local_path(&self) -> &Path {
107+
match self {
108+
RealFileName::Named(p)
109+
| RealFileName::Devirtualized { local_path: p, virtual_name: _ } => &p,
110+
}
111+
}
112+
113+
/// Returns the path suitable for reading from the file system on the local host.
114+
/// Avoid embedding this in build artifacts; see `stable_name` for that.
115+
pub fn into_local_path(self) -> PathBuf {
116+
match self {
117+
RealFileName::Named(p)
118+
| RealFileName::Devirtualized { local_path: p, virtual_name: _ } => p,
119+
}
120+
}
121+
122+
/// Returns the path suitable for embedding into build artifacts. Note that
123+
/// a virtualized path will not correspond to a valid file system path; see
124+
/// `local_path` for something that is more likely to return paths into the
125+
/// local host file system.
126+
pub fn stable_name(&self) -> &Path {
127+
match self {
128+
RealFileName::Named(p)
129+
| RealFileName::Devirtualized { local_path: _, virtual_name: p } => &p,
130+
}
131+
}
132+
}
133+
84134
/// Differentiates between real files and common virtual files.
85135
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, RustcDecodable, RustcEncodable)]
86136
#[derive(HashStable_Generic)]
87137
pub enum FileName {
88-
Real(PathBuf),
138+
Real(RealFileName),
89139
/// Call to `quote!`.
90140
QuoteExpansion(u64),
91141
/// Command line.
@@ -109,7 +159,13 @@ impl std::fmt::Display for FileName {
109159
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
110160
use FileName::*;
111161
match *self {
112-
Real(ref path) => write!(fmt, "{}", path.display()),
162+
Real(RealFileName::Named(ref path)) => write!(fmt, "{}", path.display()),
163+
// FIXME: might be nice to display both compoments of Devirtualized.
164+
// But for now (to backport fix for issue #70924), best to not
165+
// perturb diagnostics so its obvious test suite still works.
166+
Real(RealFileName::Devirtualized { ref local_path, virtual_name: _ }) => {
167+
write!(fmt, "{}", local_path.display())
168+
}
113169
QuoteExpansion(_) => write!(fmt, "<quote expansion>"),
114170
MacroExpansion(_) => write!(fmt, "<macro expansion>"),
115171
Anon(_) => write!(fmt, "<anon>"),
@@ -126,7 +182,7 @@ impl std::fmt::Display for FileName {
126182
impl From<PathBuf> for FileName {
127183
fn from(p: PathBuf) -> Self {
128184
assert!(!p.to_string_lossy().ends_with('>'));
129-
FileName::Real(p)
185+
FileName::Real(RealFileName::Named(p))
130186
}
131187
}
132188

src/librustc_span/source_map.rs

+28-10
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ impl FileLoader for RealFileLoader {
8686
#[derive(Copy, Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, Debug)]
8787
pub struct StableSourceFileId(u128);
8888

89+
// FIXME: we need a more globally consistent approach to the problem solved by
90+
// StableSourceFileId, perhaps built atop source_file.name_hash.
8991
impl StableSourceFileId {
9092
pub fn new(source_file: &SourceFile) -> StableSourceFileId {
9193
StableSourceFileId::new_from_pieces(
@@ -95,14 +97,21 @@ impl StableSourceFileId {
9597
)
9698
}
9799

98-
pub fn new_from_pieces(
100+
fn new_from_pieces(
99101
name: &FileName,
100102
name_was_remapped: bool,
101103
unmapped_path: Option<&FileName>,
102104
) -> StableSourceFileId {
103105
let mut hasher = StableHasher::new();
104106

105-
name.hash(&mut hasher);
107+
if let FileName::Real(real_name) = name {
108+
// rust-lang/rust#70924: Use the stable (virtualized) name when
109+
// available. (We do not want artifacts from transient file system
110+
// paths for libstd to leak into our build artifacts.)
111+
real_name.stable_name().hash(&mut hasher)
112+
} else {
113+
name.hash(&mut hasher);
114+
}
106115
name_was_remapped.hash(&mut hasher);
107116
unmapped_path.hash(&mut hasher);
108117

@@ -235,7 +244,7 @@ impl SourceMap {
235244

236245
fn try_new_source_file(
237246
&self,
238-
filename: FileName,
247+
mut filename: FileName,
239248
src: String,
240249
) -> Result<Lrc<SourceFile>, OffsetOverflowError> {
241250
// The path is used to determine the directory for loading submodules and
@@ -245,13 +254,22 @@ impl SourceMap {
245254
// be empty, so the working directory will be used.
246255
let unmapped_path = filename.clone();
247256

248-
let (filename, was_remapped) = match filename {
249-
FileName::Real(filename) => {
250-
let (filename, was_remapped) = self.path_mapping.map_prefix(filename);
251-
(FileName::Real(filename), was_remapped)
257+
let was_remapped;
258+
if let FileName::Real(real_filename) = &mut filename {
259+
match real_filename {
260+
RealFileName::Named(path_to_be_remapped)
261+
| RealFileName::Devirtualized {
262+
local_path: path_to_be_remapped,
263+
virtual_name: _,
264+
} => {
265+
let mapped = self.path_mapping.map_prefix(path_to_be_remapped.clone());
266+
was_remapped = mapped.1;
267+
*path_to_be_remapped = mapped.0;
268+
}
252269
}
253-
other => (other, false),
254-
};
270+
} else {
271+
was_remapped = false;
272+
}
255273

256274
let file_id =
257275
StableSourceFileId::new_from_pieces(&filename, was_remapped, Some(&unmapped_path));
@@ -998,7 +1016,7 @@ impl SourceMap {
9981016
}
9991017
pub fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool {
10001018
source_file.add_external_src(|| match source_file.name {
1001-
FileName::Real(ref name) => self.file_loader.read_file(name).ok(),
1019+
FileName::Real(ref name) => self.file_loader.read_file(name.local_path()).ok(),
10021020
_ => None,
10031021
})
10041022
}

src/librustdoc/html/render.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ pub fn run(
473473
} = options;
474474

475475
let src_root = match krate.src {
476-
FileName::Real(ref p) => match p.parent() {
476+
FileName::Real(ref p) => match p.local_path().parent() {
477477
Some(p) => p.to_path_buf(),
478478
None => PathBuf::new(),
479479
},
@@ -1621,9 +1621,10 @@ impl Context {
16211621

16221622
// We can safely ignore synthetic `SourceFile`s.
16231623
let file = match item.source.filename {
1624-
FileName::Real(ref path) => path,
1624+
FileName::Real(ref path) => path.local_path().to_path_buf(),
16251625
_ => return None,
16261626
};
1627+
let file = &file;
16271628

16281629
let (krate, path) = if item.source.cnum == LOCAL_CRATE {
16291630
if let Some(path) = self.shared.local_sources.get(file) {

src/librustdoc/html/render/cache.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ impl Cache {
173173
// Cache where all our extern crates are located
174174
for &(n, ref e) in &krate.externs {
175175
let src_root = match e.src {
176-
FileName::Real(ref p) => match p.parent() {
176+
FileName::Real(ref p) => match p.local_path().parent() {
177177
Some(p) => p.to_path_buf(),
178178
None => PathBuf::new(),
179179
},

src/librustdoc/html/sources.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ impl<'a> SourceCollector<'a> {
6767
/// Renders the given filename into its corresponding HTML source file.
6868
fn emit_source(&mut self, filename: &FileName) -> Result<(), Error> {
6969
let p = match *filename {
70-
FileName::Real(ref file) => file,
70+
FileName::Real(ref file) => file.local_path().to_path_buf(),
7171
_ => return Ok(()),
7272
};
73-
if self.scx.local_sources.contains_key(&**p) {
73+
if self.scx.local_sources.contains_key(&*p) {
7474
// We've already emitted this source
7575
return Ok(());
7676
}

src/librustdoc/test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ impl Collector {
688688
let filename = source_map.span_to_filename(self.position);
689689
if let FileName::Real(ref filename) = filename {
690690
if let Ok(cur_dir) = env::current_dir() {
691-
if let Ok(path) = filename.strip_prefix(&cur_dir) {
691+
if let Ok(path) = filename.local_path().strip_prefix(&cur_dir) {
692692
return path.to_owned().into();
693693
}
694694
}
@@ -718,7 +718,7 @@ impl Tester for Collector {
718718
// FIXME(#44940): if doctests ever support path remapping, then this filename
719719
// needs to be the result of `SourceMap::span_to_unmapped_path`.
720720
let path = match &filename {
721-
FileName::Real(path) => path.clone(),
721+
FileName::Real(path) => path.local_path().to_path_buf(),
722722
_ => PathBuf::from(r"doctest.rs"),
723723
};
724724

0 commit comments

Comments
 (0)