Skip to content

/rust

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

Path fragments for drop obligations #18234

Merged
merged 10 commits into from Nov 25, 2014

Conversation

Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@pnkfelix @rust-highfive @alexcrichton @nikomatsakis @bors
@pnkfelix
Copy link
Member

pnkfelix commented Oct 22, 2014

Code to fragment paths into pieces based on subparts being moved around, e.g. moving x.1 out of a tuple (A,B,C) leaves behind the fragments x.0: A and x.2: C. Further discussion in borrowck/doc.rs.

Includes differentiation between assigned_fragments and moved_fragments, support for all-but-one array fragments, and instrumentation to print out the moved/assigned/unmmoved/parents for each function, factored out into a separate submodule.

These fragments can then be used by trans to inject stack-local dynamic drop flags. (They also can be hooked up with dataflow to reduce the expected number of injected flags.)
@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Oct 22, 2014

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@pnkfelix

This comment has been minimized.

Sign in to view
Copy link
Member Author

pnkfelix commented Oct 22, 2014

r? @nikomatsakis

@nikomatsakis nikomatsakis self-assigned this Oct 30, 2014

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented Nov 16, 2014

ping @pnkfelix
@pnkfelix

This comment has been minimized.

Sign in to view
Copy link
Member Author

pnkfelix commented Nov 17, 2014

@alexcrichton was that an implied request for me to rebase my PR based on @nikomatsakis 's feedback so far? I note that it seems like he is going through the commits (rather than the diff), so I would assume it might be better for me to wait until he is done with the initial review ...
@pnkfelix

This comment has been minimized.

Sign in to view
Copy link
Member Author

pnkfelix commented Nov 17, 2014

