Properly handle collecting default impls of methods with lifetime parameters.

[michaelwoerister]

r? @eddyb

Fixes#47309.

[michaelwoerister]

Would it be better to erase regions after applying the trait substs?

[eddyb]

Maybe if it makes the code simpler.

[emilyalbini]

Ping from triage @eddyb! This PR needs your review.

[michaelwoerister]

This could also be reviewed by somebody else from @rust-lang/compiler, if @eddyb doesn't have time.

[arielb1]

I would prefer to change push_if_root to create a proper substs for the item (with the relevant lifetime parameters substituted with ReErased) instead of adding special casing for exotic substs.

[arielb1]

Yes I'm back (somewhat).

[michaelwoerister]

I'll look into it.

[michaelwoerister]

@arielb1, push_if_root() already does that by using Instance::mono(). Default methods are a special case, as they are generic but we happen to know the type arguments. We can't really use push_if_root() here.

[bors]

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

[arielb1]

@michaelwoerister

I misread that code - you really are not using push_if_root. In any case, the "correct" way of doing things is to create a "correct" method substs with erased regions, as this code does:

rust/src/librustc/traits/mod.rs

Lines 764 to 769 in e96e54d

	// the method may have some early-bound lifetimes, add
	// regions for those
	let substs = Substs::for_item(tcx, def_id,
	|_, _| tcx.types.re_erased,
	|def, _| trait_ref.substs().type_for_def(def));

I think Substs::extend_to would also work and it might be a better refactoring to use it instead of Substs::for_item.

[michaelwoerister]

Much nicer now, thanks @arielb1!

[eddyb]

@bors r+

[bors]

📌 Commit 5218c2d has been approved by eddyb

[bors]

⌛ Testing commit 5218c2d with merge cc34ca1...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing cc34ca1 to master...