Refactor variance diagnostics to work with more types #89336
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit f97bb8adee90d5df2c6d27ddfec3f67a76f57d87 with merge 78c916af8206ddc392ba5affbc9f3443134ac39a... |
|
☀️ Try build successful - checks-actions |
|
Queued 78c916af8206ddc392ba5affbc9f3443134ac39a with parent 8f8092c, future comparison URL. |
|
Finished benchmarking commit (78c916af8206ddc392ba5affbc9f3443134ac39a): comparison url. Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never |
rustbot
added
perf-regression
cjgillot
reviewed
Sep 29, 2021
Aaron1011
force-pushed
the
variance-struct-diag
branch
from
f97bb8a to
a4fbd4d
Compare
This comment has been minimized.
This comment has been minimized.
Aaron1011
force-pushed
the
variance-struct-diag
branch
from
a4fbd4d to
85046e3
Compare
|
☔ The latest upstream changes (presumably #89549) made this pull request unmergeable. Please resolve the merge conflicts. |
apiraino
added
the
T-compiler
|
The perf looks overall a slight regression. Do you have an idea where it comes from? I am not yet certain the current diagnostic improvement justifies it. |
cjgillot
reviewed
Oct 17, 2021
src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr
Show resolved
Hide resolved
jackh726
added
S-waiting-on-author
|
triage: merge conflicts and hasn't been updated for months. |
Aaron1011
force-pushed
the
variance-struct-diag
branch
from
85046e3 to
96335f7
Compare
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 96335f7bef91b2fc617605e8e797eabe5ffab7ec with merge 8f5328a4aac51b855358a1acdef28b26309c47f9... |
|
Queued 3c38ccd60f62c2cbc57e83dc787cb23f77bad2ff with parent df96fb1, future comparison URL. |
|
Finished benchmarking commit (3c38ccd60f62c2cbc57e83dc787cb23f77bad2ff): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
rustbot
removed
the
S-waiting-on-perf
Instead of special-casing mutable pointers/references, we now support general generic types (currently, we handle `ty::Ref`, `ty::RawPtr`, and `ty::Adt`) When a `ty::Adt` is involved, we show an additional note explaining which of the type's generic parameters is invariant (e.g. the `T` in `Cell<T>`). Currently, we don't explain *why* a particular generic parameter ends up becoming invariant. In the general case, this could require printing a long 'backtrace' of types, so doing this would be more suitable for a follow-up PR. We still only handle the case where our variance switches to `ty::Invariant`.
Aaron1011
force-pushed
the
variance-struct-diag
branch
from
4c8e57c to
b15cb29
Compare
|
@cjgillot I've squashed the commits after applying your suggestion |
|
@bors r+ |
|
📌 Commit b15cb29 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit b15cb29 with merge 420b62c47e63bf158d0e0729e4220c99c0ff8ea2... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (f8d4ee7): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Apr 4, 2022
…n1011 Note invariance reason for FnDef types Fixes rust-lang#95272. Is it worthwhile even printing a variance explanation here? Or should I try to track down which function parameter is responsible for the invariance? r? `@Aaron1011` since you wrote rust-lang#89336
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Apr 5, 2022
…n1011 Note invariance reason for FnDef types Fixes rust-lang#95272. Is it worthwhile even printing a variance explanation here? Or should I try to track down which function parameter is responsible for the invariance? r? `@Aaron1011` since you wrote rust-lang#89336
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Apr 5, 2022
…n1011 Note invariance reason for FnDef types Fixes rust-lang#95272. Is it worthwhile even printing a variance explanation here? Or should I try to track down which function parameter is responsible for the invariance? r? ``@Aaron1011`` since you wrote rust-lang#89336
JohnTitor
added a commit
to JohnTitor/rust-semverver
that referenced
this pull request
Apr 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of special-casing mutable pointers/references, we
now support general generic types (currently, we handle
ty::Ref,ty::RawPtr, andty::Adt)When a
ty::Adtis involved, we show an additional noteexplaining which of the type's generic parameters is
invariant (e.g. the
TinCell<T>). Currently, we don'texplain why a particular generic parameter ends up becoming
invariant. In the general case, this could require printing
a long 'backtrace' of types, so doing this would be
more suitable for a follow-up PR.
We still only handle the case where our variance switches
to
ty::Invariant.