Unhelpful compiler message when a reference is used inside of an await block

[DusterTheFirst]

When using a reference to self in an instance method and then attempting to call another method on the self reference inside of a async block (static lifetime) i get this unhelpful error, that does not mention anywhere where the usage of self is problematic. All it shows is where the block begins, and then where the self was defined.

The actual problem was on line 106, never mentioned in the error (inside of the async block)

[hellow554]

Hey @DusterTheFirst

as the tag suggests, we need an mcve to reproduce the issue. Can you share the login function, or better the project with us? E.g. a github link or similar, or maybe you can minimize the problem.

E.g. down to one file, preferable reproduceable on the playground.

If you need any help, just post it here, we are more than willing to help you.

[DusterTheFirst]

MCVE

use async_std::task;
use std::sync::{
    atomic::{AtomicU32, Ordering},
    Arc,
};

#[async_std::main]
async fn main() {
    let problem = Problem::new();
    problem.start().await;
}

struct Problem {
    x: Arc<AtomicU32>,
}

impl Problem {
    pub fn new() -> Problem {
        Problem {
            x: Arc::new(AtomicU32::new(0)),
        }
    }

    pub async fn start(&self) {
        // An arbitrary **SAFE** access to a value held "inside" of self
        let x = self.x.clone();

        // Spawn concurrent task
        task::spawn(async move { // <== This whole block is shown as the error, even though the problem lies within one line
            loop {
                // Some arbitrary other operation that occurs using self (but is not causing a problem)
                x.fetch_sub(1, Ordering::Relaxed);

                // A call on self that is causing the problem, but not explained in the error
                self.fn_that_takes_self().await;
            }
        });

        loop {
            // This works fine
            self.fn_that_takes_self().await;
        }
    }

    pub async fn fn_that_takes_self(&self) {
        // Use the self variable
        self.x.fetch_add(1, Ordering::Relaxed);
    }
}

The soulution to the problem, is to wrap the self into an arc so that it can be accessed by the 'static task and the other task simultaneously, which is not apparent from the error message.

Unhelpful Error message

The Error message should at least mention the line where the self is being used as to make it 'static. But for the sake of developer friendliness should also provide information that the reference cannot be passed safely through tasks without using a construct from std::sync.

Workaround to allow for both tasks to access self

use async_std::task;
use std::sync::{
    atomic::{AtomicU32, Ordering},
    Arc,
};

#[async_std::main]
async fn main() {
    let problem = Arc::new(Problem::new());
    problem.start().await;
}

struct Problem {
    x: Arc<AtomicU32>,
}

impl Problem {
    pub fn new() -> Problem {
        Problem {
            x: Arc::new(AtomicU32::new(0)),
        }
    }

    pub async fn start(self: Arc<Self>) {
        // An arbitrary **SAFE** access to a value held "inside" of self
        let x = self.x.clone();
        
        let arc_self = self.clone();

        // Spawn concurrent task
        task::spawn(async move {
            loop {
                // Some arbitrary other operation that occurs using self (but is not causing a problem)
                x.fetch_sub(1, Ordering::Relaxed);

                arc_self.fn_that_takes_self().await;
            }
        });

        loop {
            // This works fine (as it should)
            self.fn_that_takes_self().await;
        }
    }

    pub async fn fn_that_takes_self(&self) {
        // Use the self variable
        self.x.fetch_add(1, Ordering::Relaxed);
    }
}

[Aaron1011]

Minimized:

fn require_static<T: 'static>(val: T) -> T { val }

struct Problem;

impl Problem {
    pub async fn start(&self) {
        require_static(async move {
            &self;
        });
    }
}

fn main() {}

[Aaron1011]

This error is emitted during type-checking. In order to emit a nicer error, I think we'll need -Z borrowck=mir to be enabled, so that we have access to MIR when we construct the error.

cc @matthewjasper

[yoshuawuyts]

This is the current compiler output (June 2021, Rust 1.53):

   Compiling playground v0.0.1 (/playground)
error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
 --> src/main.rs:6:24
  |
