Don't ICE when no bound vars found while doing closure hir type check #113396

lenko-d · 2023-07-06T03:47:46Z

The problem was that we were not visiting the const generic default argument in a bound where predicate when the HIR gets traversed in hir_analysis -> collect -> resolve_bound_vars.

Fixes 112574

rustbot · 2023-07-06T03:47:53Z

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

compiler-errors

I don't think is the correct solution. Rust will still ICE if the closure has >0 inputs.

This PR also needs more description. It should at least briefly explain why this fixes the issue and/or why it's correct.

compiler/rustc_hir_typeck/src/closure.rs

WaffleLapkin · 2023-07-06T05:10:33Z

r? @compiler-errors

oli-obk · 2023-07-07T08:43:13Z

@rustbot author

bors · 2023-07-14T17:34:57Z

☔ The latest upstream changes (presumably #113591) made this pull request unmergeable. Please resolve the merge conflicts.

Dylan-DPC · 2023-08-19T17:47:35Z

@lenko-d any updates on this? thanks

lenko-d · 2023-09-22T01:48:21Z

@Dylan-DPC I'm still working on this occasionally. I believe that I have some understanding of what the problem is.

I believe that the problem is that the closure is not visited when the HIR gets traversed in hir_analysis -> collect -> resolve_bound_vars. Not sure yet why it is not visited and how to fix it.

compiler-errors · 2023-09-25T02:30:48Z

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

+                for param in bound_generic_params {
+                    match param.kind {
+                        GenericParamKind::Lifetime { .. } => {}
+                        GenericParamKind::Type { default, .. } => {
+                            if let Some(ty) = default {
+                                self.visit_ty(ty);
+                            }
+                        }
+                        GenericParamKind::Const { ty, default } => {
+                            self.visit_ty(ty);
+                            if let Some(default) = default {
+                                self.visit_body(self.tcx.hir().body(default.body));
+                            }
+                        }
+                    }
+                }


This is generally the correct fix.

Though I feel like this could be merged with the same logic for handling lifetimes in const defaults in item generics (#93669). Maybe you could move the visit_ty and visit_body calls into visit_generic_param, and then this can be replaced with:

Suggested change

for param in bound_generic_params {

match param.kind {

GenericParamKind::Lifetime { .. } => {}

GenericParamKind::Type { default, .. } => {

if let Some(ty) = default {

self.visit_ty(ty);

}

}

GenericParamKind::Const { ty, default } => {

self.visit_ty(ty);

if let Some(default) = default {

self.visit_body(self.tcx.hir().body(default.body));

}

}

}

}

walk_list!(self, visit_generic_param, bound_generic_params);

…id ICE on no bound vars.

lenko-d · 2023-09-25T11:46:00Z

@rustbot ready

compiler-errors · 2023-09-25T13:52:20Z

@bors r+

bors · 2023-09-25T13:52:22Z

📌 Commit a1d181d has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-09-25T13:53:31Z

⌛ Testing commit a1d181d with merge a1454e7...

…for_lifetime_binders, r=compiler-errors Don't ICE when no bound vars found while doing closure hir type check The problem was that we were not visiting the const generic default argument in a bound where predicate when the HIR gets traversed in hir_analysis -> collect -> resolve_bound_vars. Fixes [112574](rust-lang#112574)

bors · 2023-09-25T13:54:45Z

💔 Test failed - checks-actions

rust-log-analyzer · 2023-09-25T13:55:07Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

 "NO_DEBUG_ASSERTIONS": 1,
 "NO_OVERFLOW_CHECKS": 1
}
##[endgroup]
jq: error: Top-level program not given (try ".")
jq: 1 compile error
##[error]Process completed with exit code 3.
[command]/usr/local/bin/git version
git version 2.42.0
Copying '/Users/runner/.gitconfig' to '/Users/runner/work/_temp/81184fea-3da0-4d74-8c84-b453e02a17f9/.gitconfig'
Temporarily overriding HOME='/Users/runner/work/_temp/81184fea-3da0-4d74-8c84-b453e02a17f9' before making global git config changes

compiler-errors · 2023-09-25T14:20:57Z

@bors retry

bors · 2023-09-25T16:32:37Z

⌛ Testing commit a1d181d with merge 1cbfeab...

bors · 2023-09-25T18:28:18Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 1cbfeab to master...

bors · 2023-09-25T18:28:18Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 1cbfeab to master...

rust-timer · 2023-09-25T20:39:01Z

Finished benchmarking commit (1cbfeab): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.4%	[4.4%, 4.4%]	1
Improvements ✅ (primary)	-2.9%	[-3.8%, -1.3%]	3
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	2
All ❌✅ (primary)	-2.9%	[-3.8%, -1.3%]	3

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 632.116s -> 631.251s (-0.14%)
Artifact size: 317.31 MiB -> 317.22 MiB (-0.03%)

rustbot assigned WaffleLapkin Jul 6, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 6, 2023

compiler-errors requested changes Jul 6, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/closure.rs Outdated Show resolved Hide resolved

rustbot assigned compiler-errors and unassigned WaffleLapkin Jul 6, 2023

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 7, 2023

lenko-d force-pushed the dont_ICE_when_no_bound_vars_for_lifetime_binders branch from 6920064 to 241c75f Compare September 25, 2023 02:15

compiler-errors reviewed Sep 25, 2023

View reviewed changes

Added additional visit steps to visit_generic_param() in order to avo…

a1d181d

…id ICE on no bound vars.

lenko-d force-pushed the dont_ICE_when_no_bound_vars_for_lifetime_binders branch from 241c75f to a1d181d Compare September 25, 2023 04:41

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 25, 2023

compiler-errors approved these changes Sep 25, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2023

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 25, 2023

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 25, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 25, 2023

bors merged commit 1cbfeab into rust-lang:master Sep 25, 2023

rustbot added this to the 1.74.0 milestone Sep 25, 2023

Don't ICE when no bound vars found while doing closure hir type check #113396

Don't ICE when no bound vars found while doing closure hir type check #113396

Uh oh!

Conversation

lenko-d commented Jul 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 6, 2023

Uh oh!

compiler-errors left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WaffleLapkin commented Jul 6, 2023

Uh oh!

oli-obk commented Jul 7, 2023

Uh oh!

bors commented Jul 14, 2023

Uh oh!

Dylan-DPC commented Aug 19, 2023

Uh oh!

lenko-d commented Sep 22, 2023

Uh oh!

compiler-errors Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

lenko-d commented Sep 25, 2023

Uh oh!

compiler-errors commented Sep 25, 2023

Uh oh!

bors commented Sep 25, 2023

Uh oh!

bors commented Sep 25, 2023

Uh oh!

bors commented Sep 25, 2023

Uh oh!

rust-log-analyzer commented Sep 25, 2023

Uh oh!

compiler-errors commented Sep 25, 2023

Uh oh!

bors commented Sep 25, 2023

Uh oh!

bors commented Sep 25, 2023

Uh oh!

bors commented Sep 25, 2023

Uh oh!

rust-timer commented Sep 25, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

lenko-d commented Jul 6, 2023 •

edited

Loading

compiler-errors left a comment •

edited

Loading