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

[NLL] Dangly paths for box #52782

Merged
merged 6 commits into from
Aug 2, 2018

Commits on Aug 1, 2018

  1. Special-case Box in rustc_mir::borrow_check.

    This should address issue 45696.
    
    Since we know dropping a box will not access any `&mut` or `&`
    references, it is safe to model its destructor as only touching the
    contents *owned* by the box.
    
    Note: At some point we may want to generalize this machinery to other
    reference and collection types that are "pure" in the same sense as
    box. If we add a `&move` reference type, it would probably also fall
    into this branch of code. But for the short term, we will be
    conservative and restrict this change to `Box<T>` alone.
    
    The code works by recursively descending a deref of the `Box`. We
    prevent `visit_terminator_drop` infinite-loop (which can arise in a
    very obscure scenario) via a linked-list of seen types.
    
    Note: A similar style stack-only linked-list definition can be found
    in `rustc_mir::borrow_check::places_conflict`. It might be good at
    some point in the future to unify the two types and put the resulting
    definition into `librustc_data_structures/`.
    
    ----
    
    One final note: Review feedback led to significant simplification of
    logic here.
    
    During review, eddyb RalfJung and I uncovered the heart of why I
    needed a so-called "step 2" aka the Shallow Write to the Deref of the
    box. It was because the `visit_terminator_drop`, in its base case,
    will not emit any write at all (shallow or deep) to a place unless
    that place has a need_drop.
    
    So I was encoding a Shallow Write by hand for a `Box<T>`, as a
    separate step from recursively descending through `*a_box` (which was
    at the time known as "step 1"; it is now the *only* step, apart from
    the change to the base case for `visit_terminator_drop` that this
    commit now has encoded).
    
    eddyb aruged that *something* should be emitting some sort of write in
    the base case here (even a shallow one), of the dropped place, since
    by analogy we also emit a write when you *move* a place. That led
    to the revision here in this commit.
    
     * (Its possible that this desired write should be attached in some
       manner to StorageDead instead of Drop. But in this PR, I tried to
       leave the StorageDead logic alone and focus my attention solely on
       how Drop(x) is modelled in MIR-borrowck.)
    pnkfelix committed Aug 1, 2018
    Configuration menu
    Copy the full SHA
    c3618c8 View commit details
    Browse the repository at this point in the history
  2. minor fallout from the change.

    (Presumably the place that borrow_check ends up reporting for the
    error about is no longer the root `Local` itself, and thus the note
    diagnostic here stops firing.)
    pnkfelix committed Aug 1, 2018
    Configuration menu
    Copy the full SHA
    88284ba View commit details
    Browse the repository at this point in the history
  3. Regression tests.

    pnkfelix committed Aug 1, 2018
    Configuration menu
    Copy the full SHA
    08b3a8e View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    469d6a8 View commit details
    Browse the repository at this point in the history
  5. Expand long-live-borrows-in-boxes test to include simplier illustrati…

    …ve cases.
    
    After talking about the PR with eddyb, I decided it was best to try to
    have some test cases that simplify the problem down to its core, so
    that people trying to understand what the issue is here will see those
    core examples first.
    pnkfelix committed Aug 1, 2018
    Configuration menu
    Copy the full SHA
    a1b8a93 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    c02c00b View commit details
    Browse the repository at this point in the history