Fix ICE with inferred type in const or static item

[FabianWolff]

Fixes #88643. Currently, there is a check in the compiler that handles the special case of a constant/static variable having a trait object type:

rust/compiler/rustc_typeck/src/collect.rs

Lines 837 to 838 in 840acd3

	// (#75889): Account for `const C: dyn Fn() -> _ = "";`
	if let hir::TyKind::TraitObject(..) = ty.kind {

But this fails to account for the possibility that the dyn type could be nested (as in Vec<dyn Fn(& _)>), which causes the ICE described in #88643.

[rust-highfive]

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

[michaelwoerister]

r? @nikomatsakis maybe?

[bors]

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

[nikomatsakis]

I'm not overly familiar with this code.

[nikomatsakis]

this span doesn't seem as helpful as before :(

[FabianWolff]

Fixed, though there are now a few almost-duplicate diagnostics if the static or const item has a function pointer type. I haven't been able to track this down/avoid this.

[nikomatsakis]

Sorry, my review comments were ... kind of inscrutable! I was in a hurry. What I wanted to ask was: can you explain what's going wrong? I'm not overly familiar with this code and I can't quite tell what it's intent is.

[FabianWolff]

Sure, although I'm no expert on this code either, I just tried to fix the bug in #88643.

Basically, as you can see e.g. in #88643 (comment), the main problem is:

error: internal compiler error: bad_placeholder_type
  --> ice.rs:11:64
   |
11 | static CALLBACKS: HashMap<*const dyn WidgetExt, dyn FnMut(&mut _) + 'static> = HashMap::new();
   |                                                                ^
   |
   = note: delayed at compiler/rustc_typeck/src/collect.rs:390:20

In other words, there should be an error about the use of _ in the type of a static/const item, but there isn't. From my understanding, this error should be emitted around here:

rust/compiler/rustc_typeck/src/collect/type_of.rs

Lines 355 to 377 in 25ec827

	ItemKind::Static(ty, .., body_id) => {
	if is_suggestable_infer_ty(ty) {
	infer_placeholder_type(
	tcx,
	def_id,
	body_id,
	ty.span,
	item.ident,
	"static variable",
	)
	} else {
	icx.to_ty(ty)
	}
	}
	ItemKind::Const(ty, body_id) => {
	if is_suggestable_infer_ty(ty) {
	infer_placeholder_type(
	tcx, def_id, body_id, ty.span, item.ident, "constant",
	)
	} else {
	icx.to_ty(ty)
	}
	}

infer_placeholder_type() can emit errors, but apparently it doesn't handle dyn types properly, which is why there already is some special case handling here:

rust/compiler/rustc_typeck/src/collect.rs

Lines 833 to 834 in 25ec827

	// (#75889): Account for `const C: dyn Fn() -> _ = "";`
	if let hir::TyKind::TraitObject(..) = ty.kind {

However, the latter check only handles dyn as the outermost type, but in #88643, the dyn appears as part of a generic argument to HashMap (i.e. ty.kind is not TraitObject), which is not caught by this check.

I have therefore extended this check to not only look at the outermost type kind, but to actually visit the type and see whether it finds a _ somewhere. I have wrapped the check in if !is_suggestable_infer_ty(ty) {} to match the check in type_of.rs to avoid duplicate diagnostics, but apparently a few of them still get through.

[pnkfelix]

@rustbot assign @pnkfelix

[bors]

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

[Dylan-DPC]

@FabianWolff if you can rebase this, we can push this forward

[JohnCSimon]

@FabianWolff
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this. Thanks for your contribution.

@rustbot label: +S-inactive