Report a specialized error when a `'static` obligation comes from an `impl dyn Trait` #121274

estebank · 2024-02-18T20:04:28Z

error: lifetime may not live long enough
  --> $DIR/static-impl-obligation.rs:8:27
   |
LL |     fn bar<'a>(x: &'a &'a u32) {
   |            -- lifetime `'a` defined here
LL |         let y: &dyn Foo = x;
   |                           ^ cast requires that `'a` must outlive `'static`
LL |         y.hello();
   |         --------- calling this method introduces a `'static` lifetime requirement
   |
help: relax the implicit `'static` bound on the impl
   |
LL |     impl dyn Foo + '_ {
   |                  ++++

error: lifetime may not live long enough
  --> $DIR/static-impl-obligation.rs:173:27
   |
LL |     fn bar<'a>(x: &'a &'a u32) {
   |            -- lifetime `'a` defined here
LL |         let y: &dyn Foo = x;
   |                           ^ cast requires that `'a` must outlive `'static`
LL |         y.hello();
   |         --------- calling this method introduces a `'static` lifetime requirement
   |
note: the `impl` on `(dyn p::Foo + 'static)` has `'static` lifetime requirements
  --> $DIR/static-impl-obligation.rs:169:20
   |
LL |     impl dyn Foo + 'static where Self: 'static {
   |                    ^^^^^^^             ^^^^^^^
LL |         fn hello(&self) where Self: 'static {}
   |                                     ^^^^^^^

Fix #83246, fix #80701.

error[E0478]: lifetime bound not satisfied
   --> tests/ui/lifetimes/point-at-lifetime-obligation-from-trait-in-trait-object.rs:4:21
    |
4   |     broken: Box<dyn Any + 'a>
    |                     ^^^   ^^
    |
note: lifetime parameter instantiated with the lifetime `'a` as defined here
   --> tests/ui/lifetimes/point-at-lifetime-obligation-from-trait-in-trait-object.rs:3:18
    |
3   | struct Something<'a> {
    |                  ^^
    = note: but lifetime parameter must outlive the static lifetime
note: unmet `'static` obligations introduced here
   --> rust/library/core/src/any.rs:115:16
    |
115 | pub trait Any: 'static {
    |                ^^^^^^^

Fix #83325.

rustbot · 2024-02-18T20:04:35Z

r? @compiler-errors

rustbot has assigned @compiler-errors.
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

estebank · 2024-02-18T20:04:48Z

There are a couple of things I need to clean up, but most of the logic is close to done.

fmease · 2024-02-18T23:59:12Z

Thanks for considering a different approach! <3

Could this be generalized to trait impls? I don't see why this needs to be specific to inherent impls. Ideally we would provide a very similar suggestion & note for trait impls:

trait Foo {}
impl<'a> Foo for &'a u32 {}

trait Trait { fn hello(&self) {} }

impl Trait for dyn Foo {
    fn hello(&self) {}

}
fn convert<'a>(x: &'a &'a u32) {
    let y: &dyn Foo = x;
    y.hello();
}

current stderr

error: lifetime may not live long enough
  --> src/lib.rs:11:23
   |
10 | fn convert<'a>(x: &'a &'a u32) {
   |            -- lifetime `'a` defined here
11 |     let y: &dyn Foo = x;
   |                       ^ cast requires that `'a` must outlive `'static`

estebank · 2024-02-19T00:55:05Z

@fmease with the last change to the PR, on that code:

error: lifetime may not live long enough
  --> f103.rs:11:23
   |
10 | fn convert<'a>(x: &'a &'a u32) {
   |            -- lifetime `'a` defined here
11 |     let y: &dyn Foo = x;
   |                       ^ cast requires that `'a` must outlive `'static`
12 |     y.hello();
   |     --------- calling this method introduces a `'static` lifetime requirement
   |
help: consider relaxing the implicit `'static` requirement on the impl
   |
6  | impl Trait for dyn Foo + '_ {
   |                        ++++

Edit: added to the test suite.

compiler-errors · 2024-02-19T00:55:59Z

r? fmease

rustbot · 2024-02-19T00:55:59Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustbot · 2024-02-19T19:09:46Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

