Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop tracking misses variables consumed in let statements #93674

Closed
tmiasko opened this issue Feb 5, 2022 · 6 comments
Closed

Drop tracking misses variables consumed in let statements #93674

tmiasko opened this issue Feb 5, 2022 · 6 comments
Assignees
Labels
A-async-await Area: Async & Await A-coroutines Area: Coroutines AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Feb 5, 2022

For example when compiling with nightly-2022-01-22 (which included drop tracking):

#![feature(generators)]
#![feature(negative_impls)]

struct NotSend;
impl !Send for NotSend {}

fn assert_send<T: Send>(_: T) {}

fn main() {
    // Fails:
    assert_send(|| {
        let a = NotSend;
        let b = a; // a should be consumed here
        drop(b);
        yield;
    });
    // Succeeds:
    assert_send(|| {
        let a;
        a = NotSend;
        let b;
        b = a;
        drop(b);
        yield;
    });
}

This seems to be a mismatch between record_consumed_borrow::ExprUseDelegate which records a consume of a but associates it with a let statement let b = a;, while cfg_build::DropRangeVisitor considers only variables consumed by expressions.

@tmiasko tmiasko added C-bug Category: This is a bug. A-async-await Area: Async & Await A-coroutines Area: Coroutines labels Feb 5, 2022
@eholk
Copy link
Contributor

eholk commented Feb 7, 2022

Thanks for the test case! I moved this to the On Deck board. I should be able to get to it this week, but I'll leave it unclaimed in case someone else wants to get there first.

@rustbot label +AsyncAwait-Triaged +AsyncAwait-Polish

@rustbot
Copy link
Collaborator

rustbot commented Feb 7, 2022

Error: Label AsyncAwait-Triaged can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@eholk
Copy link
Contributor

eholk commented Feb 14, 2022

@rustbot label +AsyncAwait-Triaged +AsyncAwait-Polish

@rustbot rustbot added AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. labels Feb 14, 2022
@tmandry tmandry moved this to On deck in wg-async work Dec 8, 2022
@vincenzopalazzo
Copy link
Member

vincenzopalazzo commented Sep 23, 2023

Ok I think I am missing some context here, and do not have the error that was discussed
it is a little bit confusing.

So with the following example

#![feature(generators)]
#![feature(negative_impls)]

struct NotSend;
impl !Send for NotSend {}

fn assert_send<T: Send>(_: T) {}

fn main() {
    // Fails:
    assert_send(|| {
        let a = NotSend;
        let b = a; // a should be consumed here
        drop(b);
        yield;
    });
    // Succeeds:
    assert_send(|| {
        let a;
        a = NotSend;
        let b;
        b = a;
        drop(b);
        yield;
    });
}

The error generated is the following on with the commit 136d74f

➜  rust git:(macros/112850) ✗ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc test.rs --edition 2021
error: generator cannot be sent between threads safely
  --> test.rs:11:17
   |
11 |       assert_send(|| {
   |  _________________^
12 | |         let a = NotSend;
13 | |         let b = a; // a should be consumed here
14 | |         drop(b);
15 | |         yield;
16 | |     });
   | |_____^ generator is not `Send`
   |
   = help: within `{generator@test.rs:11:17: 11:19}`, the trait `Send` is not implemented for `NotSend`
note: generator is not `Send` as this value is used across a yield
  --> test.rs:15:9
   |
12 |         let a = NotSend;
   |             - has type `NotSend` which is not `Send`
...
15 |         yield;
   |         ^^^^^ yield occurs here, with `a` maybe used later
16 |     });
   |     - `a` is later dropped here
note: required by a bound in `assert_send`
  --> test.rs:7:19
   |
7  | fn assert_send<T: Send>(_: T) {}
   |                   ^^^^ required by this bound in `assert_send`

error: generator cannot be sent between threads safely
  --> test.rs:18:17
   |
18 |       assert_send(|| {
   |  _________________^
19 | |         let a;
20 | |         a = NotSend;
21 | |         let b;
...  |
24 | |         yield;
25 | |     });
   | |_____^ generator is not `Send`
   |
   = help: within `{generator@test.rs:18:17: 18:19}`, the trait `Send` is not implemented for `NotSend`
note: generator is not `Send` as this value is used across a yield
  --> test.rs:24:9
   |
19 |         let a;
   |             - has type `NotSend` which is not `Send`
...
24 |         yield;
   |         ^^^^^ yield occurs here, with `a` maybe used later
25 |     });
   |     - `a` is later dropped here
note: required by a bound in `assert_send`
  --> test.rs:7:19
   |
7  | fn assert_send<T: Send>(_: T) {}
   |                   ^^^^ required by this bound in `assert_send`

error: aborting due to 2 previous errors

@rustbot claim

@vincenzopalazzo
Copy link
Member

Mh looking at the error looks like the following help message is wrong?

   = help: within `{generator@test.rs:11:17: 11:19}`, the trait `Send` is not implemented for `NotSend`
note: generator is not `Send` as this value is used across a yield
  --> test.rs:15:9
   |
12 |         let a = NotSend;
   |             - has type `NotSend` which is not `Send`
...
15 |         yield;
   |         ^^^^^ yield occurs here, with `a` maybe used later
16 |     });
   |     - `a` is later dropped here
note: required by a bound in `assert_send`
  --> test.rs:7:19
   |
7  | fn assert_send<T: Send>(_: T) {}
   |                   ^^^^ required by this bound in `assert_send`

@tmiasko
Copy link
Contributor Author

tmiasko commented Sep 23, 2023

Note that the drop tracking is gated by -Zdrop-tracking.

The test case was fixed by -Zdrop-tracking-mir. Closing as fixed.

@tmiasko tmiasko closed this as completed Sep 23, 2023
@github-project-automation github-project-automation bot moved this from On deck to Done in wg-async work Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-coroutines Area: Coroutines AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug.
Projects
Status: Done
Development

No branches or pull requests

4 participants