Refactor hir::Place

For the following code
```rust
let c = || bar(foo.x, foo.x)
```

We generate two different `hir::Place`s for both `foo.x`.
Handling this adds overhead for analysis we need to do for RFC 2229.

We also want to store type information at each Projection to support
analysis as part of the RFC. This resembles what we have for
`mir::Place`

This commit modifies the Place as follows:
- Rename to `PlaceWithHirId`, where there `hir_id` is that of the
expressioin.
- Move any other information that describes the access out to another
struct now called `Place`.
- Removed `Span`, it can be accessed using the [hir
API](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/hir/map/struct.Map.html#method.span)
- Modify `Projection` to be a strucutre of its own, that currently only
contains the `ProjectionKind`.

Adding type information to projections wil be completed as part of https://github.com/rust-lang/project-rfc-2229/issues/5

Closes https://github.com/rust-lang/project-rfc-2229/issues/3

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
This commit is contained in:
Aman Arora 2020-06-17 18:13:05 -04:00 committed by Aman Arora
parent 93696f45ff
commit 922ff8e485
4 changed files with 38 additions and 36 deletions

View file

@ -6,7 +6,7 @@ use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span; use rustc_span::source_map::Span;
use rustc_target::abi::LayoutOf; use rustc_target::abi::LayoutOf;
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Place, PlaceBase}; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId, PlaceBase};
use crate::utils::span_lint; use crate::utils::span_lint;
@ -112,9 +112,9 @@ fn is_argument(map: rustc_middle::hir::map::Map<'_>, id: HirId) -> bool {
} }
impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
fn consume(&mut self, cmt: &Place<'tcx>, mode: ConsumeMode) { fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, mode: ConsumeMode) {
if cmt.projections.is_empty() { if cmt.place.projections.is_empty() {
if let PlaceBase::Local(lid) = cmt.base { if let PlaceBase::Local(lid) = cmt.place.base {
if let ConsumeMode::Move = mode { if let ConsumeMode::Move = mode {
// moved out or in. clearly can't be localized // moved out or in. clearly can't be localized
self.set.remove(&lid); self.set.remove(&lid);
@ -132,16 +132,16 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
} }
} }
fn borrow(&mut self, cmt: &Place<'tcx>, _: ty::BorrowKind) { fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: ty::BorrowKind) {
if cmt.projections.is_empty() { if cmt.place.projections.is_empty() {
if let PlaceBase::Local(lid) = cmt.base { if let PlaceBase::Local(lid) = cmt.place.base {
self.set.remove(&lid); self.set.remove(&lid);
} }
} }
} }
fn mutate(&mut self, cmt: &Place<'tcx>) { fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
if cmt.projections.is_empty() { if cmt.place.projections.is_empty() {
let map = &self.cx.tcx.hir(); let map = &self.cx.tcx.hir();
if is_argument(*map, cmt.hir_id) { if is_argument(*map, cmt.hir_id) {
// Skip closure arguments // Skip closure arguments
@ -150,7 +150,7 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
return; return;
} }
if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { if is_non_trait_box(cmt.place.ty) && !self.is_large_box(cmt.place.ty) {
self.set.insert(cmt.hir_id); self.set.insert(cmt.hir_id);
} }
return; return;

View file

@ -28,7 +28,7 @@ use rustc_middle::ty::{self, Ty, TyS};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span; use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol; use rustc_span::symbol::Symbol;
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Place, PlaceBase}; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId, PlaceBase};
use std::iter::{once, Iterator}; use std::iter::{once, Iterator};
use std::mem; use std::mem;
@ -1489,42 +1489,43 @@ fn check_for_loop_over_map_kv<'a, 'tcx>(
} }
} }
struct MutatePairDelegate { struct MutatePairDelegate<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
hir_id_low: Option<HirId>, hir_id_low: Option<HirId>,
hir_id_high: Option<HirId>, hir_id_high: Option<HirId>,
span_low: Option<Span>, span_low: Option<Span>,
span_high: Option<Span>, span_high: Option<Span>,
} }
impl<'tcx> Delegate<'tcx> for MutatePairDelegate { impl<'a, 'tcx> Delegate<'tcx> for MutatePairDelegate<'a, 'tcx> {
fn consume(&mut self, _: &Place<'tcx>, _: ConsumeMode) {} fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {}
fn borrow(&mut self, cmt: &Place<'tcx>, bk: ty::BorrowKind) { fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk { if let ty::BorrowKind::MutBorrow = bk {
if let PlaceBase::Local(id) = cmt.base { if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low { if Some(id) == self.hir_id_low {
self.span_low = Some(cmt.span) self.span_low = Some(self.cx.tcx.hir().span(cmt.hir_id))
} }
if Some(id) == self.hir_id_high { if Some(id) == self.hir_id_high {
self.span_high = Some(cmt.span) self.span_high = Some(self.cx.tcx.hir().span(cmt.hir_id))
} }
} }
} }
} }
fn mutate(&mut self, cmt: &Place<'tcx>) { fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
if let PlaceBase::Local(id) = cmt.base { if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low { if Some(id) == self.hir_id_low {
self.span_low = Some(cmt.span) self.span_low = Some(self.cx.tcx.hir().span(cmt.hir_id))
} }
if Some(id) == self.hir_id_high { if Some(id) == self.hir_id_high {
self.span_high = Some(cmt.span) self.span_high = Some(self.cx.tcx.hir().span(cmt.hir_id))
} }
} }
} }
} }
impl<'tcx> MutatePairDelegate { impl<'a, 'tcx> MutatePairDelegate<'a, 'tcx> {
fn mutation_span(&self) -> (Option<Span>, Option<Span>) { fn mutation_span(&self) -> (Option<Span>, Option<Span>) {
(self.span_low, self.span_high) (self.span_low, self.span_high)
} }
@ -1579,12 +1580,13 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr<'_>) -> Option<Hi
None None
} }
fn check_for_mutation( fn check_for_mutation<'a, 'tcx> (
cx: &LateContext<'_, '_>, cx: &LateContext<'a, 'tcx>,
body: &Expr<'_>, body: &Expr<'_>,
bound_ids: &[Option<HirId>], bound_ids: &[Option<HirId>],
) -> (Option<Span>, Option<Span>) { ) -> (Option<Span>, Option<Span>) {
let mut delegate = MutatePairDelegate { let mut delegate = MutatePairDelegate {
cx: cx,
hir_id_low: bound_ids[0], hir_id_low: bound_ids[0],
hir_id_high: bound_ids[1], hir_id_high: bound_ids[1],
span_low: None, span_low: None,

View file

@ -326,21 +326,21 @@ struct MovedVariablesCtxt {
} }
impl MovedVariablesCtxt { impl MovedVariablesCtxt {
fn move_common(&mut self, cmt: &euv::Place<'_>) { fn move_common(&mut self, cmt: &euv::PlaceWithHirId<'_>) {
if let euv::PlaceBase::Local(vid) = cmt.base { if let euv::PlaceBase::Local(vid) = cmt.place.base {
self.moved_vars.insert(vid); self.moved_vars.insert(vid);
} }
} }
} }
impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt {
fn consume(&mut self, cmt: &euv::Place<'tcx>, mode: euv::ConsumeMode) { fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, mode: euv::ConsumeMode) {
if let euv::ConsumeMode::Move = mode { if let euv::ConsumeMode::Move = mode {
self.move_common(cmt); self.move_common(cmt);
} }
} }
fn borrow(&mut self, _: &euv::Place<'tcx>, _: ty::BorrowKind) {} fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: ty::BorrowKind) {}
fn mutate(&mut self, _: &euv::Place<'tcx>) {} fn mutate(&mut self, _: &euv::PlaceWithHirId<'tcx>) {}
} }

View file

@ -8,7 +8,7 @@ use rustc_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_middle::ty; use rustc_middle::ty;
use rustc_span::symbol::{Ident, Symbol}; use rustc_span::symbol::{Ident, Symbol};
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Place, PlaceBase}; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId, PlaceBase};
/// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &'a LateContext<'a, 'tcx>) -> Option<FxHashSet<HirId>> { pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &'a LateContext<'a, 'tcx>) -> Option<FxHashSet<HirId>> {
@ -46,8 +46,8 @@ struct MutVarsDelegate {
impl<'tcx> MutVarsDelegate { impl<'tcx> MutVarsDelegate {
#[allow(clippy::similar_names)] #[allow(clippy::similar_names)]
fn update(&mut self, cat: &Place<'tcx>) { fn update(&mut self, cat: &PlaceWithHirId<'tcx>) {
match cat.base { match cat.place.base {
PlaceBase::Local(id) => { PlaceBase::Local(id) => {
self.used_mutably.insert(id); self.used_mutably.insert(id);
}, },
@ -63,15 +63,15 @@ impl<'tcx> MutVarsDelegate {
} }
impl<'tcx> Delegate<'tcx> for MutVarsDelegate { impl<'tcx> Delegate<'tcx> for MutVarsDelegate {
fn consume(&mut self, _: &Place<'tcx>, _: ConsumeMode) {} fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {}
fn borrow(&mut self, cmt: &Place<'tcx>, bk: ty::BorrowKind) { fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk { if let ty::BorrowKind::MutBorrow = bk {
self.update(&cmt) self.update(&cmt)
} }
} }
fn mutate(&mut self, cmt: &Place<'tcx>) { fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
self.update(&cmt) self.update(&cmt)
} }
} }