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 ignores borrows with projections #93648

Closed
tmiasko opened this issue Feb 4, 2022 · 1 comment · Fixed by #93751
Closed

Drop tracking ignores borrows with projections #93648

tmiasko opened this issue Feb 4, 2022 · 1 comment · Fixed by #93751
Assignees
Labels
A-async-await Area: Async & Await A-coroutines Area: Coroutines C-bug Category: This is a bug.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Feb 4, 2022

For example:

fn main() {
    let _ = async {
        let mut s = (String::new(),);
        s.0.push_str("abc");
        std::mem::drop(s);
        async {}.await;
    };
}
$ rustc +nightly-2022-01-22 --edition=2021 b.rs
error: internal compiler error: compiler/rustc_mir_transform/src/generator.rs:755:13: Broken MIR: generator contains type (String,) in MIR, but typeck only knows about {ResumeTy, impl Future<Output = [async output]>, ()} and []

The TryFrom<&PlaceWithHirId<'_>> for TrackedValue fails for places with projections, so in that case borrow is never recorded:

impl TryFrom<&PlaceWithHirId<'_>> for TrackedValue {
type Error = TrackedValueConversionError;
fn try_from(place_with_id: &PlaceWithHirId<'_>) -> Result<Self, Self::Error> {
if !place_with_id.place.projections.is_empty() {
debug!(
"TrackedValue from PlaceWithHirId: {:?} has projections, which are not supported.",
place_with_id
);
return Err(TrackedValueConversionError::PlaceProjectionsNotSupported);
}

fn borrow(
&mut self,
place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
_diag_expr_id: HirId,
_bk: rustc_middle::ty::BorrowKind,
) {
place_with_id
.try_into()
.map_or(false, |tracked_value| self.places.borrowed.insert(tracked_value));
}

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

eholk commented Feb 7, 2022

Thanks for the test case and the analysis!

It looks like I got the logic around this wrong. Basically, to goal was to have it so that consumes of a projection (e.g. drop(x.y)) are ignored, and reinitializations (x.y = foo) count as reinitializing all of x. It seems the code that ignores projections is too aggressive in ignoring things.

I'll try to get a fix out soon!

@rustbot claim

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 C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants