Skip to content

Commit

Permalink
Don't use guess_head_span in predicates_of for foreign span
Browse files Browse the repository at this point in the history
Previously, the result of `predicates_of` for a foreign trait
would depend on the *current* state of the corresponding source
file in the foreign crate. This could lead to ICEs during incremental
compilation, since the on-disk contents of the upstream source file
could potentially change without the upstream crate being recompiled.

Additionally, this ensure that that the metadata we produce for a crate
only depends on its *compiled* upstream dependencies (e.g an rlib or
rmeta file), *not* the current on-disk state of the upstream crate
source files.
  • Loading branch information
Aaron1011 committed Aug 28, 2021
1 parent ac50a53 commit c9157ef
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 15 deletions.
11 changes: 11 additions & 0 deletions compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,17 @@ impl SourceMap {
}
}

/// Returns whether or not this span points into a file
/// in the current crate. This may be `false` for spans
/// produced by a macro expansion, or for spans associated
/// with the definition of an item in a foreign crate
pub fn is_local_span(&self, sp: Span) -> bool {
let local_begin = self.lookup_byte_offset(sp.lo());
let local_end = self.lookup_byte_offset(sp.hi());
// This might be a weird span that covers multiple files
local_begin.sf.src.is_some() && local_end.sf.src.is_some()
}

