Add a `Place::is_indirect` method to determine whether a `Place` contains a `Deref` projection#64005

ecstatic-morse · 2019-08-29T20:39:33Z

Working on #63860 requires tracking some property about each local. This requires differentiating Places like x and x.field[index] from ones like *x and *x.field, since the first two will always access the same region of memory as x while the latter two may access any region of memory. This functionality is duplicated in various places across the compiler. This PR adds a helper method to Place which determines whether that Place has a Deref projection at any point and changes some existing code to use the new method.

I've not converted qualify_consts.rs to use the new method, since it's not a trivial conversion and it will get replaced anyway by #63860. There may be other potential uses besides the two I change in this PR.

r? @oli-obk

This returns whether a `Place` references the same region of memory as its base, or equivalently whether it contains a `Deref` projection. This is helpful for analyses that must track state for locals, since an assignment to `x` or `x.field` is fundamentally different than one to `*x`, which may mutate any memory region.

oli-obk

From the changes in this PR it looks to me like all use cases could use the find_local function instead?

oli-obk · 2019-08-29T21:15:46Z

src/librustc_mir/borrow_check/path_utils.rs

What do you think about changing the true to !place.is_indirect()?

That works better.

oli-obk · 2019-08-29T21:20:15Z

r? @spastorino this looks like something right up your alley

src/librustc_mir/dataflow/impls/borrowed_locals.rs

spastorino · 2019-09-04T04:29:44Z

Nice work and looks good to me. I'm not sure I like the name of the function but can't think of a better one right now. Any other ideas?, @oli-obk do you consider the fn name good enough? any better ideas?.

oli-obk · 2019-09-04T05:32:15Z

@bors r+

Nope, no better ideas here

bors · 2019-09-04T05:32:17Z

📌 Commit 96ac02b has been approved by oli-obk

@oli-obk

Add a `Place::is_indirect` method to determine whether a `Place` contains a `Deref` projection Working on rust-lang#63860 requires tracking some property about each local. This requires differentiating `Place`s like `x` and `x.field[index]` from ones like `*x` and `*x.field`, since the first two will always access the same region of memory as `x` while the latter two may access any region of memory. This functionality is duplicated in various places across the compiler. This PR adds a helper method to `Place` which determines whether that `Place` has a `Deref` projection at any point and changes some existing code to use the new method. I've not converted `qualify_consts.rs` to use the new method, since it's not a trivial conversion and it will get replaced anyway by rust-lang#63860. There may be other potential uses besides the two I change in this PR. r? @oli-obk

@oli-obk

Add a `Place::is_indirect` method to determine whether a `Place` contains a `Deref` projection Working on rust-lang#63860 requires tracking some property about each local. This requires differentiating `Place`s like `x` and `x.field[index]` from ones like `*x` and `*x.field`, since the first two will always access the same region of memory as `x` while the latter two may access any region of memory. This functionality is duplicated in various places across the compiler. This PR adds a helper method to `Place` which determines whether that `Place` has a `Deref` projection at any point and changes some existing code to use the new method. I've not converted `qualify_consts.rs` to use the new method, since it's not a trivial conversion and it will get replaced anyway by rust-lang#63860. There may be other potential uses besides the two I change in this PR. r? @oli-obk

@ghost

Rollup of 15 pull requests Successful merges: - #62860 (Stabilize checked_duration_since for 1.38.0) - #63549 (Rev::rposition counts from the wrong end) - #63985 (Stabilize pin_into_inner in 1.39.0) - #64005 (Add a `Place::is_indirect` method to determine whether a `Place` contains a `Deref` projection) - #64031 (Harden `param_attrs` test wrt. usage of a proc macro `#[attr]`) - #64038 (Check impl trait substs when checking for recursive types) - #64043 (Add some more tests for underscore imports) - #64092 (Update xLTO compatibility table in rustc book.) - #64110 (Refer to "`self` type" instead of "receiver type") - #64120 (Move path parsing earlier) - #64123 (Added warning around code with reference to uninit bytes) - #64128 (unused_parens: account for or-patterns and `&(mut x)`) - #64141 (Minimize uses of `LocalInternedString`) - #64142 (Fix doc links in `std::cmp` module) - #64148 (fix a few typos in comments) Failed merges: r? @ghost

rust-highfive assigned oli-obk Aug 29, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2019

ecstatic-morse force-pushed the is-indirect branch 2 times, most recently from bd8c5d1 to e9e075f Compare August 29, 2019 20:56

oli-obk reviewed Aug 29, 2019

View reviewed changes

rust-highfive assigned spastorino and unassigned oli-obk Aug 29, 2019

ecstatic-morse commented Aug 29, 2019

View reviewed changes

src/librustc_mir/dataflow/impls/borrowed_locals.rs Outdated Show resolved Hide resolved

Use new Place::is_indirect API where possible

96ac02b

ecstatic-morse force-pushed the is-indirect branch from 7542cb9 to 96ac02b Compare August 30, 2019 21:08

spastorino approved these changes Sep 4, 2019

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2019

Centril mentioned this pull request Sep 5, 2019

Rollup of 15 pull requests #64159

Closed

Centril mentioned this pull request Sep 5, 2019

Rollup of 15 pull requests #64160

Merged

bors merged commit 96ac02b into rust-lang:master Sep 5, 2019

ecstatic-morse deleted the is-indirect branch October 6, 2020 01:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a `Place::is_indirect` method to determine whether a `Place` contains a `Deref` projection#64005