Make local_path in RealFileName::Remapped Option to be removed in exported metadata

7 files changed, 81 insertions, 36 deletions

diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs
index f88b05e6c8f..5c83d6c7ad5 100644
--- a/compiler/rustc_expand/src/base.rs
+++ b/compiler/rustc_expand/src/base.rs
@@ -1073,7 +1073,9 @@ impl<'a> ExtCtxt<'a> {
         if !path.is_absolute() {
             let callsite = span.source_callsite();
             let mut result = match self.source_map().span_to_filename(callsite) {
-                FileName::Real(name) => name.into_local_path(),
+                FileName::Real(name) => name
+                    .into_local_path()
+                    .expect("attempting to resolve a file path in an external file"),
                 FileName::DocTest(path, _) => path,
                 other => {
                     return Err(self.struct_span_err(
diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
index 7bb29d20e4a..03910f4e18d 100644
--- a/compiler/rustc_expand/src/expand.rs
+++ b/compiler/rustc_expand/src/expand.rs
@@ -362,7 +362,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
     // make crate a first class expansion target instead.
     pub fn expand_crate(&mut self, mut krate: ast::Crate) -> ast::Crate {
         let file_path = match self.cx.source_map().span_to_filename(krate.span) {
-            FileName::Real(name) => name.into_local_path(),
+            FileName::Real(name) => name
+                .into_local_path()
+                .expect("attempting to resolve a file path in an external file"),
             other => PathBuf::from(other.to_string()),
         };
         let dir_path = file_path.parent().unwrap_or(&file_path).to_owned();
diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs
index 4089dc88b7b..f91c0d83138 100644
--- a/compiler/rustc_expand/src/proc_macro_server.rs
+++ b/compiler/rustc_expand/src/proc_macro_server.rs
@@ -623,6 +623,7 @@ impl server::SourceFile for Rustc<'_> {
         match file.name {
             FileName::Real(ref name) => name
                 .local_path()
+                .expect("attempting to get a file path in an imported file in `proc_macro::SourceFile::path`")
                 .to_str()
                 .expect("non-UTF8 file path in `proc_macro::SourceFile::path`")
                 .to_string(),
diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs
index 74325c62ed3..4cb90d5a6d6 100644
--- a/compiler/rustc_metadata/src/rmeta/decoder.rs
+++ b/compiler/rustc_metadata/src/rmeta/decoder.rs
@@ -1676,7 +1676,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
                                     new_path.display(),
                                 );
                                 let new_name = rustc_span::RealFileName::Remapped {
-                                    local_path: new_path,
+                                    local_path: Some(new_path),
                                     virtual_name,
                                 };
                                 *old_name = new_name;
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
index fbae2278f1e..9a398d902a7 100644
--- a/compiler/rustc_metadata/src/rmeta/encoder.rs
+++ b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -497,9 +497,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                             RealFileName::LocalPath(local_path) => {
                                 Path::new(&working_dir).join(local_path).into()
                             }
-                            RealFileName::Remapped { local_path, virtual_name } => {
+                            RealFileName::Remapped { local_path: _, virtual_name } => {
                                 FileName::Real(RealFileName::Remapped {
-                                    local_path: Path::new(&working_dir).join(local_path),
+                                    // We do not want any local path to be exported into metadata
+                                    local_path: None,
                                     virtual_name: virtual_name.clone(),
                                 })
                             }
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs
index 44617b5aa00..e5e6f31886c 100644
--- a/compiler/rustc_span/src/lib.rs
+++ b/compiler/rustc_span/src/lib.rs
@@ -115,38 +115,80 @@ scoped_tls::scoped_thread_local!(pub static SESSION_GLOBALS: SessionGlobals);
 
 // FIXME: We should use this enum or something like it to get rid of the
 // use of magic `/rust/1.x/...` paths across the board.
-#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
-#[derive(HashStable_Generic, Decodable, Encodable)]
+#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd)]
+#[derive(HashStable_Generic, Decodable)]
 pub enum RealFileName {
     LocalPath(PathBuf),
     /// For remapped paths (namely paths into libstd that have been mapped
     /// to the appropriate spot on the local host's file system, and local file
     /// system paths that have been remapped with `FilePathMapping`),
     Remapped {
-        /// `local_path` is the (host-dependent) local path to the file.
-        local_path: PathBuf,
+        /// `local_path` is the (host-dependent) local path to the file. This is
+        /// None if the file was imported from another crate
+        local_path: Option<PathBuf>,
         /// `virtual_name` is the stable path rustc will store internally within
         /// build artifacts.
         virtual_name: PathBuf,
     },
 }
 
+impl Hash for RealFileName {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        // To prevent #70924 from happening again we should only hash the
+        // remapped (virtualized) path if that exists. This is because
+        // virtualized paths to sysroot crates (/rust/$hash or /rust/$version)
+        // remain stable even if the corresponding local_path changes
+        self.remapped_path_if_available().hash(state)
+    }
+}
+
+// This is functionally identical to #[derive(Encodable)], with the exception of
+// an added assert statement
+impl<S: Encoder> Encodable<S> for RealFileName {
+    fn encode(&self, encoder: &mut S) -> Result<(), S::Error> {
+        encoder.emit_enum("RealFileName", |encoder| match *self {
+            RealFileName::LocalPath(ref local_path) => {
+                encoder.emit_enum_variant("LocalPath", 0, 1, |encoder| {
+                    Ok({
+                        encoder.emit_enum_variant_arg(0, |encoder| local_path.encode(encoder))?;
+                    })
+                })
+            }
+
+            RealFileName::Remapped { ref local_path, ref virtual_name } => encoder
+                .emit_enum_variant("Remapped", 1, 2, |encoder| {
+                    // For privacy and build reproducibility, we must not embed host-dependant path in artifacts
+                    // if they have been remapped by --remap-path-prefix
+                    assert!(local_path.is_none());
+                    Ok({
+                        encoder.emit_enum_variant_arg(0, |encoder| local_path.encode(encoder))?;
+                        encoder.emit_enum_variant_arg(1, |encoder| virtual_name.encode(encoder))?;
+                    })
+                }),
+        })
+    }
+}
+
 impl RealFileName {
-    /// Returns the path suitable for reading from the file system on the local host.
+    /// Returns the path suitable for reading from the file system on the local host,
+    /// if this information exists.
     /// Avoid embedding this in build artifacts; see `stable_name()` for that.
-    pub fn local_path(&self) -> &Path {
+    pub fn local_path(&self) -> Option<&Path> {
         match self {
-            RealFileName::LocalPath(p)
-            | RealFileName::Remapped { local_path: p, virtual_name: _ } => &p,
+            RealFileName::LocalPath(p) => Some(p),
+            RealFileName::Remapped { local_path: p, virtual_name: _ } => {
+                p.as_ref().map(PathBuf::as_path)
+            }
         }
     }
 
-    /// Returns the path suitable for reading from the file system on the local host.
+    /// Returns the path suitable for reading from the file system on the local host,
+    /// if this information exists.
     /// Avoid embedding this in build artifacts; see `stable_name()` for that.
-    pub fn into_local_path(self) -> PathBuf {
+    pub fn into_local_path(self) -> Option<PathBuf> {
         match self {
-            RealFileName::LocalPath(p)
-            | RealFileName::Remapped { local_path: p, virtual_name: _ } => p,
+            RealFileName::LocalPath(p) => Some(p),
+            RealFileName::Remapped { local_path: p, virtual_name: _ } => p,
         }
     }
 
diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
index 03be11f828b..4e60d071c68 100644
--- a/compiler/rustc_span/src/source_map.rs
+++ b/compiler/rustc_span/src/source_map.rs
@@ -133,9 +133,6 @@ impl StableSourceFileId {
     fn new_from_name(name: &FileName) -> StableSourceFileId {
         let mut hasher = StableHasher::new();
 
-        // If name was remapped, we need to take both the local path
-        // and stablised path into account, in case two different paths were
-        // mapped to the same
         name.hash(&mut hasher);
 
         StableSourceFileId(hasher.finish())
@@ -954,7 +951,13 @@ 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.local_path()).ok(),
+            FileName::Real(ref name) => {
+                if let Some(local_path) = name.local_path() {
+                    self.file_loader.read_file(local_path).ok()
+                } else {
+                    None
+                }
+            }
             _ => None,
         })
     }
@@ -999,23 +1002,17 @@ impl FilePathMapping {
     fn map_filename_prefix(&self, file: &FileName) -> (FileName, bool) {
         match file {
             FileName::Real(realfile) => {
-                // If the file is the Name variant with only local_path, then clearly we want to map that
-                // to a virtual_name
-                // If the file is already remapped, then we want to map virtual_name further
-                // but we leave local_path alone
-                let path = realfile.stable_name();
-                let (mapped_path, mapped) = self.map_prefix(path.to_path_buf());
-                if mapped {
-                    let mapped_realfile = match realfile {
-                        RealFileName::LocalPath(local_path)
-                        | RealFileName::Remapped { local_path, virtual_name: _ } => {
-                            RealFileName::Remapped {
-                                local_path: local_path.clone(),
-                                virtual_name: mapped_path,
-                            }
+                if let RealFileName::LocalPath(local_path) = realfile {
+                    let (mapped_path, mapped) = self.map_prefix(local_path.to_path_buf());
+                    let realfile = if mapped {
+                        RealFileName::Remapped {
+                            local_path: Some(local_path.clone()),
+                            virtual_name: mapped_path,
                         }
+                    } else {
+                        realfile.clone()
                     };
-                    (FileName::Real(mapped_realfile), mapped)
+                    (FileName::Real(realfile), mapped)
                 } else {
                     unreachable!("attempted to remap an already remapped filename");
                 }