/// Returns the source snippet as `String` corresponding to the given `Span`.
pub fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
self.span_to_source(sp, |src, start_index, end_index| {
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,16 @@ fn predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericPredicates<'_> {
// prove that the trait applies to the types that were
// used, and adding the predicate into this list ensures
// that this is done.
let span = tcx.sess.source_map().guess_head_span(tcx.def_span(def_id));
let mut span = tcx.def_span(def_id);
if tcx.sess.source_map().is_local_span(span) {
// `guess_head_span` reads the actual source file from
// disk to try to determine the 'head' snippet of the span.
// Don't do this for a span that comes from a file outside
// of our crate, since this would make our query output
// (and overall crate metadata) dependent on the
// *current* state of an external file.
span = tcx.sess.source_map().guess_head_span(span);
}
result.predicates =
tcx.arena.alloc_from_iter(result.predicates.iter().copied().chain(std::iter::once((
ty::TraitRef::identity(tcx, def_id).without_const().to_predicate(tcx),
Expand Down
21 changes: 21 additions & 0 deletions src/test/run-make/incr-foreign-head-span/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
include ../../run-make-fulldeps/tools.mk

# ignore-none no-std is not supported
# ignore-nvptx64-nvidia-cuda FIXME: can't find crate for 'std'

# Ensure that modifying a crate on disk (without recompiling it)
# does not cause ICEs in downstream crates.
# Previously, we would call `SourceMap.guess_head_span` on a span
# from an external crate, which would cause us to read an upstream
# source file from disk during compilation of a downstream crate
# See #86480 for more details

INCR=$(TMPDIR)/incr

all:
cp first_crate.rs second_crate.rs $(TMPDIR)
$(RUSTC) $(TMPDIR)/first_crate.rs -C incremental=$(INCR) --target $(TARGET) --crate-type lib
$(RUSTC) $(TMPDIR)/second_crate.rs -C incremental=$(INCR) --target $(TARGET) --extern first-crate=$(TMPDIR) --crate-type lib
rm $(TMPDIR)/first_crate.rs
$(RUSTC) $(TMPDIR)/second_crate.rs -C incremental=$(INCR) --target $(TARGET) --cfg second_run --crate-type lib

1 change: 1 addition & 0 deletions src/test/run-make/incr-foreign-head-span/first_crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub trait OtherTrait {}
8 changes: 8 additions & 0 deletions src/test/run-make/incr-foreign-head-span/second_crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
extern crate first_crate;
use first_crate::OtherTrait;

#[cfg(not(second_run))]
trait Foo: OtherTrait {}

#[cfg(second_run)]
trait Bar: OtherTrait {}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
note: required by a bound in `Send`
--> $SRC_DIR/core/src/marker.rs:LL:COL
|
LL | pub unsafe auto trait Send {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Send`
LL | / pub unsafe auto trait Send {
LL | | // empty.
LL | | }
| |_^ required by this bound in `Send`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Send {
Expand All @@ -25,8 +27,14 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
note: required by a bound in `Iterator`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
|
LL | pub trait Iterator {
| ^^^^^^^^^^^^^^^^^^ required by this bound in `Iterator`
LL | / pub trait Iterator {
LL | | /// The type of the elements being iterated over.
LL | | #[stable(feature = "rust1", since = "1.0.0")]
LL | | type Item;
... |
LL | | }
LL | | }
| |_^ required by this bound in `Iterator`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Iterator {
Expand All @@ -42,8 +50,14 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
note: required by a bound in `Sync`
--> $SRC_DIR/core/src/marker.rs:LL:COL
|
LL | pub unsafe auto trait Sync {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Sync`
LL | / pub unsafe auto trait Sync {
LL | | // FIXME(estebank): once support to add notes in `rustc_on_unimplemented`
LL | | // lands in beta, and it has been extended to check whether a closure is
LL | | // anywhere in the requirement chain, extend it as such (#48534):
... |
LL | | // Empty
LL | | }
| |_^ required by this bound in `Sync`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Sync {
Expand Down
20 changes: 16 additions & 4 deletions src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@ LL | type A: Iterator<Item: Debug>;
note: required by a bound in `Debug`
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
|
LL | pub trait Debug {
| ^^^^^^^^^^^^^^^ required by this bound in `Debug`
LL | / pub trait Debug {
LL | | /// Formats the value using the given formatter.
LL | | ///
LL | | /// # Examples
... |
LL | | fn fmt(&self, f: &mut Formatter<'_>) -> Result;
LL | | }
| |_^ required by this bound in `Debug`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::A as Iterator>::Item: Debug {
Expand All @@ -24,8 +30,14 @@ LL | pub trait Foo { type Out: Baz<Assoc: Default>; }
note: required by a bound in `Default`
--> $SRC_DIR/core/src/default.rs:LL:COL
|
LL | pub trait Default: Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Default`
LL | / pub trait Default: Sized {
LL | | /// Returns the "default value" for a type.
LL | | ///
LL | | /// Default values are often some kind of initial value, identity value, or anything else that
... |
LL | | fn default() -> Self;
LL | | }
| |_^ required by this bound in `Default`
help: consider further restricting the associated type
|
LL | pub trait Foo where <<Self as Foo>::Out as Baz>::Assoc: Default { type Out: Baz<Assoc: Default>; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,10 @@ LL | type A: Iterator<Item: Copy>;
note: required by a bound in `Copy`
--> $SRC_DIR/core/src/marker.rs:LL:COL
|
LL | pub trait Copy: Clone {
| ^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Copy`
LL | / pub trait Copy: Clone {
LL | | // Empty.
LL | | }
| |_^ required by this bound in `Copy`
help: consider further restricting the associated type
|
LL | trait _Tr3 where <<Self as _Tr3>::A as Iterator>::Item: Copy {
Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/traits/issue-85735.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ LL | T: FnMut(&'a ()),
note: required by a bound in `FnMut`
--> $SRC_DIR/core/src/ops/function.rs:LL:COL
|
LL | pub trait FnMut<Args>: FnOnce<Args> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `FnMut`
LL | / pub trait FnMut<Args>: FnOnce<Args> {
LL | | /// Performs the call operation.
LL | | #[unstable(feature = "fn_traits", issue = "29625")]
LL | | extern "rust-call" fn call_mut(&mut self, args: Args) -> Self::Output;
LL | | }
| |_^ required by this bound in `FnMut`

error: aborting due to previous error

Expand Down

0 comments on commit c9157ef

Please sign in to comment.