Auto merge of rust-lang#88414 - Aaron1011:guess-foreign-head-span, r=estebank

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.

Author: bors

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<String, SpanSnippetError> {
         self.span_to_source(sp, |src, start_index, end_index| {

compiler/rustc_typeck/src/collect.rs

@@ -2002,7 +2002,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),

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
+

src/test/run-make/incr-foreign-head-span/first_crate.rs

@@ -0,0 +1 @@
+pub trait OtherTrait {}

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 {}

src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr

@@ -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 {
@@ -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 {
@@ -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 {

src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr

@@ -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 {
@@ -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>; }

src/test/ui/feature-gates/feature-gate-associated_type_bounds.stderr

@@ -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 {

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<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