Check elaborated projections from dyn don't mention unconstrained late bound lifetimes

[compiler-errors]

Check that the projections that are not explicitly written but which we deduce from elaborating the principal of a dyn also do not reference unconstrained late-bound lifetimes, just like the ones that the user writes by hand.

That is to say, given:

trait Foo<T>: Bar<Assoc = T> {}

trait Bar {
    type Assoc;
}

The type dyn for<'a> Foo<&'a T> (basically) elaborates to dyn for<'a> Foo<&'a T> + for<'a> Bar<Assoc = &'a T>¹. However, the Bar projection predicate is not well-formed, since 'a must show up in the trait's arguments to be referenced in the term of a projection. We must error in this situation², or else dyn for<'a> Foo<&'a T> is unsound.

We already detect this for user-written projections during HIR->rustc_middle conversion, so this largely replicates that logic using the helper functions that were already conveniently defined.

---

I'm cratering this first to see the fallout; if it's minimal or zero, then let's land it as-is. If not, the way that this is implemented is very conducive to an FCW.

---

Fixes #130347

Footnotes

1. We don't actually elaborate it like that in rustc; we only keep the principal trait ref Foo<&'a T> and the projection part of Bar<Assoc = ...>, but it's useful to be a bit verbose here for the purpose of explaining the issue. ↩

2. Well, we could also make dyn for<'a> Foo<&'a T> not implement for<'a> Bar<Assoc = &'a T>, but this is inconsistent with the case where the user writes Assoc = ... in the type itself, and it overly complicates the implementation of trait objects' built-in impls. ↩

[compiler-errors]

@bors try

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

HIR ty lowering was modified

cc @fmease

[compiler-errors]

r? types

[compiler-errors]

If this ends up having too much fallout, I guess we could easily make this an FCW. But this seems pretty soundness-critical, given the number of creative ways that people have found in #130347 to make this unsound :D

[bors]

⌛ Trying commit f16d211 with merge f827120...

[bors]

☀️ Try build successful - checks-actions
Build commit: f827120 (f827120c9528a1a0e4c9881169eacd52170f9a08)

[compiler-errors]

@craterbot check

[craterbot]

👌 Experiment pr-130367 created and queued.
🤖 Automatically detected try build f827120
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[jackh726]

I'm curious why the constrained case (https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9a297b502ceb3bd1e07ceae618f9a31e) does fail today.

If it was only elaboration that played a role here, I would expect the above example to both compile (but also be unsound). The deeper problem here is likely instead around implied bounds for higher-ranked lifetimes resulting in implied 'static.

[compiler-errors]

@jackh726:

I'm curious why the constrained case (https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9a297b502ceb3bd1e07ceae618f9a31e) does fail today.

I think you wrote that example wrong. we want T to be equal to &'?0 str here, but due to the way you added that new X variable to B, it's being set to the dyn type. That's why it's resulting in a type mismatch rather than a lifetime error. I believe a more faithful way of representing this is:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9483d15a409bdeac141dd529b4465da2

Specifically, I've renamed the T variable of the Erase struct to DYN to make it clear that it isn't the same as the T variable in traits A and B.

The deeper problem here is likely instead around implied bounds for higher-ranked lifetimes resulting in implied 'static.

I don't think this has to do with implied bounds. It really does only have to do with elaboration AFAICT.

[compiler-errors]

Perhaps a more illustrative example is taking this example:

trait A<T>: B<T = T> {}

trait B {
    type T;
}

fn foo<'b>(a: &'b str) -> <dyn for<'a> A<&'a str> as B>::T {
    a
}

fn main() {
    let x = foo(String::from("hello").as_str());
    dbg!(x);
}

In order to properly constrain B, we'd need to introduce a new type parameter T to B:

trait A<T>: B<T, T = T> {}

trait B<T> {
    type T;
}

That necessitates modifying foo like:

fn foo<'b>(a: &'b str) -> <dyn for<'a> A<&'a str> as B</* what do we put here? */>>::T {
    a
}

But wait... what do we need to put in /* what do we put here? */.

• If we put &'b str, then the "lifetime extension" doesn't end up working, since the caller can no longer constrain the lifetime to 'static: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=79c6834ac84d3c593ae2305872856b6b
• We can't put &'a str because 'a is not in scope: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f5a3916e3dde17f927a3f36b0b95b920
• If we put &'static str, then the caller is happy, but the callee is unimplementable: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1e07947124d7e00e526c6e6fafdb723e

[craterbot]

🚧 Experiment pr-130367 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-130367 is completed!
📊 6 regressed and 2 fixed (513136 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

I looked at all the regressions and none seem relevant; just "no space" and "signal bus" and linker errors that every crater run features.

See the description for what this PR now validates.

@rfcbot fcp merge

[rfcbot]

Team member @compiler-errors has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @oli-obk
• @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[spastorino]

r=me after FCP finishes

[bors]

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

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[compiler-errors]

@bors r=spastorino

[bors]

📌 Commit fd7ee48 has been approved by spastorino

It is now in the queue for this repository.