Skip to content

Commit

Permalink
Auto merge of #88363 - michaelwoerister:remapped-diagnostics, r=estebank
Browse files Browse the repository at this point in the history
Path remapping: Make behavior of diagnostics output dependent on presence of --remap-path-prefix.

This PR fixes a regression (#87745) with `--remap-path-prefix` where the flag stopped causing diagnostic messages to be remapped as well. The regression was introduced in #83813 where we erroneously assumed that remapping of diagnostic messages was not desired anymore (because #70642 partially undid that functionality with nobody objecting).

The issue is fixed by making `--remap-path-prefix` remap diagnostic messages again, including for paths that have been remapped in upstream crates (e.g. the standard library). This means that "sysroot-localization" (implemented in #70642) is also disabled if `rustc` is invoked with `--remap-path-prefix`. The assumption is that once someone starts explicitly remapping paths they also don't want paths to their local Rust installation in their build output.

In the future we might want to give more fine-grained control over this behavior via compiler flags (see rust-lang/rfcs#3127 for a related RFC). For now this PR is intended as a regression fix.

This PR is an alternative to #88191, which makes diagnostic messages be remapped unconditionally. That approach, however, would effectively revert #70642.

Fixes #87745.

cc `@cbeuw`
r? `@ghost`
  • Loading branch information
bors committed Sep 3, 2021
2 parents 371f3cd + c296c89 commit 97f2698
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 41 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl<'tcx> DebugContext<'tcx> {
rustc_interface::util::version_str().unwrap_or("unknown version"),
cranelift_codegen::VERSION,
);
let comp_dir = tcx.sess.opts.working_dir.to_string_lossy(false).into_owned();
let comp_dir = tcx.sess.opts.working_dir.to_string_lossy(FileNameDisplayPreference::Remapped).into_owned();
let (name, file_info) = match tcx.sess.local_crate_source_file.clone() {
Some(path) => {
let name = path.to_string_lossy().into_owned();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ mod vtable;
mod prelude {
pub(crate) use std::convert::{TryFrom, TryInto};

pub(crate) use rustc_span::Span;
pub(crate) use rustc_span::{Span, FileNameDisplayPreference};

pub(crate) use rustc_hir::def_id::{DefId, LOCAL_CRATE};
pub(crate) use rustc_middle::bug;
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use rustc_middle::ty::{self, AdtKind, GeneratorSubsts, ParamEnv, Ty, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::config::{self, DebugInfo};
use rustc_span::symbol::{Interner, Symbol};
use rustc_span::FileNameDisplayPreference;
use rustc_span::{self, SourceFile, SourceFileHash, Span};
use rustc_target::abi::{Abi, Align, HasDataLayout, Integer, LayoutOf, TagEncoding};
use rustc_target::abi::{Int, Pointer, F32, F64};
Expand Down Expand Up @@ -771,7 +772,13 @@ pub fn file_metadata(cx: &CodegenCx<'ll, '_>, source_file: &SourceFile) -> &'ll
let hash = Some(&source_file.src_hash);
let file_name = Some(source_file.name.prefer_remapped().to_string());
let directory = if source_file.is_real_file() && !source_file.is_imported() {
Some(cx.sess().opts.working_dir.to_string_lossy(false).to_string())
Some(
cx.sess()
.opts
.working_dir
.to_string_lossy(FileNameDisplayPreference::Remapped)
.to_string(),
)
} else {
// If the path comes from an upstream crate we assume it has been made
// independent of the compiler's working directory one way or another.
Expand Down Expand Up @@ -999,7 +1006,7 @@ pub fn compile_unit_metadata(
let producer = format!("clang LLVM ({})", rustc_producer);

let name_in_debuginfo = name_in_debuginfo.to_string_lossy();
let work_dir = tcx.sess.opts.working_dir.to_string_lossy(false);
let work_dir = tcx.sess.opts.working_dir.to_string_lossy(FileNameDisplayPreference::Remapped);
let flags = "\0";
let output_filenames = tcx.output_filenames(());
let out_dir = &output_filenames.out_directory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl AnnotateSnippetEmitterWriter {
}
// owned: line source, line index, annotations
type Owned = (String, usize, Vec<crate::snippet::Annotation>);
let filename = primary_lo.file.name.prefer_local();
let filename = source_map.filename_for_diagnostics(&primary_lo.file.name);
let origin = filename.to_string_lossy();
let annotated_files: Vec<Owned> = annotated_files
.into_iter()
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1320,7 +1320,7 @@ impl EmitterWriter {
buffer_msg_line_offset,
&format!(
"{}:{}:{}",
loc.file.name.prefer_local(),
sm.filename_for_diagnostics(&loc.file.name),
sm.doctest_offset_line(&loc.file.name, loc.line),
loc.col.0 + 1,
),
Expand All @@ -1334,7 +1334,7 @@ impl EmitterWriter {
0,
&format!(
"{}:{}:{}: ",
loc.file.name.prefer_local(),
sm.filename_for_diagnostics(&loc.file.name),
sm.doctest_offset_line(&loc.file.name, loc.line),
loc.col.0 + 1,
),
Expand Down Expand Up @@ -1362,12 +1362,12 @@ impl EmitterWriter {
};
format!(
"{}:{}{}",
annotated_file.file.name.prefer_local(),
sm.filename_for_diagnostics(&annotated_file.file.name),
sm.doctest_offset_line(&annotated_file.file.name, first_line.line_index),
col
)
} else {
format!("{}", annotated_file.file.name.prefer_local())
format!("{}", sm.filename_for_diagnostics(&annotated_file.file.name))
};
buffer.append(buffer_msg_line_offset + 1, &loc, Style::LineAndColumn);
for _ in 0..max_line_num_len {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ impl DiagnosticSpan {
});

DiagnosticSpan {
file_name: start.file.name.prefer_local().to_string(),
file_name: je.sm.filename_for_diagnostics(&start.file.name).to_string(),
byte_start: start.file.original_relative_byte_pos(span.lo()).0,
byte_end: start.file.original_relative_byte_pos(span.hi()).0,
line_start: start.line,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,7 @@ impl<'a> ExtCtxt<'a> {
span,
&format!(
"cannot resolve relative path in non-file source `{}`",
other.prefer_local()
self.source_map().filename_for_diagnostics(&other)
),
));
}
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,14 +1630,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
(TypeError::Sorts(values), extra) => {
let sort_string = |ty: Ty<'tcx>| match (extra, ty.kind()) {
(true, ty::Opaque(def_id, _)) => {
let pos = self
.tcx
.sess
.source_map()
.lookup_char_pos(self.tcx.def_span(*def_id).lo());
let sm = self.tcx.sess.source_map();
let pos = sm.lookup_char_pos(self.tcx.def_span(*def_id).lo());
format!(
" (opaque type at <{}:{}:{}>)",
pos.file.name.prefer_local(),
sm.filename_for_diagnostics(&pos.file.name),
pos.line,
pos.col.to_usize() + 1,
)
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_mir/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,12 @@ impl<'tcx> fmt::Display for FrameInfo<'tcx> {
write!(f, "inside `{}`", self.instance)?;
}
if !self.span.is_dummy() {
let lo = tcx.sess.source_map().lookup_char_pos(self.span.lo());
let sm = tcx.sess.source_map();
let lo = sm.lookup_char_pos(self.span.lo());
write!(
f,
" at {}:{}:{}",
lo.file.name.prefer_local(),
sm.filename_for_diagnostics(&lo.file.name),
lo.line,
lo.col.to_usize() + 1
)?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ pub fn maybe_file_to_stream(
let src = source_file.src.as_ref().unwrap_or_else(|| {
sess.span_diagnostic.bug(&format!(
"cannot lex `source_file` without source: {}",
source_file.name.prefer_local()
sess.source_map().filename_for_diagnostics(&source_file.name)
));
});

Expand Down
31 changes: 21 additions & 10 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,12 @@ impl RealFileName {
}
}

pub fn to_string_lossy(&self, prefer_local: bool) -> Cow<'_, str> {
if prefer_local {
self.local_path_if_available().to_string_lossy()
} else {
self.remapped_path_if_available().to_string_lossy()
pub fn to_string_lossy(&self, display_pref: FileNameDisplayPreference) -> Cow<'_, str> {
match display_pref {
FileNameDisplayPreference::Local => self.local_path_if_available().to_string_lossy(),
FileNameDisplayPreference::Remapped => {
self.remapped_path_if_available().to_string_lossy()
}
}
}
}
Expand Down Expand Up @@ -300,17 +301,23 @@ impl From<PathBuf> for FileName {
}
}

#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
pub enum FileNameDisplayPreference {
Remapped,
Local,
}

pub struct FileNameDisplay<'a> {
inner: &'a FileName,
prefer_local: bool,
display_pref: FileNameDisplayPreference,
}

impl fmt::Display for FileNameDisplay<'_> {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use FileName::*;
match *self.inner {
Real(ref name) => {
write!(fmt, "{}", name.to_string_lossy(self.prefer_local))
write!(fmt, "{}", name.to_string_lossy(self.display_pref))
}
QuoteExpansion(_) => write!(fmt, "<quote expansion>"),
MacroExpansion(_) => write!(fmt, "<macro expansion>"),
Expand All @@ -328,7 +335,7 @@ impl fmt::Display for FileNameDisplay<'_> {
impl FileNameDisplay<'_> {
pub fn to_string_lossy(&self) -> Cow<'_, str> {
match self.inner {
FileName::Real(ref inner) => inner.to_string_lossy(self.prefer_local),
FileName::Real(ref inner) => inner.to_string_lossy(self.display_pref),
_ => Cow::from(format!("{}", self)),
}
}
Expand All @@ -352,13 +359,17 @@ impl FileName {
}

pub fn prefer_remapped(&self) -> FileNameDisplay<'_> {
FileNameDisplay { inner: self, prefer_local: false }
FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Remapped }
}

// This may include transient local filesystem information.
// Must not be embedded in build outputs.
pub fn prefer_local(&self) -> FileNameDisplay<'_> {
FileNameDisplay { inner: self, prefer_local: true }
FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Local }
}

pub fn display(&self, display_pref: FileNameDisplayPreference) -> FileNameDisplay<'_> {
FileNameDisplay { inner: self, display_pref }
}

pub fn macro_expansion_source_code(src: &str) -> FileName {
Expand Down
23 changes: 17 additions & 6 deletions compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl SourceMap {
}
}

fn span_to_string(&self, sp: Span, prefer_local: bool) -> String {
fn span_to_string(&self, sp: Span, filename_display_pref: FileNameDisplayPreference) -> String {
if self.files.borrow().source_files.is_empty() || sp.is_dummy() {
return "no-location".to_string();
}
Expand All @@ -436,7 +436,7 @@ impl SourceMap {
let hi = self.lookup_char_pos(sp.hi());
format!(
"{}:{}:{}: {}:{}",
if prefer_local { lo.file.name.prefer_local() } else { lo.file.name.prefer_remapped() },
lo.file.name.display(filename_display_pref),
lo.line,
lo.col.to_usize() + 1,
hi.line,
Expand All @@ -446,20 +446,24 @@ impl SourceMap {

/// Format the span location suitable for embedding in build artifacts
pub fn span_to_embeddable_string(&self, sp: Span) -> String {
self.span_to_string(sp, false)
self.span_to_string(sp, FileNameDisplayPreference::Remapped)
}

/// Format the span location to be printed in diagnostics. Must not be emitted
/// to build artifacts as this may leak local file paths. Use span_to_embeddable_string
/// for string suitable for embedding.
pub fn span_to_diagnostic_string(&self, sp: Span) -> String {
self.span_to_string(sp, true)
self.span_to_string(sp, self.path_mapping.filename_display_for_diagnostics)
}

pub fn span_to_filename(&self, sp: Span) -> FileName {
self.lookup_char_pos(sp.lo()).file.name.clone()
}

pub fn filename_for_diagnostics<'a>(&self, filename: &'a FileName) -> FileNameDisplay<'a> {
filename.display(self.path_mapping.filename_display_for_diagnostics)
}

pub fn is_multiline(&self, sp: Span) -> bool {
let lo = self.lookup_source_file_idx(sp.lo());
let hi = self.lookup_source_file_idx(sp.hi());
Expand Down Expand Up @@ -1013,15 +1017,22 @@ impl SourceMap {
#[derive(Clone)]
pub struct FilePathMapping {
mapping: Vec<(PathBuf, PathBuf)>,
filename_display_for_diagnostics: FileNameDisplayPreference,
}

impl FilePathMapping {
pub fn empty() -> FilePathMapping {
FilePathMapping { mapping: vec![] }
FilePathMapping::new(Vec::new())
}

pub fn new(mapping: Vec<(PathBuf, PathBuf)>) -> FilePathMapping {
FilePathMapping { mapping }
let filename_display_for_diagnostics = if mapping.is_empty() {
FileNameDisplayPreference::Local
} else {
FileNameDisplayPreference::Remapped
};

FilePathMapping { mapping, filename_display_for_diagnostics }
}

/// Applies any path prefix substitution as defined by the mapping.
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/remap-path-prefix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// compile-flags: --remap-path-prefix={{src-base}}=remapped

fn main() {
// We cannot actually put an ERROR marker here because
// the file name in the error message is not what the
// test framework expects (since the filename gets remapped).
// We still test the expected error in the stderr file.
ferris
}
9 changes: 9 additions & 0 deletions src/test/ui/remap-path-prefix.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0425]: cannot find value `ferris` in this scope
--> remapped/remap-path-prefix.rs:8:5
|
LL | ferris
| ^^^^^^ not found in this scope

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.
7 changes: 2 additions & 5 deletions src/tools/clippy/clippy_lints/src/macro_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,8 @@ pub struct MacroRefData {

impl MacroRefData {
pub fn new(name: String, callee: Span, cx: &LateContext<'_>) -> Self {
let mut path = cx
.sess()
.source_map()
.span_to_filename(callee)
.prefer_local()
let sm = cx.sess().source_map();
let mut path = sm.filename_for_diagnostics(&sm.span_to_filename(callee))
.to_string();

// std lib paths are <::std::module::file type>
Expand Down

0 comments on commit 97f2698

Please sign in to comment.