Don't use guess_head_span in predicates_of for foreign span
#88414
Conversation
|
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
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.
Aaron1011
force-pushed
the
guess-foreign-head-span
branch
from
b27a608
to
c9157ef
Compare
|
In the future, I'd like to encode the head span into the crate metadata (for items where it might be needed). That would allow us to use the trait head span in all cases, without needing to read from a from an upstream crate's on-disk source. However, that will require a much larger effort. This change should hopefully be enough to fix the |
|
For functions, |
|
I think we might want to use the full trait span in some cases (cc @estebank) |
I'm pretty sure there are cases where this is true (although I can't think of any off the top of my head), but I think it might be a handful of cases and concentrated on things that are in the local crate. |
| 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): |
There was a problem hiding this comment.
Oh, this is... like being caught coming out of the shower 😅
| 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` |
There was a problem hiding this comment.
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.
|
Let's use r? @estebank |
Where do you want me to switch to |
|
@bors r+ I misread how you were getting the span. We should still make the |
|
📌 Commit c9157ef has been approved by |
bors
added
S-waiting-on-bors
|
An unusually large number of people have been reporting @bors p=1 |
|
☀️ Test successful - checks-actions |
Previously, the result of
predicates_offor a foreign traitwould 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.