@alexcrichton (oh, in case its not clear, @nikomatsakis has already noted to me in several priv msg's that he is intending to return to this review...)
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Nov 17, 2014

@pnkfelix indeed I am doing that RIGHT NOW!
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Nov 17, 2014

ok, r+ modulo the handling of _ and *x patterns. Fix nits as you feel, nothing I noted was particularly egregious, just being persnickety.
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Nov 17, 2014

This is really nice, well documented.
@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented Nov 17, 2014

ah sorry didn't mean to annoy, just making sure the queue is moving!

@pnkfelix pnkfelix force-pushed the pnkfelix:fsk-type-fragments-for-needsdrop-2 branch 3 times, most recently from afc70f3 to 52ef4e4 Nov 20, 2014

@pnkfelix pnkfelix force-pushed the pnkfelix:fsk-type-fragments-for-needsdrop-2 branch from 52ef4e4 to de78321 Nov 24, 2014

bors added a commit that referenced this pull request Nov 24, 2014

@bors
auto merge of #18234 : pnkfelix/rust/fsk-type-fragments-for-needsdrop… 
…-2, r=nikomatsakis

Code to fragment paths into pieces based on subparts being moved around, e.g. moving `x.1` out of a tuple `(A,B,C)` leaves behind the fragments `x.0: A` and `x.2: C`.  Further discussion in borrowck/doc.rs.

Includes differentiation between assigned_fragments and moved_fragments, support for all-but-one array fragments, and instrumentation to print out the moved/assigned/unmmoved/parents for each function, factored out into a separate submodule.

These fragments can then be used by `trans` to inject stack-local dynamic drop flags.  (They also can be hooked up with dataflow to reduce the expected number of injected flags.)
7276ddb

pnkfelix added some commits Sep 16, 2014

@pnkfelix
Add `LpDowncast`, `LoanPath` variant tracking downcasts in match arms. 
`LpDowncast` carries the `DefId` of the variant itself.  To support
this, added the enum variant `DefId` to the `cat_downcast` variant in
`mem_categorization::categorization`.

(updated to fix mem_categorization to handle downcast of enum
struct-variants properly.)
d6c8f3b
@pnkfelix
Add `ty` to `LoanPath`. 
To make this clean, refactored old `LoanPath` enum into a
`LoanPath` struct with a `ty::t` and a newly-added `LoanPathVariant` enum.

This enabled me to get rid of the ugly and fragile `LoanPath::to_type`
method, and I can probably also get rid of other stuff that was
supporting it, maybe.
91b88c5
@pnkfelix
Track what drop obligations are established on match arms. 
This is accomplished by:

1. Add `MatchMode` enum to `expr_use_visitor`.

2. Computing the match mode for each pattern via a pre-pass, and then
   passing the mode along when visiting the pattern in
   expr_use_visitor.

3. Adding a `fn matched_pat` callback to expr_use_visitor, which is
   called on interior struct and enum nodes of the pattern (as opposed
   to `fn consume_pat`, which is only invoked for identifiers at the
   leaves of the pattern), and invoking it accordingly.

Of particular interest are the `cat_downcast` instances established
when matching enum variants.
09d67fd
@pnkfelix
Added fragments.rs: compute drop obligations remaining post moves. 
Includes differentiation between assigned_fragments and
moved_fragments, support for all-but-one array fragments, and
instrumentation to print out the moved/assigned/unmmoved/parents for
each function, factored out into separate submodule.
c9a1c37
@pnkfelix
First tests making use of the new fn move-fragments instrumentation. 
The tests use new "//~| ERROR" follow syntax.

Includes a test for moves involving array elements.  It was easier
than i realized to get something naive off the ground here.
Loading status checks…
5fbe0ca
@pnkfelix
Check other fields are consistent in `LoanPath::common`.
21fe017
@pnkfelix
`MemCategorizationContext::pat_ty(BindByRef)` yields type of borrowed… 
… val now.

This is to fix a problem where I could not reliably map attach the
type for each loan-path to the loan-path itself because the same
loan-path was ending up associated with two different types, because
the cmt's had diverged in their interpretation of the path.
1217b82
@pnkfelix
Override `LoanPath` Eq impl to enforce invariant: eq lp's always have… 
… eq types.
6f7cb8c

@pnkfelix pnkfelix force-pushed the pnkfelix:fsk-type-fragments-for-needsdrop-2 branch from de78321 to 5fbe0ca Nov 25, 2014

@pnkfelix

This comment has been minimized.

Sign in to view
Copy link
Owner Author

pnkfelix commented on 5fbe0ca Nov 25, 2014

r=nikomatsakis
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented on 5fbe0ca Nov 25, 2014

saw approval from nikomatsakis
at pnkfelix@5fbe0ca

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors replied Nov 25, 2014

merging pnkfelix/rust/fsk-type-fragments-for-needsdrop-2 = 5fbe0ca into auto

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors replied Nov 25, 2014

pnkfelix/rust/fsk-type-fragments-for-needsdrop-2 = 5fbe0ca merged ok, testing candidate = 0e06f71

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors replied Nov 25, 2014

fast-forwarding master to auto = 0e06f71

bors added a commit that referenced this pull request Nov 25, 2014

@bors
auto merge of #18234 : pnkfelix/rust/fsk-type-fragments-for-needsdrop… 
…-2, r=nikomatsakis

Code to fragment paths into pieces based on subparts being moved around, e.g. moving `x.1` out of a tuple `(A,B,C)` leaves behind the fragments `x.0: A` and `x.2: C`.  Further discussion in borrowck/doc.rs.

Includes differentiation between assigned_fragments and moved_fragments, support for all-but-one array fragments, and instrumentation to print out the moved/assigned/unmmoved/parents for each function, factored out into a separate submodule.

These fragments can then be used by `trans` to inject stack-local dynamic drop flags.  (They also can be hooked up with dataflow to reduce the expected number of injected flags.)
Loading status checks…
0e06f71

@bors bors closed this Nov 25, 2014

@bors bors merged commit 5fbe0ca into rust-lang:master Nov 25, 2014
2 checks passed

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.