[WIP] Fix compiler regression from 1.64.0 relating to &(dyn Trait + '_) params

[BGR360]

This is my attempt to fix #103762. It doesn't fully work. I'd like some input on what to try next.

What I did was special-case the treatment of anonymous lifetime bounds (+ '_) in trait object types when they are encountered inside a function's parameters. I simply resolved them to LifetimeRes::Infer to try to let typeck figure them out.

This does successfully get around the error that the OP of #103762 encountered, and goes through to astconv.

However, now it fails in astconv with the following error:

error[E0228]: the lifetime bound for this object type cannot be deduced from context; please supply an explicit bound
  --> src/test/ui/lifetimes/issue-103762-1.rs:19:23
   |
19 | fn b1(_: &i32, _: Box<dyn Trait + '_>) -> &str { loop {} }
   |                       ^^^^^^^^^^^^^^

And I discovered an existing test case which shows that (_: Box<dyn Trait + '_>) was previously accepted, which I found interesting. It now fails with the same error though:

error[E0228]: the lifetime bound for this object type cannot be deduced from context; please supply an explicit bound
  --> src/test/ui/regions/regions-trait-1.rs:25:18
   |
25 | fn get_v(gc: Box<dyn GetCtxt + '_>) -> usize {
   |                  ^^^^^^^^^^^^^^^^

I don't know for sure what this means, but it sounds like I'm sorta between a rock and a hard place. I think astconv probably wants Fresh lifetimes for these anonymous bounds, but resolve_fn_signature does not want that. Does there perhaps want to be a LifetimeRes::FreshIfNoOtherFresh?

I'm definitely in over my head a bit here. I've learned quite a bit about rustc_resolve in trying to do this, but the amount of time it would take me alone to learn enough about it to figure out where to go next is too much. I think I really need some direction from somebody who's familiar with this.

r? @cjgillot since it seems you've been the one doing most of the work to do more lifetime checking in rustc_resolve.

But if they're busy, then @compiler-errors is this an area of rustc I could pick your brain about at your office hours?

[compiler-errors]

is this an area of rustc I could pick your brain about at your office hours?

Feel free, though I am not super familiar, it's definitely something I can dig into.

[cjgillot]

This is likely to be incorrect in some corner cases. Could you:

• iterate on self.lifetime_ribs like resolve_anonymous_lifetime does,
• check LifetimeRibKind::Fresh?

Is the behaviour restricted to function parameters, or anywhere we have a Fresh rib?
What should impl &(dyn Trait + '_) { /**/ } mean?

[BGR360]

impl &(dyn Trait + '_) { ... } isn't allowed:

error[E0390]: cannot define inherent `impl` for primitive types
  --> /Users/ben/Documents/Rust/rust-lang/rust/src/test/ui/lifetimes/issue-103762-1.rs:42:6
   |
LL | impl &(dyn Trait + '_) {
   |      ^^^^^^^^^^^^^^^^^
   |
   = help: consider using an extension trait instead
   = note: you could also try moving the reference to uses of `dyn Trait` (such as `self`) within the implementation

I can believe that this way of doing it is brittle. Can you try to think of any more edge cases? I'm fresh out of ideas at the moment.

I'll try out your strategy of checking for the presence of the rib. Though I think you probably meant LifetimeRibKind::AnonymousCreateParameter rather than LifetimeRibKind::Fresh?

[cjgillot]

impl Trait for &(dyn Trait2 + '_) {} is allowed.

If you want corner cases:

fn foo(_: fn(&(dyn Trait + '_)) -> &(dyn Trait + '_)) {}
fn bar(_: [u8; { { fn blah() -> &(dyn Trait + '_) {} }; 5 }]) {}

I'll try out your strategy of checking for the presence of the rib. Though I think you probably meant LifetimeRibKind::AnonymousCreateParameter rather than LifetimeRibKind::Fresh?

Exactly.

[cjgillot]

The resolution is not Infer. In HIR, we should get an ImplicitObjectLifetimeDefault, but I did not create such variant in LifetimeRes. Could you create an extra ImplicitObjectLifetimeDefault variant in LifetimeRes and adapt lowering to translate it to LifetimeName::ImplicitObjectLifetimeDefault?

Infer means that type-checking will try to find the correct lifetime through type-inference. This is typically what appears in function bodies. ImplicitObjectLifetimeDefault means that we will inspect the surrounding type to apply the object lifetime rules: &'a (dyn Trait + '_) implicitly becomes &'a (dyn Trait + 'a) etc.

[BGR360]

Thank you for the pointer!

This does indeed get us past any errors in resolve and astconv, and all the tests I wrote compile as I had intended.

However, it seems ImplicitObjectLifetimeDefault results in the wrong lifetime for Box<dyn Trait + '_>, as this failed test shows:

// check-pass

struct Ctxt {
    v: usize,
}

trait GetCtxt {
    // Here the `&` is bound in the method definition:
    fn get_ctxt(&self) -> &Ctxt;
}

struct HasCtxt<'a> {
    c: &'a Ctxt,
}

impl<'a> GetCtxt for HasCtxt<'a> {
    // Ok: Have implied bound of WF(&'b HasCtxt<'a>)
    // so know 'a: 'b
    // so know &'a Ctxt <: &'b Ctxt
    fn get_ctxt<'b>(&'b self) -> &'a Ctxt {
        self.c
    }
}

fn get_v(gc: Box<dyn GetCtxt + '_>) -> usize {
    gc.get_ctxt().v
}

fn main() {
    let ctxt = Ctxt { v: 22 };
    let hc = HasCtxt { c: &ctxt };
    assert_eq!(get_v(Box::new(hc) as Box<dyn GetCtxt>), 22);
}

---- [ui] src/test/ui/regions/regions-trait-1.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/Users/ben/Documents/Rust/rust-lang/rust/build/aarch64-apple-darwin/stage1/bin/rustc" "/Users/ben/Documents/Rust/rust-lang/rust/src/test/ui/regions/regions-trait-1.rs" "-Zthreads=1" "--target=aarch64-apple-darwin" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/Users/ben/Documents/Rust/rust-lang/rust/build/aarch64-apple-darwin/test/ui/regions/regions-trait-1" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/Users/ben/Documents/Rust/rust-lang/rust/build/aarch64-apple-darwin/native/rust-test-helpers" "-L" "/Users/ben/Documents/Rust/rust-lang/rust/build/aarch64-apple-darwin/test/ui/regions/regions-trait-1/auxiliary"
stdout: none
--- stderr -------------------------------
error[E0597]: `ctxt` does not live long enough
  --> /Users/ben/Documents/Rust/rust-lang/rust/src/test/ui/regions/regions-trait-1.rs:31:27
   |
LL |     let hc = HasCtxt { c: &ctxt };
   |                           ^^^^^ borrowed value does not live long enough
LL |     assert_eq!(get_v(Box::new(hc) as Box<dyn GetCtxt>), 22);
   |                      ------------ cast requires that `ctxt` is borrowed for `'static`
LL | }
   | - `ctxt` dropped here while still borrowed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
------------------------------------------

So, I'm not sure where to go from there.

[BGR360]

^^ This is the more concerning bit. Please advise when you can.

[cjgillot]

@BGR360 I realise that I led you in a completely wrong direction. I'm sorry. The fix is actually much simpler than all this: ignore the lifetime from the trait object when looking for elision candidates.
That was the behaviour in the old HIR-based code:

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

Lines 2857 to 2861 in b276b65

	// Stay on the safe side and don't include the object
	// lifetime default (which may not end up being used).
	if !lifetime.is_elided() {
	self.visit_lifetime(lifetime);
	}

The easiest way to do that is to thread a ast::visit::BoundKind in ast::visit::LifetimeCtxt::Bound, and skip inserting anything into self.lifetime_elision_candidates if the lifetime is elided and the context is LifetimeCtxt::Bound(BoundKind::TraitObject).

[bors]

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

[Dylan-DPC]

Closing this PR as inactive. Feel free to open a new pr if you wish to continue working on this.