Skip to content

Commit

Permalink
Refactor hir::Place
Browse files Browse the repository at this point in the history
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 rust-lang/project-rfc-2229#5

Closes rust-lang/project-rfc-2229#3

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
  • Loading branch information
arora-aman and roxelo committed Jun 18, 2020
1 parent 93696f4 commit 922ff8e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 36 deletions.
20 changes: 10 additions & 10 deletions clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
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;

Expand Down Expand Up @@ -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> {
fn consume(&mut self, cmt: &Place<'tcx>, mode: ConsumeMode) {
if cmt.projections.is_empty() {
if let PlaceBase::Local(lid) = cmt.base {
fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, mode: ConsumeMode) {
if cmt.place.projections.is_empty() {
if let PlaceBase::Local(lid) = cmt.place.base {
if let ConsumeMode::Move = mode {
// moved out or in. clearly can't be localized
self.set.remove(&lid);
Expand All @@ -132,16 +132,16 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
}
}

fn borrow(&mut self, cmt: &Place<'tcx>, _: ty::BorrowKind) {
if cmt.projections.is_empty() {
if let PlaceBase::Local(lid) = cmt.base {
fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: ty::BorrowKind) {
if cmt.place.projections.is_empty() {
if let PlaceBase::Local(lid) = cmt.place.base {
self.set.remove(&lid);
}
}
}

fn mutate(&mut self, cmt: &Place<'tcx>) {
if cmt.projections.is_empty() {
fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
if cmt.place.projections.is_empty() {
let map = &self.cx.tcx.hir();
if is_argument(*map, cmt.hir_id) {
// Skip closure arguments
Expand All @@ -150,7 +150,7 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
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);
}
return;
Expand Down
32 changes: 17 additions & 15 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use rustc_middle::ty::{self, Ty, TyS};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
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::mem;

Expand Down Expand Up @@ -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_high: Option<HirId>,
span_low: Option<Span>,
span_high: Option<Span>,
}

impl<'tcx> Delegate<'tcx> for MutatePairDelegate {
fn consume(&mut self, _: &Place<'tcx>, _: ConsumeMode) {}
impl<'a, 'tcx> Delegate<'tcx> for MutatePairDelegate<'a, 'tcx> {
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 PlaceBase::Local(id) = cmt.base {
if let PlaceBase::Local(id) = cmt.place.base {
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 {
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>) {
if let PlaceBase::Local(id) = cmt.base {
fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
if let PlaceBase::Local(id) = cmt.place.base {
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 {
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>) {
(self.span_low, self.span_high)
}
Expand Down Expand Up @@ -1579,12 +1580,13 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr<'_>) -> Option<Hi
None
}

fn check_for_mutation(
cx: &LateContext<'_, '_>,
fn check_for_mutation<'a, 'tcx> (
cx: &LateContext<'a, 'tcx>,
body: &Expr<'_>,
bound_ids: &[Option<HirId>],
) -> (Option<Span>, Option<Span>) {
let mut delegate = MutatePairDelegate {
cx: cx,
hir_id_low: bound_ids[0],
hir_id_high: bound_ids[1],
span_low: None,
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/needless_pass_by_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,21 +326,21 @@ struct MovedVariablesCtxt {
}

impl MovedVariablesCtxt {
fn move_common(&mut self, cmt: &euv::Place<'_>) {
if let euv::PlaceBase::Local(vid) = cmt.base {
fn move_common(&mut self, cmt: &euv::PlaceWithHirId<'_>) {
if let euv::PlaceBase::Local(vid) = cmt.place.base {
self.moved_vars.insert(vid);
}
}
}

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 {
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>) {}
}
12 changes: 6 additions & 6 deletions clippy_lints/src/utils/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_middle::ty;
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.
pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &'a LateContext<'a, 'tcx>) -> Option<FxHashSet<HirId>> {
Expand Down Expand Up @@ -46,8 +46,8 @@ struct MutVarsDelegate {

impl<'tcx> MutVarsDelegate {
#[allow(clippy::similar_names)]
fn update(&mut self, cat: &Place<'tcx>) {
match cat.base {
fn update(&mut self, cat: &PlaceWithHirId<'tcx>) {
match cat.place.base {
PlaceBase::Local(id) => {
self.used_mutably.insert(id);
},
Expand All @@ -63,15 +63,15 @@ impl<'tcx> 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 {
self.update(&cmt)
}
}

fn mutate(&mut self, cmt: &Place<'tcx>) {
fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
self.update(&cmt)
}
}
Expand Down

0 comments on commit 922ff8e

Please sign in to comment.