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

Track devirtualized filenames #72767

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc_expand/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ impl<'a> ExtCtxt<'a> {
if !path.is_absolute() {
let callsite = span.source_callsite();
let mut result = match self.source_map().span_to_unmapped_path(callsite) {
FileName::Real(path) => path,
FileName::Real(name) => name.into_local_path(),
FileName::DocTest(path, _) => path,
other => {
return Err(self.struct_span_err(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_expand/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
let mut module = ModuleData {
mod_path: vec![Ident::from_str(&self.cx.ecfg.crate_name)],
directory: match self.cx.source_map().span_to_unmapped_path(krate.span) {
FileName::Real(path) => path,
FileName::Real(name) => name.into_local_path(),
other => PathBuf::from(other.to_string()),
},
};
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_expand/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ crate fn parse_external_mod(
// Extract the directory path for submodules of `module`.
let path = sess.source_map().span_to_unmapped_path(module.inner);
let mut path = match path {
FileName::Real(path) => path,
FileName::Real(name) => name.into_local_path(),
other => PathBuf::from(other.to_string()),
};
path.pop();
Expand Down Expand Up @@ -189,7 +189,8 @@ fn error_cannot_declare_mod_here<'a, T>(
let mut err =
sess.span_diagnostic.struct_span_err(span, "cannot declare a new module at this location");
if !span.is_dummy() {
if let FileName::Real(src_path) = sess.source_map().span_to_filename(span) {
if let FileName::Real(src_name) = sess.source_map().span_to_filename(span) {
let src_path = src_name.into_local_path();
if let Some(stem) = src_path.file_stem() {
let mut dest_path = src_path.clone();
dest_path.set_file_name(stem);
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_expand/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,8 @@ impl server::SourceFile for Rustc<'_> {
}
fn path(&mut self, file: &Self::SourceFile) -> String {
match file.name {
FileName::Real(ref path) => path
FileName::Real(ref name) => name
.local_path()
.to_str()
.expect("non-UTF8 file path in `proc_macro::SourceFile::path`")
.to_string(),
Expand Down
11 changes: 7 additions & 4 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use rustc_session::output::{filename_for_input, filename_for_metadata};
use rustc_session::search_paths::PathKind;
use rustc_session::Session;
use rustc_span::symbol::Symbol;
use rustc_span::FileName;
use rustc_span::{FileName, RealFileName};
use rustc_trait_selection::traits;
use rustc_typeck as typeck;

Expand Down Expand Up @@ -569,13 +569,16 @@ fn write_out_deps(
for cnum in resolver.cstore().crates_untracked() {
let source = resolver.cstore().crate_source_untracked(cnum);
if let Some((path, _)) = source.dylib {
files.push(escape_dep_filename(&FileName::Real(path)));
let file_name = FileName::Real(RealFileName::Named(path));
files.push(escape_dep_filename(&file_name));
}
if let Some((path, _)) = source.rlib {
files.push(escape_dep_filename(&FileName::Real(path)));
let file_name = FileName::Real(RealFileName::Named(path));
files.push(escape_dep_filename(&file_name));
}
if let Some((path, _)) = source.rmeta {
files.push(escape_dep_filename(&FileName::Real(path)));
let file_name = FileName::Real(RealFileName::Named(path));
files.push(escape_dep_filename(&file_name));
}
}
});
Expand Down
25 changes: 16 additions & 9 deletions src/librustc_metadata/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1471,15 +1471,22 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

if let Some(virtual_dir) = virtual_rust_source_base_dir {
if let Some(real_dir) = &sess.real_rust_source_base_dir {
if let rustc_span::FileName::Real(path) = name {
if let Ok(rest) = path.strip_prefix(virtual_dir) {
let new_path = real_dir.join(rest);
debug!(
"try_to_translate_virtual_to_real: `{}` -> `{}`",
path.display(),
new_path.display(),
);
*path = new_path;
if let rustc_span::FileName::Real(old_name) = name {
if let rustc_span::RealFileName::Named(one_path) = old_name {
if let Ok(rest) = one_path.strip_prefix(virtual_dir) {
let virtual_name = one_path.clone();
let new_path = real_dir.join(rest);
debug!(
"try_to_translate_virtual_to_real: `{}` -> `{}`",
virtual_name.display(),
new_path.display(),
);
let new_name = rustc_span::RealFileName::Devirtualized {
local_path: new_path,
virtual_name,
};
*old_name = new_name;
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ impl<'tcx> EncodeContext<'tcx> {
// any relative paths are potentially relative to a
// wrong directory.
FileName::Real(ref name) => {
let name = name.stable_name();
let mut adapted = (**source_file).clone();
adapted.name = Path::new(&working_dir).join(name).into();
adapted.name_hash = {
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_save_analysis/span_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ impl<'a> SpanUtils<'a> {

pub fn make_filename_string(&self, file: &SourceFile) -> String {
match &file.name {
FileName::Real(path) if !file.name_was_remapped => {
FileName::Real(name) if !file.name_was_remapped => {
let path = name.local_path();
if path.is_absolute() {
self.sess
.source_map()
.path_mapping()
.map_prefix(path.clone())
.map_prefix(path.into())
.0
.display()
.to_string()
Expand Down
61 changes: 57 additions & 4 deletions src/librustc_span/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use std::cmp::{self, Ordering};
use std::fmt;
use std::hash::Hash;
use std::ops::{Add, Sub};
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::str::FromStr;

use md5::Md5;
Expand Down Expand Up @@ -81,11 +81,58 @@ impl Globals {

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

/// FIXME: Perhaps this should not implement Rustc{Decodable, Encodable}
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, RustcDecodable, RustcEncodable)]
#[derive(HashStable_Generic)]
pub enum RealFileName {
Named(PathBuf),
/// For de-virtualized paths (namely paths into libstd that have been mapped
/// to the appropriate spot on the local host's file system),
Devirtualized {
/// `local_path` is the (host-dependent) local path to the file.
local_path: PathBuf,
/// `virtual_name` is the stable path rustc will store internally within
/// build artifacts.
virtual_name: PathBuf,
},
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
}

impl RealFileName {
/// Returns the path suitable for reading from the file system on the local host.
/// Avoid embedding this in build artifacts; see `stable_name` for that.
pub fn local_path(&self) -> &Path {
match self {
RealFileName::Named(p)
| RealFileName::Devirtualized { local_path: p, virtual_name: _ } => &p,
}
}

/// Returns the path suitable for reading from the file system on the local host.
/// Avoid embedding this in build artifacts; see `stable_name` for that.
pub fn into_local_path(self) -> PathBuf {
match self {
RealFileName::Named(p)
| RealFileName::Devirtualized { local_path: p, virtual_name: _ } => p,
}
}

/// Returns the path suitable for embedding into build artifacts. Note that
/// a virtualized path will not correspond to a valid file system path; see
/// `local_path` for something that is more likely to return paths into the
/// local host file system.
pub fn stable_name(&self) -> &Path {
match self {
RealFileName::Named(p)
| RealFileName::Devirtualized { local_path: _, virtual_name: p } => &p,
}
}
}

/// Differentiates between real files and common virtual files.
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, RustcDecodable, RustcEncodable)]
#[derive(HashStable_Generic)]
pub enum FileName {
Real(PathBuf),
Real(RealFileName),
/// Call to `quote!`.
QuoteExpansion(u64),
/// Command line.
Expand All @@ -107,7 +154,13 @@ impl std::fmt::Display for FileName {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use FileName::*;
match *self {
Real(ref path) => write!(fmt, "{}", path.display()),
Real(RealFileName::Named(ref path)) => write!(fmt, "{}", path.display()),
// FIXME: might be nice to display both compoments of Devirtualized.
// But for now (to backport fix for issue #70924), best to not
// perturb diagnostics so its obvious test suite still works.
Real(RealFileName::Devirtualized { ref local_path, virtual_name: _ }) => {
write!(fmt, "{}", local_path.display())
}
QuoteExpansion(_) => write!(fmt, "<quote expansion>"),
MacroExpansion(_) => write!(fmt, "<macro expansion>"),
Anon(_) => write!(fmt, "<anon>"),
Expand All @@ -123,7 +176,7 @@ impl std::fmt::Display for FileName {
impl From<PathBuf> for FileName {
fn from(p: PathBuf) -> Self {
assert!(!p.to_string_lossy().ends_with('>'));
FileName::Real(p)
FileName::Real(RealFileName::Named(p))
}
}

Expand Down
38 changes: 28 additions & 10 deletions src/librustc_span/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ impl FileLoader for RealFileLoader {
#[derive(Copy, Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, Debug)]
pub struct StableSourceFileId(u128);

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

pub fn new_from_pieces(
fn new_from_pieces(
name: &FileName,
name_was_remapped: bool,
unmapped_path: Option<&FileName>,
) -> StableSourceFileId {
let mut hasher = StableHasher::new();

name.hash(&mut hasher);
if let FileName::Real(real_name) = name {
// rust-lang/rust#70924: Use the stable (virtualized) name when
// available. (We do not want artifacts from transient file system
// paths for libstd to leak into our build artifacts.)
real_name.stable_name().hash(&mut hasher)
} else {
name.hash(&mut hasher);
}
Comment on lines +107 to +114
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed a problem with this (although it's unlikely to cause issues in practice): the hashed data could overlap.
A better approach might be to hash tuples that are (0u8, ...) in the first case and (1u8, ...) in the second one.

If we had an Either enum around, producing a value of that and hashing it would be even better (since there would be a single hash call).

name_was_remapped.hash(&mut hasher);
unmapped_path.hash(&mut hasher);

Expand Down Expand Up @@ -235,7 +244,7 @@ impl SourceMap {

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

let (filename, was_remapped) = match filename {
FileName::Real(filename) => {
let (filename, was_remapped) = self.path_mapping.map_prefix(filename);
(FileName::Real(filename), was_remapped)
let was_remapped;
if let FileName::Real(real_filename) = &mut filename {
match real_filename {
RealFileName::Named(path_to_be_remapped)
| RealFileName::Devirtualized {
local_path: path_to_be_remapped,
virtual_name: _,
} => {
let mapped = self.path_mapping.map_prefix(path_to_be_remapped.clone());
was_remapped = mapped.1;
*path_to_be_remapped = mapped.0;
}
}
other => (other, false),
};
} else {
was_remapped = false;
}

let file_id =
StableSourceFileId::new_from_pieces(&filename, was_remapped, Some(&unmapped_path));
Expand Down Expand Up @@ -998,7 +1016,7 @@ impl SourceMap {
}
pub fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool {
source_file.add_external_src(|| match source_file.name {
FileName::Real(ref name) => self.file_loader.read_file(name).ok(),
FileName::Real(ref name) => self.file_loader.read_file(name.local_path()).ok(),
_ => None,
})
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ pub fn run(
} = options;

let src_root = match krate.src {
FileName::Real(ref p) => match p.parent() {
FileName::Real(ref p) => match p.local_path().parent() {
Some(p) => p.to_path_buf(),
None => PathBuf::new(),
},
Expand Down Expand Up @@ -1621,9 +1621,10 @@ impl Context {

// We can safely ignore synthetic `SourceFile`s.
let file = match item.source.filename {
FileName::Real(ref path) => path,
FileName::Real(ref path) => path.local_path().to_path_buf(),
_ => return None,
};
let file = &file;

let (krate, path) = if item.source.cnum == LOCAL_CRATE {
if let Some(path) = self.shared.local_sources.get(file) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl Cache {
// Cache where all our extern crates are located
for &(n, ref e) in &krate.externs {
let src_root = match e.src {
FileName::Real(ref p) => match p.parent() {
FileName::Real(ref p) => match p.local_path().parent() {
Some(p) => p.to_path_buf(),
None => PathBuf::new(),
},
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/html/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ impl<'a> SourceCollector<'a> {
/// Renders the given filename into its corresponding HTML source file.
fn emit_source(&mut self, filename: &FileName) -> Result<(), Error> {
let p = match *filename {
FileName::Real(ref file) => file,
FileName::Real(ref file) => file.local_path().to_path_buf(),
_ => return Ok(()),
};
if self.scx.local_sources.contains_key(&**p) {
if self.scx.local_sources.contains_key(&*p) {
// We've already emitted this source
return Ok(());
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ impl Collector {
let filename = source_map.span_to_filename(self.position);
if let FileName::Real(ref filename) = filename {
if let Ok(cur_dir) = env::current_dir() {
if let Ok(path) = filename.strip_prefix(&cur_dir) {
if let Ok(path) = filename.local_path().strip_prefix(&cur_dir) {
return path.to_owned().into();
}
}
Expand Down Expand Up @@ -718,7 +718,7 @@ impl Tester for Collector {
// FIXME(#44940): if doctests ever support path remapping, then this filename
// needs to be the result of `SourceMap::span_to_unmapped_path`.
let path = match &filename {
FileName::Real(path) => path.clone(),
FileName::Real(path) => path.local_path().to_path_buf(),
_ => PathBuf::from(r"doctest.rs"),
};

Expand Down