Normalize closure signature after construction #101708
Conversation
rustbot
added
the
T-compiler
|
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Doing a perf run because normalizing always might be too expensive -- also quite possible to just check if the inputs/outputs have escaping bound vars first, then do the normalization only if needed. @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 2db0492 with merge 5df914b094598d26bd074278416ca1d1d6f048ad... |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
☀️ Try build successful - checks-actions |
|
Queued 5df914b094598d26bd074278416ca1d1d6f048ad with parent fa521a4, future comparison URL. |
| } | ||
|
|
||
| fn main() { | ||
| foo(|a: <MyType as AsVariantTrait>::Type| { |
There was a problem hiding this comment.
Is it worth adding another test for the notation that tries to use the type alias Variant instead?
fn main() {
foo(|a: Variant<MyType>| {
a.field;
});
}There was a problem hiding this comment.
Oh, I can just change this test to use that. I only changed this to the associated type syntax to make sure it wasn't a bug with type aliases.
There was a problem hiding this comment.
Maybe just throw both call-sites into main and call it a day. 😆
The type alias notation is what I was originally trying to use when I found the ICE.
|
Finished benchmarking commit (5df914b094598d26bd074278416ca1d1d6f048ad): comparison URL. Overall result: no relevant changes - no action neededBenchmarking 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 may lead to changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Footnotes |
rustbot
removed
the
S-waiting-on-perf
|
Were the perf results on this PR acceptable? I'm wondering if anything is blocking the merge. |
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (9279c54): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Footnotes |
…ackh726 don't ICE when normalizing closure input tys We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx. `normalize_and_add_constraints` doesn't update `universe_causes` when creating new universes, causing an ICE. Remove it! Add spans to better track normalization constraints. Fix couple places where `universe_causes` is not updated correctly to track newly added universes. Fixes rust-lang#102800 ~Fixess rust-lang#99665~ (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by rust-lang#101708, but the changes in this PR are still correct and should prevent potential future ICEs)
…ackh726 don't ICE when normalizing closure input tys We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx. `normalize_and_add_constraints` doesn't update `universe_causes` when creating new universes, causing an ICE. Remove it! Add spans to better track normalization constraints. Fix couple places where `universe_causes` is not updated correctly to track newly added universes. Fixes rust-lang#102800 ~Fixess rust-lang#99665~ (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by rust-lang#101708, but the changes in this PR are still correct and should prevent potential future ICEs)
…ackh726 don't ICE when normalizing closure input tys We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx. `normalize_and_add_constraints` doesn't update `universe_causes` when creating new universes, causing an ICE. Remove it! Add spans to better track normalization constraints. Fix couple places where `universe_causes` is not updated correctly to track newly added universes. Fixes rust-lang#102800 ~Fixess rust-lang#99665~ (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by rust-lang#101708, but the changes in this PR are still correct and should prevent potential future ICEs)
Astconv can't normalize inputs or outputs with escaping bound vars (see this), so normalize them after we've wrapped them in a binder.
Fixes #101696