Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import SourceFiles from crate before decoding foreign Span #92175

Merged
merged 1 commit into from
Dec 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Import SourceFiles from crate before decoding foreign Span
Fixes #92163
Fixes #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 calling `imported_source_files()`
when we encounter a `SourceFile` with a foreign `CrateNum`.
This ensures that all `SourceFile`s from that crate are imported
into the current session.
  • Loading branch information
Aaron1011 committed Dec 23, 2021
commit d9220924dc5603730b480706d94f5d68663f69b2
4 changes: 4 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,4 +536,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);
}
}
14 changes: 14 additions & 0 deletions compiler/rustc_query_impl/src/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_session/src/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub enum Foo {
Variant
}
Original file line number Diff line number Diff line change
@@ -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!()
}
Original file line number Diff line number Diff line change
@@ -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() {}