6 |     pub async fn start(&self) {
  |                        ^^^^^
  |                        |
  |                        this data with an anonymous lifetime `'_`...
  |                        ...is captured here...
7 |         require_static(async move {
  |         -------------- ...and is required to live as long as `'static` here

This seems like it addresses the issue raised here?

[DusterTheFirst]

@yoshuawuyts The issue, which still exists, is that the compiler does not show where in the async block is self used. The line that uses self is the problem and needs to be fixed, not the async block itself (which is what is pointed out in the error messages currently). The

...and is required to live as long as 'static here

message should point to the async block and also reference the line where self is used. Since it currently doesn't, it is confusing and unhelpful, leading to the programmer having to fish out the problem, rather than the compiler, which knows exactly where the problem occurred, showing it to them.

[estebank]

I feel it is reasonable to special case this in the typechecker, detect that this is an argument in an async fn and suggest self: Arc<Self>, likely with caveats about perf implications, but that making that change will be likely what people want.

I would also want us to look at the desugaring to avoid the following presentation, where the span for "this data" and "the capture" overlap but should likely be the same:

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
 --> f9.rs:8:24
  |
8 |     pub async fn start(&self) {
  |                        ^^^^^
  |                        |
  |                        this data with an anonymous lifetime `'_`...
  |                        ...is captured here...
9 |         require_static(async move {
  |         -------------- ...and is required to live as long as `'static` here

and if we are going to fix that, we should likely have better wording to deal with async fns in general.

---

Edit: well, I wasn't wrong to add this FIXME 😅

https://github.com/rust-lang/rust//blob/bfaf13af4e5071b09248979e51c6614bf1353f52/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs#L133

CC #62097

---

This should look closer to

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
  --> f9.rs:8:24
   |
 8 |     pub async fn start(&self) {
   |                        ^^^^^ this data with an anonymous lifetime `'_`...
 9 |         require_static(async move {
   |         -------------- ...is required to live as long as `'static` here...
10 |             &self;
   |             ----- ...because it is captured here
help: consider using a reference counter wrapper to avoid the lifetime issues
 8 -     pub async fn start(&self) {
 8 +     pub async fn start(self: Arc<Self>) {

but we don't store enough metadata to do so in the SubSupConflict. We might be able to "fake" it by walking the async fn body to look for require_static and see if it has a closure that captures the param_name. That would be hacky, but could potentially work for cases like this one. But for now we can provide the suggestion and have only one span label on line 8, relatively easily.

[DusterTheFirst]

@estebank In your example, the main missing diagnostic that would be most universally helpful is the diagnostic on line 10 that you showed:

10 |             &self;
   |             ----- ...because it is captured here

The diagnostics on line 8 and 9 are helpful in getting context, but they are not the root cause of the error. The root cause is using the self parameter in the async block. The diagnostics in its current state suggest that the block or the function signature are the 2 and only culprits.

The help messages at the end would be helpful, but not strictly necessary to address my specific "gripe" with the diagnostics.

[estebank]

@DusterTheFirst I agree with your assessment, but the problem is that given where this error is being emitted, finding that span is the hardest thing to accomplish. I wouldn't close this ticket until all the parts of the problem are addressed, but I also fear that when we delay "simple, incomplete" stop gap measures because they are... well... incomplete and simple, we (and by we I mean the project and myself) fail to help our users. If the 100% solution is delayed because of knowledge of the time and design work that it will take to get to it, we won't have helped people at all in the meantime. Does that make sense?

[eholk]

@rustbot assign @estebank

[estebank]

I have code to perform the following output, just by cleaning up the Spans:

But it doesn't fix the problem: we need to point at the capture point of &self in this error. To do so, I've been looking at LexicalResolver::collect_errors where I could see the following info, which is what we want to expose:

collect_errors: constraint=VarSubVar('_#1r, '_#0r) origin=DataBorrowed(&Problem, f16.rs:8:13: 8:18 (#0))

That constraint/origin is not being surfaced anywhere and I'm unsure how to bubble it up correctly, but at least I'm confident we have the information we need to make this error as good as it should be.

[estebank]

Still need some more work... but:

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
 --> f16.rs:6:24
  |
6 |     pub async fn start(&self) {
  |                        ^^^^^ this data with an anonymous lifetime `'_`...
7 |         require_static(async move {
  |         -------------- ...and is required to live as long as `'static` here
8 |             &self;
  |             ----- data is captured here
  |
note: `'static` obligation introduced by this bound
 --> f16.rs:1:22
  |
1 | fn require_static<T: 'static>(val: T) -> T { val }
  |                      ^^^^^^^

I need to clean it up because one of the existing tests is showing

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
  --> $DIR/issue-62097.rs:12:31
   |
LL |     pub async fn run_dummy_fn(&self) {
   |                               ^^^^^ this data with an anonymous lifetime `'_`...
LL |         foo(|| self.bar()).await;
   |         --- ...and is required to live as long as `'static` here
   |
note: data is captured here
  --> $DIR/issue-62097.rs:13:9
   |
LL |         foo(|| self.bar()).await;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
note: data is captured here
  --> $DIR/issue-62097.rs:13:9
   |
LL |         foo(|| self.bar()).await;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
note: data is captured here
  --> $DIR/issue-62097.rs:13:9
   |
LL |         foo(|| self.bar()).await;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
note: `'static` obligation introduced by this bound
  --> $DIR/issue-62097.rs:4:19
   |
LL |     F: FnOnce() + 'static
   |                   ^^^^^^^

and I still need to figure out how to deal with it, and need to verify that I'm not keeping unrelated captures around (for which I'd need a test that could potentially trigger incorrect spans) but I feel like I'm close 😃

---

Edit: satisfied with the results, now to see if the reviewer notices I've done something wrong:

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
 --> f16.rs:6:24
  |
6 |     pub async fn start(&self) {
  |                        ^^^^^ this data with an anonymous lifetime `'_`...
7 |         require_static(async move {
  |         -------------- ...is required to live as long as `'static` here...
8 |             &self;
  |             ----- ...and is captured here
  |
note: `'static` obligation introduced by this bound
 --> f16.rs:1:22
  |
1 | fn require_static<T: 'static>(val: T) -> T { val }
  |                      ^^^^^^^

[eholk]

@rustbot label +AsyncAwait-Polish