Skip to content

Commit

Permalink
Rollup merge of rust-lang#61554 - spastorino:change_visit_api, r=oli-obk
Browse files Browse the repository at this point in the history
Change visit api

r? @oli-obk

In the [first commit](rust-lang@37386d3) of this PR, I'm changing `visit_place` to be the function that traverses the `Place` and have only that responsibility. Then there are two other functions `visit_place_base` and `visit_projection` which are the ones in charge of visiting the base and the projection. Visitor implementors can implement any of those.

In the [second commit](rust-lang@e786f63) we can already see some things that confuses me, which I think this division will make more clear. The old code, first checked if the place was a base, did something with it and then called `super_place` [here](rust-lang@e786f63#diff-d583e4efe1a72516e274158e53223633L678). `super_place` checks again if it's a base [here](https://github.com/rust-lang/rust/blob/master/src/librustc/mir/visit.rs#L679-L684) and in case is a local, visits the local and stuff like that. That's not very obvious on the code, and if I'm not wrong it's not needed. In this PR or we have [this](rust-lang@e786f63#diff-d583e4efe1a72516e274158e53223633R673) as I did or we can just do `- => self.super_place_base(...)` and that will be obvious that I'm letting the default implementation process the base.
  • Loading branch information
Centril authored Jun 6, 2019
2 parents 654854f + 67197e2 commit 4c74056
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 39 deletions.
58 changes: 31 additions & 27 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,17 @@ macro_rules! make_mir_visitor {
self.super_place(place, context, location);
}

fn visit_projection(&mut self,
place: & $($mutability)? Projection<'tcx>,
fn visit_place_base(&mut self,
place_base: & $($mutability)? PlaceBase<'tcx>,
context: PlaceContext,
location: Location) {
self.super_projection(place, context, location);
self.super_place_base(place_base, context, location);
}

fn visit_projection_elem(&mut self,
place: & $($mutability)? PlaceElem<'tcx>,
location: Location) {
self.super_projection_elem(place, location);
fn visit_projection(&mut self,
place: & $($mutability)? Projection<'tcx>,
location: Location) {
self.super_projection(place, location);
}

fn visit_constant(&mut self,
Expand Down Expand Up @@ -676,36 +676,40 @@ macro_rules! make_mir_visitor {
context: PlaceContext,
location: Location) {
match place {
Place::Base(PlaceBase::Local(local)) => {
self.visit_local(local, context, location);
}
Place::Base(PlaceBase::Static(box Static { kind: _, ty })) => {
self.visit_ty(& $($mutability)? *ty, TyContext::Location(location));
Place::Base(place_base) => {
self.visit_place_base(place_base, context, location);
}
Place::Projection(proj) => {
self.visit_projection(proj, context, location);
let context = if context.is_mutating_use() {
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
};

self.visit_place(& $($mutability)? proj.base, context, location);
self.visit_projection(proj, location);
}
}
}

fn super_projection(&mut self,
proj: & $($mutability)? Projection<'tcx>,
fn super_place_base(&mut self,
place_base: & $($mutability)? PlaceBase<'tcx>,
context: PlaceContext,
location: Location) {
let Projection { base, elem } = proj;
let context = if context.is_mutating_use() {
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
};
self.visit_place(base, context, location);
self.visit_projection_elem(elem, location);
match place_base {
PlaceBase::Local(local) => {
self.visit_local(local, context, location);
}
PlaceBase::Static(box Static { kind: _, ty }) => {
self.visit_ty(& $($mutability)? *ty, TyContext::Location(location));
}
}
}

fn super_projection_elem(&mut self,
proj: & $($mutability)? PlaceElem<'tcx>,
location: Location) {
match proj {
fn super_projection(&mut self,
proj: & $($mutability)? Projection<'tcx>,
location: Location) {
match & $($mutability)? proj.elem {
ProjectionElem::Deref => {
}
ProjectionElem::Subslice { from: _, to: _ } => {
Expand Down
25 changes: 13 additions & 12 deletions src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ use rustc::ty::{self, TypeFoldable, Ty, TyCtxt, GenericParamDefKind, Instance};
use rustc::ty::print::obsolete::DefPathBasedNames;
use rustc::ty::adjustment::{CustomCoerceUnsized, PointerCast};
use rustc::session::config::EntryFnType;
use rustc::mir::{self, Location, Place, PlaceBase, Promoted, Static, StaticKind};
use rustc::mir::{self, Location, PlaceBase, Promoted, Static, StaticKind};
use rustc::mir::visit::Visitor as MirVisitor;
use rustc::mir::mono::{MonoItem, InstantiationMode};
use rustc::mir::interpret::{Scalar, GlobalId, GlobalAlloc, ErrorHandled};
Expand Down Expand Up @@ -655,14 +655,12 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
self.super_terminator_kind(kind, location);
}

fn visit_place(&mut self,
place: &mir::Place<'tcx>,
context: mir::visit::PlaceContext,
location: Location) {
match place {
Place::Base(
PlaceBase::Static(box Static{ kind:StaticKind::Static(def_id), .. })
) => {
fn visit_place_base(&mut self,
place_base: &mir::PlaceBase<'tcx>,
_context: mir::visit::PlaceContext,
location: Location) {
match place_base {
PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. }) => {
debug!("visiting static {:?} @ {:?}", def_id, location);

let tcx = self.tcx;
Expand All @@ -671,10 +669,13 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
self.output.push(MonoItem::Static(*def_id));
}
}
_ => {}
PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. }) => {
// FIXME: should we handle promoteds here instead of eagerly in collect_neighbours?
}
PlaceBase::Local(_) => {
// Locals have no relevance for collector
}
}

self.super_place(place, context, location);
}
}

Expand Down

0 comments on commit 4c74056

Please sign in to comment.