Auto merge of #86368 - michaelwoerister:lexing-ice, r=davidtwco

Disambiguate between SourceFiles from different crates even if they have the same path

This PR fixes an ICE that can occur when the compiler encounters a source file that is part of both the local crate and an upstream crate:

1. While importing source files from an upstream crate the compiler creates a `SourceFile` entry for `foo.rs` in the `SourceMap`. Since this is an imported source file its `src` field is `None`.
2. At a later point the parser encounters `foo.rs` again. It tells the `SourceMap` to load the file but because we already have an entry for `foo.rs` the `SourceMap` will return the existing version with `src == None`.
3. The parser proceeds under the assumption that `src.is_some()` and panics when actually trying to use the file's contents.

This PR fixes the issue by adding the source file's associated `CrateNum` to the `SourceMap`'s interning key. As a consequence the two instances of the file will each have a separate entry in the `SourceMap`. They just happen to share the same file path. This approach seemed less problematic to me than trying to mutate the `SourceFile` after it had already been created.

Another, more involved, approach might be to merge the `src` and the `external_src` field.

Fixes #85955

1 files changed, 31 insertions, 10 deletions

diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
index 1d45cd172b3..32031ac7071 100644
--- a/compiler/rustc_span/src/source_map.rs
+++ b/compiler/rustc_span/src/source_map.rs
@@ -117,25 +117,42 @@ impl FileLoader for RealFileLoader {
     }
 }
 
-// This is a `SourceFile` identifier that is used to correlate `SourceFile`s between
-// subsequent compilation sessions (which is something we need to do during
-// incremental compilation).
+/// This is a [SourceFile] identifier that is used to correlate source files between
+/// subsequent compilation sessions (which is something we need to do during
+/// incremental compilation).
+///
+/// The [StableSourceFileId] also contains the CrateNum of the crate the source
+/// file was originally parsed for. This way we get two separate entries in
+/// the [SourceMap] if the same file is part of both the local and an upstream
+/// crate. Trying to only have one entry for both cases is problematic because
+/// at the point where we discover that there's a local use of the file in
+/// addition to the upstream one, we might already have made decisions based on
+/// the assumption that it's an upstream file. Treating the two files as
+/// different has no real downsides.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Encodable, Decodable, Debug)]
-pub struct StableSourceFileId(u128);
+pub struct StableSourceFileId {
+    // A hash of the source file's FileName. This is hash so that it's size
+    // is more predictable than if we included the actual FileName value.
+    pub file_name_hash: u64,
+
+    // The CrateNum of the crate this source file was originally parsed for.
+    // We cannot include this information in the hash because at the time
+    // of hashing we don't have the context to map from the CrateNum's numeric
+    // value to a StableCrateId.
+    pub cnum: CrateNum,
+}
 
 // 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_name(&source_file.name)
+        StableSourceFileId::new_from_name(&source_file.name, source_file.cnum)
     }
 
-    fn new_from_name(name: &FileName) -> StableSourceFileId {
+    fn new_from_name(name: &FileName, cnum: CrateNum) -> StableSourceFileId {
         let mut hasher = StableHasher::new();
-
         name.hash(&mut hasher);
-
-        StableSourceFileId(hasher.finish())
+        StableSourceFileId { file_name_hash: hasher.finish(), cnum }
     }
 }
 
@@ -274,7 +291,7 @@ impl SourceMap {
         // be empty, so the working directory will be used.
         let (filename, _) = self.path_mapping.map_filename_prefix(&filename);
 
-        let file_id = StableSourceFileId::new_from_name(&filename);
+        let file_id = StableSourceFileId::new_from_name(&filename, LOCAL_CRATE);
 
         let lrc_sf = match self.source_file_by_stable_id(file_id) {
             Some(lrc_sf) => lrc_sf,
@@ -288,6 +305,10 @@ impl SourceMap {
                     self.hash_kind,
                 ));
 
+                // Let's make sure the file_id we generated above actually matches
+                // the ID we generate for the SourceFile we just created.
+                debug_assert_eq!(StableSourceFileId::new(&source_file), file_id);
+
                 let mut files = self.files.borrow_mut();
 
                 files.source_files.push(source_file.clone());