From 869df76764d867818ca9e96f665f8645e38f9ce9 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Fri, 6 Jan 2023 11:28:05 +0000 Subject: [PATCH] Heuristically undo path prefix mappings. Because the compiler produces better diagnostics if it can find the source of (potentially remapped) dependencies. --- compiler/rustc_span/src/source_map.rs | 63 +++++++++++++++++-- compiler/rustc_span/src/source_map/tests.rs | 43 +++++++++++++ tests/ui/errors/auxiliary/remapped_dep.rs | 3 + ...emap-path-prefix-reverse.local-self.stderr | 14 +++++ ...p-path-prefix-reverse.remapped-self.stderr | 14 +++++ tests/ui/errors/remap-path-prefix-reverse.rs | 23 +++++++ tests/ui/{ => errors}/remap-path-prefix.rs | 7 +++ .../ui/{ => errors}/remap-path-prefix.stderr | 2 +- 8 files changed, 162 insertions(+), 7 deletions(-) create mode 100644 tests/ui/errors/auxiliary/remapped_dep.rs create mode 100644 tests/ui/errors/remap-path-prefix-reverse.local-self.stderr create mode 100644 tests/ui/errors/remap-path-prefix-reverse.remapped-self.stderr create mode 100644 tests/ui/errors/remap-path-prefix-reverse.rs rename tests/ui/{ => errors}/remap-path-prefix.rs (58%) rename tests/ui/{ => errors}/remap-path-prefix.stderr (82%) diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index fa09b4faa441f..58857730e41b6 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -17,7 +17,7 @@ use rustc_data_structures::stable_hasher::StableHasher; use rustc_data_structures::sync::{AtomicU32, Lrc, MappedReadGuard, ReadGuard, RwLock}; use std::cmp; use std::hash::Hash; -use std::path::{Path, PathBuf}; +use std::path::{self, Path, PathBuf}; use std::sync::atomic::Ordering; use std::fs; @@ -1071,12 +1071,24 @@ impl SourceMap { pub fn ensure_source_file_source_present(&self, source_file: Lrc) -> bool { source_file.add_external_src(|| { - match source_file.name { - FileName::Real(ref name) if let Some(local_path) = name.local_path() => { - self.file_loader.read_file(local_path).ok() + let FileName::Real(ref name) = source_file.name else { + return None; + }; + + let local_path: Cow<'_, Path> = match name { + RealFileName::LocalPath(local_path) => local_path.into(), + RealFileName::Remapped { local_path: Some(local_path), .. } => local_path.into(), + RealFileName::Remapped { local_path: None, virtual_name } => { + // The compiler produces better error messages if the sources of dependencies + // are available. Attempt to undo any path mapping so we can find remapped + // dependencies. + // We can only use the heuristic because `add_external_src` checks the file + // content hash. + self.path_mapping.reverse_map_prefix_heuristically(virtual_name)?.into() } - _ => None, - } + }; + + self.file_loader.read_file(&local_path).ok() }) } @@ -1277,4 +1289,43 @@ impl FilePathMapping { } } } + + /// Attempts to (heuristically) reverse a prefix mapping. + /// + /// Returns [`Some`] if there is exactly one mapping where the "to" part is + /// a prefix of `path` and has at least one non-empty + /// [`Normal`](path::Component::Normal) component. The component + /// restriction exists to avoid reverse mapping overly generic paths like + /// `/` or `.`). + /// + /// This is a heuristic and not guaranteed to return the actual original + /// path! Do not rely on the result unless you have other means to verify + /// that the mapping is correct (e.g. by checking the file content hash). + #[instrument(level = "debug", skip(self), ret)] + fn reverse_map_prefix_heuristically(&self, path: &Path) -> Option { + let mut found = None; + + for (from, to) in self.mapping.iter() { + let has_normal_component = to.components().any(|c| match c { + path::Component::Normal(s) => !s.is_empty(), + _ => false, + }); + + if !has_normal_component { + continue; + } + + let Ok(rest) = path.strip_prefix(to) else { + continue; + }; + + if found.is_some() { + return None; + } + + found = Some(from.join(rest)); + } + + found + } } diff --git a/compiler/rustc_span/src/source_map/tests.rs b/compiler/rustc_span/src/source_map/tests.rs index 3cab59e8dbe6c..b4919af65fd0b 100644 --- a/compiler/rustc_span/src/source_map/tests.rs +++ b/compiler/rustc_span/src/source_map/tests.rs @@ -344,6 +344,10 @@ fn map_path_prefix(mapping: &FilePathMapping, p: &str) -> String { mapping.map_prefix(path(p)).0.to_string_lossy().to_string() } +fn reverse_map_prefix(mapping: &FilePathMapping, p: &str) -> Option { + mapping.reverse_map_prefix_heuristically(&path(p)).map(|q| q.to_string_lossy().to_string()) +} + #[test] fn path_prefix_remapping() { // Relative to relative @@ -480,6 +484,45 @@ fn path_prefix_remapping_expand_to_absolute() { ); } +#[test] +fn path_prefix_remapping_reverse() { + // Ignores options without alphanumeric chars. + { + let mapping = + &FilePathMapping::new(vec![(path("abc"), path("/")), (path("def"), path("."))]); + + assert_eq!(reverse_map_prefix(mapping, "/hello.rs"), None); + assert_eq!(reverse_map_prefix(mapping, "./hello.rs"), None); + } + + // Returns `None` if multiple options match. + { + let mapping = &FilePathMapping::new(vec![ + (path("abc"), path("/redacted")), + (path("def"), path("/redacted")), + ]); + + assert_eq!(reverse_map_prefix(mapping, "/redacted/hello.rs"), None); + } + + // Distinct reverse mappings. + { + let mapping = &FilePathMapping::new(vec![ + (path("abc"), path("/redacted")), + (path("def/ghi"), path("/fake/dir")), + ]); + + assert_eq!( + reverse_map_prefix(mapping, "/redacted/path/hello.rs"), + Some(path_str("abc/path/hello.rs")) + ); + assert_eq!( + reverse_map_prefix(mapping, "/fake/dir/hello.rs"), + Some(path_str("def/ghi/hello.rs")) + ); + } +} + #[test] fn test_next_point() { let sm = SourceMap::new(FilePathMapping::empty()); diff --git a/tests/ui/errors/auxiliary/remapped_dep.rs b/tests/ui/errors/auxiliary/remapped_dep.rs new file mode 100644 index 0000000000000..ef26f1cd883fb --- /dev/null +++ b/tests/ui/errors/auxiliary/remapped_dep.rs @@ -0,0 +1,3 @@ +// compile-flags: --remap-path-prefix={{src-base}}/errors/auxiliary=remapped-aux + +pub struct SomeStruct {} // This line should be show as part of the error. diff --git a/tests/ui/errors/remap-path-prefix-reverse.local-self.stderr b/tests/ui/errors/remap-path-prefix-reverse.local-self.stderr new file mode 100644 index 0000000000000..2584e3e88a6e5 --- /dev/null +++ b/tests/ui/errors/remap-path-prefix-reverse.local-self.stderr @@ -0,0 +1,14 @@ +error[E0423]: expected value, found struct `remapped_dep::SomeStruct` + --> $DIR/remap-path-prefix-reverse.rs:22:13 + | +LL | let _ = remapped_dep::SomeStruct; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use struct literal syntax instead: `remapped_dep::SomeStruct {}` + | + ::: remapped-aux/remapped_dep.rs:3:1 + | +LL | pub struct SomeStruct {} // This line should be show as part of the error. + | --------------------- `remapped_dep::SomeStruct` defined here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0423`. diff --git a/tests/ui/errors/remap-path-prefix-reverse.remapped-self.stderr b/tests/ui/errors/remap-path-prefix-reverse.remapped-self.stderr new file mode 100644 index 0000000000000..e710183322acc --- /dev/null +++ b/tests/ui/errors/remap-path-prefix-reverse.remapped-self.stderr @@ -0,0 +1,14 @@ +error[E0423]: expected value, found struct `remapped_dep::SomeStruct` + --> remapped/errors/remap-path-prefix-reverse.rs:22:13 + | +LL | let _ = remapped_dep::SomeStruct; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use struct literal syntax instead: `remapped_dep::SomeStruct {}` + | + ::: remapped-aux/remapped_dep.rs:3:1 + | +LL | pub struct SomeStruct {} // This line should be show as part of the error. + | --------------------- `remapped_dep::SomeStruct` defined here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0423`. diff --git a/tests/ui/errors/remap-path-prefix-reverse.rs b/tests/ui/errors/remap-path-prefix-reverse.rs new file mode 100644 index 0000000000000..635c4164e0f8e --- /dev/null +++ b/tests/ui/errors/remap-path-prefix-reverse.rs @@ -0,0 +1,23 @@ +// aux-build:remapped_dep.rs +// compile-flags: --remap-path-prefix={{src-base}}/errors/auxiliary=remapped-aux + +// The remapped paths are not normalized by compiletest. +// normalize-stderr-test: "\\(errors)" -> "/$1" + +// revisions: local-self remapped-self +// [remapped-self]compile-flags: --remap-path-prefix={{src-base}}=remapped + +// The paths from `remapped-self` aren't recognized by compiletest, so we +// cannot use line-specific patterns for the actual error. +// error-pattern: E0423 + +// Verify that the expected source code is shown. +// error-pattern: pub struct SomeStruct {} // This line should be show + +extern crate remapped_dep; + +fn main() { + // The actual error is irrelevant. The important part it that is should show + // a snippet of the dependency's source. + let _ = remapped_dep::SomeStruct; +} diff --git a/tests/ui/remap-path-prefix.rs b/tests/ui/errors/remap-path-prefix.rs similarity index 58% rename from tests/ui/remap-path-prefix.rs rename to tests/ui/errors/remap-path-prefix.rs index 2eef970997708..29b9c7be30122 100644 --- a/tests/ui/remap-path-prefix.rs +++ b/tests/ui/errors/remap-path-prefix.rs @@ -1,5 +1,12 @@ // compile-flags: --remap-path-prefix={{src-base}}=remapped +// The remapped paths are not normalized by compiletest. +// normalize-stderr-test: "\\(errors)" -> "/$1" + +// The remapped paths aren't recognized by compiletest, so we +// cannot use line-specific patterns. +// error-pattern: E0425 + fn main() { // We cannot actually put an ERROR marker here because // the file name in the error message is not what the diff --git a/tests/ui/remap-path-prefix.stderr b/tests/ui/errors/remap-path-prefix.stderr similarity index 82% rename from tests/ui/remap-path-prefix.stderr rename to tests/ui/errors/remap-path-prefix.stderr index ad6a35d1256cd..2f421283e6995 100644 --- a/tests/ui/remap-path-prefix.stderr +++ b/tests/ui/errors/remap-path-prefix.stderr @@ -1,5 +1,5 @@ error[E0425]: cannot find value `ferris` in this scope - --> remapped/remap-path-prefix.rs:8:5 + --> remapped/errors/remap-path-prefix.rs:15:5 | LL | ferris | ^^^^^^ not found in this scope