From c9157efad6e3e7b1802a2c4f5122140e1752985d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 27 Aug 2021 21:13:08 -0500 Subject: [PATCH] Don't use `guess_head_span` in `predicates_of` for foreign span 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. --- compiler/rustc_span/src/source_map.rs | 11 ++++++++ compiler/rustc_typeck/src/collect.rs | 11 +++++++- .../run-make/incr-foreign-head-span/Makefile | 21 +++++++++++++++ .../incr-foreign-head-span/first_crate.rs | 1 + .../incr-foreign-head-span/second_crate.rs | 8 ++++++ .../bad-bounds-on-assoc-in-trait.stderr | 26 ++++++++++++++----- .../bounds-on-assoc-in-trait.stderr | 20 +++++++++++--- ...feature-gate-associated_type_bounds.stderr | 6 +++-- src/test/ui/traits/issue-85735.stderr | 8 ++++-- 9 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 src/test/run-make/incr-foreign-head-span/Makefile create mode 100644 src/test/run-make/incr-foreign-head-span/first_crate.rs create mode 100644 src/test/run-make/incr-foreign-head-span/second_crate.rs diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 2c3af802be55a..c2de5ed2f5838 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -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 { self.span_to_source(sp, |src, start_index, end_index| { diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index b514176ad529d..51b8b8736a18d 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -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), diff --git a/src/test/run-make/incr-foreign-head-span/Makefile b/src/test/run-make/incr-foreign-head-span/Makefile new file mode 100644 index 0000000000000..712965eaa883a --- /dev/null +++ b/src/test/run-make/incr-foreign-head-span/Makefile @@ -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 + diff --git a/src/test/run-make/incr-foreign-head-span/first_crate.rs b/src/test/run-make/incr-foreign-head-span/first_crate.rs new file mode 100644 index 0000000000000..69dd103bfc02b --- /dev/null +++ b/src/test/run-make/incr-foreign-head-span/first_crate.rs @@ -0,0 +1 @@ +pub trait OtherTrait {} diff --git a/src/test/run-make/incr-foreign-head-span/second_crate.rs b/src/test/run-make/incr-foreign-head-span/second_crate.rs new file mode 100644 index 0000000000000..102f6b26c8d75 --- /dev/null +++ b/src/test/run-make/incr-foreign-head-span/second_crate.rs @@ -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 {} diff --git a/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr b/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr index a694f88192ed6..57aacf67e05fd 100644 --- a/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr +++ b/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr @@ -8,8 +8,10 @@ LL | type C: Clone + Iterator 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 <::C as Iterator>::Item: Send { @@ -25,8 +27,14 @@ LL | type C: Clone + Iterator 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 <::C as Iterator>::Item: Iterator { @@ -42,8 +50,14 @@ LL | type C: Clone + Iterator 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 <::C as Iterator>::Item: Sync { diff --git a/src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr b/src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr index 775fe28f00d83..4da5a2cbd41a6 100644 --- a/src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr +++ b/src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr @@ -8,8 +8,14 @@ LL | type A: Iterator; 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 <::A as Iterator>::Item: Debug { @@ -24,8 +30,14 @@ LL | pub trait Foo { type Out: Baz; } 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 <::Out as Baz>::Assoc: Default { type Out: Baz; } diff --git a/src/test/ui/feature-gates/feature-gate-associated_type_bounds.stderr b/src/test/ui/feature-gates/feature-gate-associated_type_bounds.stderr index f24c3d69a265b..8df5fbcc7eba2 100644 --- a/src/test/ui/feature-gates/feature-gate-associated_type_bounds.stderr +++ b/src/test/ui/feature-gates/feature-gate-associated_type_bounds.stderr @@ -142,8 +142,10 @@ LL | type A: Iterator; 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 <::A as Iterator>::Item: Copy { diff --git a/src/test/ui/traits/issue-85735.stderr b/src/test/ui/traits/issue-85735.stderr index 7d7f6ea30ae31..1f81fa72547df 100644 --- a/src/test/ui/traits/issue-85735.stderr +++ b/src/test/ui/traits/issue-85735.stderr @@ -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: FnOnce { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `FnMut` +LL | / pub trait FnMut: FnOnce { +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