Auto merge of rust-lang#92175 - Aaron1011:fix-missing-source-file, r=cjgillot

Import `SourceFile`s from crate before decoding foreign `Span`

Fixes rust-lang#92163
Fixes rust-lang#92014

When writing to the incremental cache, we encode all `Span`s
we encounter, regardless of whether or not their `SourceFile`
comes from the local crate, or from a foreign crate.

When we decode a `Span`, we use the `StableSourceFileId` we encoded
to locate the matching `SourceFile` in the current session. If this
id corresponds to a `SourceFile` from another crate, then we need to
have already imported that `SourceFile` into our current session.

This usually happens automatically during resolution / macro expansion,
when we try to resolve definitions from other crates. In certain cases,
however, we may try to load a `Span` from a transitive dependency
without having ever imported the `SourceFile`s from that crate, leading
to an ICE.

This PR fixes the issue by enconding the `SourceFile`'s `CrateNum`
when we encode a `Span`. During decoding, we call `imported_source_files()`
when we encounter a foreign `CrateNum`, which ensure that all
`SourceFile`s from that crate are imported into the current session.

Author: bors

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -554,4 +554,8 @@ impl CrateStore for CStore {
     ) -> ExpnId {
         self.get_crate_data(cnum).expn_hash_to_expn_id(sess, index_guess, hash)
     }
+
+    fn import_source_files(&self, sess: &Session, cnum: CrateNum) {
+        self.get_crate_data(cnum).imported_source_files(sess);
+    }
 }

compiler/rustc_query_impl/src/on_disk_cache.rs

@@ -495,6 +495,20 @@ impl<'a, 'tcx> CacheDecoder<'a, 'tcx> {
             .entry(index)
             .or_insert_with(|| {
                 let stable_id = file_index_to_stable_id[&index].translate(tcx);
+
+                // If this `SourceFile` is from a foreign crate, then make sure
+                // that we've imported all of the source files from that crate.
+                // This has usually already been done during macro invocation.
+                // However, when encoding query results like `TypeckResults`,
+                // we might encode an `AdtDef` for a foreign type (because it
+                // was referenced in the body of the function). There is no guarantee
+                // that we will load the source files from that crate during macro
+                // expansion, so we use `import_source_files` to ensure that the foreign
+                // source files are actually imported before we call `source_file_by_stable_id`.
+                if stable_id.cnum != LOCAL_CRATE {
+                    self.tcx.cstore_untracked().import_source_files(self.tcx.sess, stable_id.cnum);
+                }
+
                 source_map
                     .source_file_by_stable_id(stable_id)
                     .expect("failed to lookup `SourceFile` in new context")

compiler/rustc_session/src/cstore.rs

@@ -201,6 +201,12 @@ pub trait CrateStore: std::fmt::Debug {
         index_guess: u32,
         hash: ExpnHash,
     ) -> ExpnId;
+
+    /// Imports all `SourceFile`s from the given crate into the current session.
+    /// This normally happens automatically when we decode a `Span` from
+    /// that crate's metadata - however, the incr comp cache needs
+    /// to trigger this manually when decoding a foreign `Span`
+    fn import_source_files(&self, sess: &Session, cnum: CrateNum);
 }
 
 pub type CrateStoreDyn = dyn CrateStore + sync::Sync;

src/test/incremental/issue-92163-missing-sourcefile/auxiliary/first_crate.rs

@@ -0,0 +1,3 @@
+pub enum Foo {
+    Variant
+}

src/test/incremental/issue-92163-missing-sourcefile/auxiliary/second_crate.rs

@@ -0,0 +1,10 @@
+// compile-flags:--extern first_crate
+
+// Note: adding `first_crate` to the extern prelude
+// (instead of using `extern_crate`) appears to be necessary to
+// trigger the original incremental compilation bug.
+// I'm not entirely sure why this is the case
+
+pub fn make_it() -> first_crate::Foo {
+    panic!()
+}

src/test/incremental/issue-92163-missing-sourcefile/issue_92163_main.rs

@@ -0,0 +1,31 @@
+// aux-build:first_crate.rs
+// aux-build:second_crate.rs
+// revisions:rpass1 rpass2
+
+// Regression test for issue #92163
+// Under certain circumstances, we may end up trying to
+// decode a foreign `Span` from the incremental cache, without previously
+// having imported the `SourceFile`s from the owning crate. This can happen
+// if the `Span` comes from a transitive dependency (so we never try to resolve
+// items from the crate during expansion/resolution).
+//
+// Previously, this would result in an ICE, since we would not have loaded
+// the corresponding `SourceFile` for the `StableSourceFileId` we decoded.
+// This test verifies that the decoding of a foreign `Span` will always
+// try to import the `SourceFile`s from the foreign crate, instead of
+// relying on that having already happened during expansion.
+
+extern crate second_crate;
+
+pub struct Outer;
+
+impl Outer {
+    pub fn use_it() {
+        // This returns `first_crate::Foo`, causing
+        // us to encode the `AdtDef `first_crate::Foo` (along with its `Span`s)
+        // into the query cache for the `TypeckResults` for this function.
+        second_crate::make_it();
+    }
+}
+
+fn main() {}