Auto merge of #88363 - michaelwoerister:remapped-diagnostics, r=estebank

bors · bors · commit 97f2698484c2 · 2021-09-03T00:23:10.000Z
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`
diff --git a/compiler/rustc_codegen_cranelift/src/debuginfo/mod.rs b/compiler/rustc_codegen_cranelift/src/debuginfo/mod.rs
@@ -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();
diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs
@@ -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;
diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
@@ -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};
@@ -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.
@@ -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;
diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
@@ -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()
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
@@ -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,
                         ),
@@ -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,
                         ),
@@ -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 {
diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs
@@ -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,
diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs
@@ -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)
                         ),
                     ));
                 }
diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -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,
                             )
diff --git a/compiler/rustc_mir/src/interpret/eval_context.rs b/compiler/rustc_mir/src/interpret/eval_context.rs
@@ -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
                 )?;
diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs
@@ -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)
         ));
     });
 
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs
@@ -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()
+            }
         }
     }
 }
@@ -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>"),
@@ -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)),
         }
     }
@@ -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 {
diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
@@ -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();
         }
@@ -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,
@@ -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());
@@ -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.
diff --git a/src/test/ui/remap-path-prefix.rs b/src/test/ui/remap-path-prefix.rs
@@ -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
+}
diff --git a/src/test/ui/remap-path-prefix.stderr b/src/test/ui/remap-path-prefix.stderr
@@ -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`.
diff --git a/src/tools/clippy/clippy_lints/src/macro_use.rs b/src/tools/clippy/clippy_lints/src/macro_use.rs
@@ -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>

Original file line number	Diff line number	Diff line change
`@@ -126,7 +126,7 @@ impl AnnotateSnippetEmitterWriter {`
`126`	`126`	`}`
`127`	`127`	`// owned: line source, line index, annotations`
`128`	`128`	`type Owned = (String, usize, Vec<crate::snippet::Annotation>);`
`129`		`- let filename = primary_lo.file.name.prefer_local();`
	`129`	`+ let filename = source_map.filename_for_diagnostics(&primary_lo.file.name);`
`130`	`130`	`let origin = filename.to_string_lossy();`
`131`	`131`	`let annotated_files: Vec<Owned> = annotated_files`
`132`	`132`	`.into_iter()`
Original file line number	Diff line number	Diff line change
`@@ -1113,7 +1113,7 @@ impl<'a> ExtCtxt<'a> {`
`1113`	`1113`	`span,`
`1114`	`1114`	`&format!(`
`1115`	`1115`	"cannot resolve relative path in non-file source `{}`",
`1116`		`- other.prefer_local()`
	`1116`	`+ self.source_map().filename_for_diagnostics(&other)`
`1117`	`1117`	`),`
`1118`	`1118`	`));`
`1119`	`1119`	`}`