Fix the ICE described in #83693

[FabianWolff]

This pull request fixes #83693 and fixes #84768.

[rust-highfive]

r? @petrochenkov

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

[petrochenkov]

I'm sufficiently sure that this patch treats symptoms rather than the root cause.

The minimized self-contained reproduction for this case is

#![feature(associated_type_defaults)]
#![feature(unboxed_closures)]

#[rustc_paren_sugar]
trait Tr<T> {
    type Output = u8;
    fn method() {}
}

impl<T> Tr<T> for u8 {}

fn main() {
    <u8 as Tr(&u8)>::method; // Sugared, ICE
    <u8 as Tr<(), Output = &u8>>::method; // Desugared, no ICE
}

What is the difference between the sugared and desugared cases?
Can we reproduce the ICE in desugared form?
Can we produce a desugaring not causing the ICE?

[FabianWolff]

I'm sufficiently sure that this patch treats symptoms rather than the root cause.

It seems that you are right; the issue is more complex than I thought at first.

Thanks for the example that you've provided; however, I think your desugaring is wrong and should look like this:

    <u8 as Tr(&u8)>::method; // ICE
    <u8 as Tr(&u8) -> ()>::method; // equivalent, ICE
    <u8 as Tr<(&u8,), Output = ()>>::method; // desugared, no ICE

Neither of the following cause an ICE:

    <u8 as Tr<(), Output = &u8>>::method;
    <u8 as Tr() -> &u8>::method;

The ICE thus probably has to do with the special treatment of lifetime elision for parenthesized generic trait arguments:

rust/compiler/rustc_resolve/src/late/lifetimes.rs

Lines 2411 to 2417 in 9d93819

	if generic_args.parenthesized {
	let was_in_fn_syntax = self.is_in_fn_syntax;
	self.is_in_fn_syntax = true;
	self.visit_fn_like_elision(generic_args.inputs(), Some(generic_args.bindings[0].ty()));
	self.is_in_fn_syntax = was_in_fn_syntax;
	return;
	}

So probably visit_fn_like_elision() should create a binder somewhere but doesn't. However, I know too little about this function to figure out where or how to fix this; do you happen to have an idea?

[petrochenkov]

do you happen to have an idea?

No (perhaps @jackh726 may know something right away).
I can investigate, but I won't get to it soon, so I suggest to dig into the code yourself, there's no rocket science in those parts of the compiler.

[jackh726]

I do know, not quite off of my head. On mobile so will followup next time I get on my computer.

If I remember correctly, visit_fn_like_elision doesn't create the binders, but instead uses the closest binders in scope.

[petrochenkov]

r? @jackh726
(I'd taken two PRs from your queue instead.)

[jackh726]

So, we probably should be bailing out of visit_fn_like_elision sooner, but we aren't.

https://github.com/rust-lang/rust/blob/master/compiler/rustc_resolve/src/late/lifetimes.rs#L2686-L2707

We actually should probably be bailing on Scope::Body too. I'm not sure if we also need to manually set any lifetimes to 'static. I think that should be the default. And that'll prevent an escaping bound var. (This is another place where a ReError would be nice)

[FabianWolff]

We actually should probably be bailing on Scope::Body too.

Sorry for not getting around to implementing this sooner @jackh726. But yes, this seems to fix the problem; thanks for your help! I've also added a test case for #84768, which was caused by the same problem.

[JohnCSimon]

Ping from triage:
@FabianWolff is this ready for review?
If so - remove the S-waiting-on-author label and add S-waiting-on-review

[FabianWolff]

@FabianWolff is this ready for review?

Yes, it is.

@rustbot label: +S-waiting-on-review -S-waiting-on-author

[jackh726]

Sorry it's taken a bit to get back to this.

@bors r+ rollup

[bors]

📌 Commit 6f0fe9b has been approved by jackh726

[FabianWolff]

Sorry it's taken a bit to get back to this.

No problem, thanks for the review!

[bors]

⌛ Testing commit 6f0fe9b with merge 478126c...

[bors]

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 478126c to master...