…fId`)

``` error[E0478]: lifetime bound not satisfied --> tests/ui/lifetimes/point-at-lifetime-obligation-from-trait-in-trait-object.rs:4:21 | 4 | broken: Box<dyn Any + 'a> | ^^^ ^^ | note: lifetime parameter instantiated with the lifetime `'a` as defined here --> tests/ui/lifetimes/point-at-lifetime-obligation-from-trait-in-trait-object.rs:3:18 | 3 | struct Something<'a> { | ^^ = note: but lifetime parameter must outlive the static lifetime note: unmet `'static` obligations introduced here --> rust/library/core/src/any.rs:115:16 | 115 | pub trait Any: 'static { | ^^^^^^^ ``` Fix rust-lang#83325.

``` error[E0478]: lifetime bound not satisfied --> $DIR/point-at-lifetime-obligation-from-trait-in-trait-object.rs:4:27 | LL | broken: Box<dyn Any + 'a> | --- ^^ lifetime bound not satisfied | | | this requires `'static` | note: lifetime parameter instantiated with the lifetime `'a` as defined here --> $DIR/point-at-lifetime-obligation-from-trait-in-trait-object.rs:3:18 | LL | struct Something<'a> { | ^^ = note: but lifetime parameter must outlive the static lifetime note: `'static` requirement introduced here --> $SRC_DIR/core/src/any.rs:LL:COL ```

bors · 2024-03-22T22:39:56Z

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

apiraino · 2024-06-20T13:43:58Z

@estebank when you have a chance a rebase. @fmease are we good here? :)

thanks folks

@rustbot author

compiler-errors

I don't have the energy to continue reviewing this since there's a lot going on, but in general, I would really urge you to you leave this code cleaner that you found it. I'm sorry if that requires a lot of annoying work, but that's really the price of making large changes if you care for the code's long-term maintainability.

Please at the very least remove those utility methods you added to the util crate. They're footguns at best.

compiler-errors · 2024-07-11T14:31:54Z

compiler/rustc_infer/src/traits/util.rs

@@ -235,6 +236,37 @@ pub fn elaborate<'tcx, O: Elaboratable<'tcx>>(
    elaborator
 }

+pub fn elaborate_predicates_of<'tcx>(


Is this only guaranteed to be called on a trait? Never a trait item? If not, using this function is a hazard because it only computes the predicates of the item and not its parents. I would prefer if you remove it and inline it into its callsites like:

elaborate(tcx, tcx.predicates_of(def_id).instantiate_own_identity())

Though if this is intended to be called on non-top-level-items, you need to use:

elaborate(tcx, tcx.predicates_of(def_id).instantiate_identity())

(and a .map(|(clause, sp)| (clause.as_predicate(), sp)) if necessary, but you can likely rework the call-sites to operate solely on Clauses and not Predicates.)

compiler-errors · 2024-07-11T14:34:20Z

compiler/rustc_infer/src/traits/util.rs

+    )
+}
+
+pub fn filter_predicates<'tcx>(


I think this function should get moved closer to the error reporting. It doesn't seem useful to me other than for making some filter calls in error reporting shorter.

Also, like, what's up with the name? filter_predicates plus the lack of documentation plus the fact that this is in the util module makes it incredibly unclear that this is meant to filter outlives regions only. It should be called filter_outlives_regions or something.

Also, the fact that it only filters TypeOutlives obligations and always takes RegionOutlives obligations seems somewhat unintuitive

compiler-errors · 2024-07-11T14:35:31Z

compiler/rustc_middle/src/traits/mod.rs

@@ -451,6 +451,9 @@ pub enum ObligationCauseCode<'tcx> {

    /// Obligations emitted during the normalization of a weak type alias.
    TypeAlias(InternedObligationCauseCode<'tcx>, Span, DefId),
+
+    /// During borrowck we've found a method call that could have introduced a lifetime requirement.
+    MethodCallConstraint(Ty<'tcx>, Span),


Please rename this to make it clear that it's a constraint that only ever results from a region constraint category. MethodCallRegionConstraint or something would be nice.

compiler-errors · 2024-07-11T14:46:25Z