Skip to content

Don't use guess_head_span in predicates_of for foreign span #88414

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

Merged
merged 1 commit into from
Aug 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
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`
Comment on lines 27 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing all of these I think we need to extend the metadata to have clearer wording here. We are pointing at a whole trait while saying "this bound in the trait". The trait itself is "the bound". I don't know what better wording would be.

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):
Comment on lines +54 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is... like being caught coming out of the shower 😅

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