Auto merge of #77274 - tmiasko:liveness-cnd, r=lcnr

Liveness refactoring continued

* Move body_owner field from IrMaps to Liveness (the only user of the field).
* Use upvars instead of FnKind to check for closures (avoids FnKind, will be useful when checking all bodies, not just fns).
* Use visit_param to add variables corresponding to params.
* Store upvars_mentioned inside Liveness struct.
* Inline visitor implementation for IrMaps, avoiding unnecessary indirection.
* Test interaction with automatically_derived attribute (not covered by any of existing tests).

No functional changes intended.
This commit is contained in:
bors 2020-09-29 19:25:10 +00:00
commit 381b445ff5
3 changed files with 245 additions and 219 deletions

View File

@ -138,40 +138,8 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String {
}
}
impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.tcx.hir())
}
fn visit_fn(
&mut self,
fk: FnKind<'tcx>,
fd: &'tcx hir::FnDecl<'tcx>,
b: hir::BodyId,
s: Span,
id: HirId,
) {
visit_fn(self, fk, fd, b, s, id);
}
fn visit_local(&mut self, l: &'tcx hir::Local<'tcx>) {
visit_local(self, l);
}
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
visit_expr(self, ex);
}
fn visit_arm(&mut self, a: &'tcx hir::Arm<'tcx>) {
visit_arm(self, a);
}
}
fn check_mod_liveness(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
tcx.hir().visit_item_likes_in_module(
module_def_id,
&mut IrMaps::new(tcx, module_def_id).as_deep_visitor(),
);
tcx.hir().visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx).as_deep_visitor());
}
pub fn provide(providers: &mut Providers) {
@ -227,7 +195,6 @@ enum VarKind {
struct IrMaps<'tcx> {
tcx: TyCtxt<'tcx>,
body_owner: LocalDefId,
live_node_map: HirIdMap<LiveNode>,
variable_map: HirIdMap<Variable>,
capture_info_map: HirIdMap<Rc<Vec<CaptureInfo>>>,
@ -236,10 +203,9 @@ struct IrMaps<'tcx> {
}
impl IrMaps<'tcx> {
fn new(tcx: TyCtxt<'tcx>, body_owner: LocalDefId) -> IrMaps<'tcx> {
fn new(tcx: TyCtxt<'tcx>) -> IrMaps<'tcx> {
IrMaps {
tcx,
body_owner,
live_node_map: HirIdMap::default(),
variable_map: HirIdMap::default(),
capture_info_map: Default::default(),
@ -302,44 +268,117 @@ impl IrMaps<'tcx> {
fn set_captures(&mut self, hir_id: HirId, cs: Vec<CaptureInfo>) {
self.capture_info_map.insert(hir_id, Rc::new(cs));
}
}
fn visit_fn<'tcx>(
ir: &mut IrMaps<'tcx>,
fk: FnKind<'tcx>,
decl: &'tcx hir::FnDecl<'tcx>,
body_id: hir::BodyId,
sp: Span,
id: hir::HirId,
) {
debug!("visit_fn {:?}", id);
// swap in a new set of IR maps for this function body:
let def_id = ir.tcx.hir().local_def_id(id);
let mut fn_maps = IrMaps::new(ir.tcx, def_id);
// Don't run unused pass for #[derive()]
if let FnKind::Method(..) = fk {
let parent = ir.tcx.hir().get_parent_item(id);
if let Some(Node::Item(i)) = ir.tcx.hir().find(parent) {
if i.attrs.iter().any(|a| ir.tcx.sess.check_name(a, sym::automatically_derived)) {
return;
fn add_from_pat(&mut self, pat: &hir::Pat<'tcx>) {
// For struct patterns, take note of which fields used shorthand
// (`x` rather than `x: x`).
let mut shorthand_field_ids = HirIdSet::default();
let mut pats = VecDeque::new();
pats.push_back(pat);
while let Some(pat) = pats.pop_front() {
use rustc_hir::PatKind::*;
match &pat.kind {
Binding(.., inner_pat) => {
pats.extend(inner_pat.iter());
}
Struct(_, fields, _) => {
let ids = fields.iter().filter(|f| f.is_shorthand).map(|f| f.pat.hir_id);
shorthand_field_ids.extend(ids);
}
Ref(inner_pat, _) | Box(inner_pat) => {
pats.push_back(inner_pat);
}
TupleStruct(_, inner_pats, _) | Tuple(inner_pats, _) | Or(inner_pats) => {
pats.extend(inner_pats.iter());
}
Slice(pre_pats, inner_pat, post_pats) => {
pats.extend(pre_pats.iter());
pats.extend(inner_pat.iter());
pats.extend(post_pats.iter());
}
_ => {}
}
}
pat.each_binding(|_, hir_id, _, ident| {
self.add_live_node_for_node(hir_id, VarDefNode(ident.span));
self.add_variable(Local(LocalInfo {
id: hir_id,
name: ident.name,
is_shorthand: shorthand_field_ids.contains(&hir_id),
}));
});
}
}
impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.tcx.hir())
}
debug!("creating fn_maps: {:p}", &fn_maps);
fn visit_fn(
&mut self,
fk: FnKind<'tcx>,
decl: &'tcx hir::FnDecl<'tcx>,
body_id: hir::BodyId,
sp: Span,
id: HirId,
) {
debug!("visit_fn {:?}", id);
let body = ir.tcx.hir().body(body_id);
// swap in a new set of IR maps for this function body:
let def_id = self.tcx.hir().local_def_id(id);
let mut fn_maps = IrMaps::new(self.tcx);
if let Some(upvars) = ir.tcx.upvars_mentioned(def_id) {
for (&var_hir_id, _upvar) in upvars {
let var_name = ir.tcx.hir().name(var_hir_id);
fn_maps.add_variable(Upvar(var_hir_id, var_name));
// Don't run unused pass for #[derive()]
if let FnKind::Method(..) = fk {
let parent = self.tcx.hir().get_parent_item(id);
if let Some(Node::Item(i)) = self.tcx.hir().find(parent) {
if i.attrs.iter().any(|a| self.tcx.sess.check_name(a, sym::automatically_derived)) {
return;
}
}
}
debug!("creating fn_maps: {:p}", &fn_maps);
let body = self.tcx.hir().body(body_id);
if let Some(upvars) = self.tcx.upvars_mentioned(def_id) {
for (&var_hir_id, _upvar) in upvars {
let var_name = self.tcx.hir().name(var_hir_id);
fn_maps.add_variable(Upvar(var_hir_id, var_name));
}
}
// gather up the various local variables, significant expressions,
// and so forth:
intravisit::walk_fn(&mut fn_maps, fk, decl, body_id, sp, id);
// compute liveness
let mut lsets = Liveness::new(&mut fn_maps, def_id);
let entry_ln = lsets.compute(&body, sp, id);
lsets.log_liveness(entry_ln, id);
// check for various error conditions
lsets.visit_body(body);
lsets.warn_about_unused_upvars(entry_ln);
lsets.warn_about_unused_args(body, entry_ln);
}
for param in body.params {
fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) {
self.add_from_pat(&local.pat);
intravisit::walk_local(self, local);
}
fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) {
self.add_from_pat(&arm.pat);
intravisit::walk_arm(self, arm);
}
fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
let is_shorthand = match param.pat.kind {
rustc_hir::PatKind::Struct(..) => true,
_ => false,
@ -350,150 +389,83 @@ fn visit_fn<'tcx>(
} else {
Param(hir_id, ident.name)
};
fn_maps.add_variable(var);
})
self.add_variable(var);
});
intravisit::walk_param(self, param);
}
// gather up the various local variables, significant expressions,
// and so forth:
intravisit::walk_fn(&mut fn_maps, fk, decl, body_id, sp, id);
// compute liveness
let mut lsets = Liveness::new(&mut fn_maps, def_id);
let entry_ln = lsets.compute(fk, &body, sp, id);
lsets.log_liveness(entry_ln, id);
// check for various error conditions
lsets.visit_body(body);
lsets.warn_about_unused_upvars(entry_ln);
lsets.warn_about_unused_args(body, entry_ln);
}
fn add_from_pat(ir: &mut IrMaps<'_>, pat: &hir::Pat<'_>) {
// For struct patterns, take note of which fields used shorthand
// (`x` rather than `x: x`).
let mut shorthand_field_ids = HirIdSet::default();
let mut pats = VecDeque::new();
pats.push_back(pat);
while let Some(pat) = pats.pop_front() {
use rustc_hir::PatKind::*;
match &pat.kind {
Binding(.., inner_pat) => {
pats.extend(inner_pat.iter());
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
match expr.kind {
// live nodes required for uses or definitions of variables:
hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) => {
debug!("expr {}: path that leads to {:?}", expr.hir_id, path.res);
if let Res::Local(_var_hir_id) = path.res {
self.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
}
intravisit::walk_expr(self, expr);
}
Struct(_, fields, _) => {
let ids = fields.iter().filter(|f| f.is_shorthand).map(|f| f.pat.hir_id);
shorthand_field_ids.extend(ids);
hir::ExprKind::Closure(..) => {
// Interesting control flow (for loops can contain labeled
// breaks or continues)
self.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
// Make a live_node for each captured variable, with the span
// being the location that the variable is used. This results
// in better error messages than just pointing at the closure
// construction site.
let mut call_caps = Vec::new();
let closure_def_id = self.tcx.hir().local_def_id(expr.hir_id);
if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
call_caps.extend(upvars.iter().map(|(&var_id, upvar)| {
let upvar_ln = self.add_live_node(UpvarNode(upvar.span));
CaptureInfo { ln: upvar_ln, var_hid: var_id }
}));
}
self.set_captures(expr.hir_id, call_caps);
intravisit::walk_expr(self, expr);
}
Ref(inner_pat, _) | Box(inner_pat) => {
pats.push_back(inner_pat);
// live nodes required for interesting control flow:
hir::ExprKind::Match(..) | hir::ExprKind::Loop(..) => {
self.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
intravisit::walk_expr(self, expr);
}
TupleStruct(_, inner_pats, _) | Tuple(inner_pats, _) | Or(inner_pats) => {
pats.extend(inner_pats.iter());
hir::ExprKind::Binary(op, ..) if op.node.is_lazy() => {
self.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
intravisit::walk_expr(self, expr);
}
Slice(pre_pats, inner_pat, post_pats) => {
pats.extend(pre_pats.iter());
pats.extend(inner_pat.iter());
pats.extend(post_pats.iter());
// otherwise, live nodes are not required:
hir::ExprKind::Index(..)
| hir::ExprKind::Field(..)
| hir::ExprKind::Array(..)
| hir::ExprKind::Call(..)
| hir::ExprKind::MethodCall(..)
| hir::ExprKind::Tup(..)
| hir::ExprKind::Binary(..)
| hir::ExprKind::AddrOf(..)
| hir::ExprKind::Cast(..)
| hir::ExprKind::DropTemps(..)
| hir::ExprKind::Unary(..)
| hir::ExprKind::Break(..)
| hir::ExprKind::Continue(_)
| hir::ExprKind::Lit(_)
| hir::ExprKind::Ret(..)
| hir::ExprKind::Block(..)
| hir::ExprKind::Assign(..)
| hir::ExprKind::AssignOp(..)
| hir::ExprKind::Struct(..)
| hir::ExprKind::Repeat(..)
| hir::ExprKind::InlineAsm(..)
| hir::ExprKind::LlvmInlineAsm(..)
| hir::ExprKind::Box(..)
| hir::ExprKind::Yield(..)
| hir::ExprKind::Type(..)
| hir::ExprKind::Err
| hir::ExprKind::Path(hir::QPath::TypeRelative(..))
| hir::ExprKind::Path(hir::QPath::LangItem(..)) => {
intravisit::walk_expr(self, expr);
}
_ => {}
}
}
pat.each_binding(|_, hir_id, _, ident| {
ir.add_live_node_for_node(hir_id, VarDefNode(ident.span));
ir.add_variable(Local(LocalInfo {
id: hir_id,
name: ident.name,
is_shorthand: shorthand_field_ids.contains(&hir_id),
}));
});
}
fn visit_local<'tcx>(ir: &mut IrMaps<'tcx>, local: &'tcx hir::Local<'tcx>) {
add_from_pat(ir, &local.pat);
intravisit::walk_local(ir, local);
}
fn visit_arm<'tcx>(ir: &mut IrMaps<'tcx>, arm: &'tcx hir::Arm<'tcx>) {
add_from_pat(ir, &arm.pat);
intravisit::walk_arm(ir, arm);
}
fn visit_expr<'tcx>(ir: &mut IrMaps<'tcx>, expr: &'tcx Expr<'tcx>) {
match expr.kind {
// live nodes required for uses or definitions of variables:
hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) => {
debug!("expr {}: path that leads to {:?}", expr.hir_id, path.res);
if let Res::Local(_var_hir_id) = path.res {
ir.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
}
intravisit::walk_expr(ir, expr);
}
hir::ExprKind::Closure(..) => {
// Interesting control flow (for loops can contain labeled
// breaks or continues)
ir.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
// Make a live_node for each captured variable, with the span
// being the location that the variable is used. This results
// in better error messages than just pointing at the closure
// construction site.
let mut call_caps = Vec::new();
let closure_def_id = ir.tcx.hir().local_def_id(expr.hir_id);
if let Some(upvars) = ir.tcx.upvars_mentioned(closure_def_id) {
call_caps.extend(upvars.iter().map(|(&var_id, upvar)| {
let upvar_ln = ir.add_live_node(UpvarNode(upvar.span));
CaptureInfo { ln: upvar_ln, var_hid: var_id }
}));
}
ir.set_captures(expr.hir_id, call_caps);
let old_body_owner = ir.body_owner;
ir.body_owner = closure_def_id;
intravisit::walk_expr(ir, expr);
ir.body_owner = old_body_owner;
}
// live nodes required for interesting control flow:
hir::ExprKind::Match(..) | hir::ExprKind::Loop(..) => {
ir.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
intravisit::walk_expr(ir, expr);
}
hir::ExprKind::Binary(op, ..) if op.node.is_lazy() => {
ir.add_live_node_for_node(expr.hir_id, ExprNode(expr.span));
intravisit::walk_expr(ir, expr);
}
// otherwise, live nodes are not required:
hir::ExprKind::Index(..)
| hir::ExprKind::Field(..)
| hir::ExprKind::Array(..)
| hir::ExprKind::Call(..)
| hir::ExprKind::MethodCall(..)
| hir::ExprKind::Tup(..)
| hir::ExprKind::Binary(..)
| hir::ExprKind::AddrOf(..)
| hir::ExprKind::Cast(..)
| hir::ExprKind::DropTemps(..)
| hir::ExprKind::Unary(..)
| hir::ExprKind::Break(..)
| hir::ExprKind::Continue(_)
| hir::ExprKind::Lit(_)
| hir::ExprKind::Ret(..)
| hir::ExprKind::Block(..)
| hir::ExprKind::Assign(..)
| hir::ExprKind::AssignOp(..)
| hir::ExprKind::Struct(..)
| hir::ExprKind::Repeat(..)
| hir::ExprKind::InlineAsm(..)
| hir::ExprKind::LlvmInlineAsm(..)
| hir::ExprKind::Box(..)
| hir::ExprKind::Yield(..)
| hir::ExprKind::Type(..)
| hir::ExprKind::Err
| hir::ExprKind::Path(hir::QPath::TypeRelative(..))
| hir::ExprKind::Path(hir::QPath::LangItem(..)) => {
intravisit::walk_expr(ir, expr);
}
}
}
@ -605,8 +577,10 @@ const ACC_USE: u32 = 4;
struct Liveness<'a, 'tcx> {
ir: &'a mut IrMaps<'tcx>,
body_owner: LocalDefId,
typeck_results: &'a ty::TypeckResults<'tcx>,
param_env: ty::ParamEnv<'tcx>,
upvars: Option<&'tcx FxIndexMap<hir::HirId, hir::Upvar>>,
successors: IndexVec<LiveNode, LiveNode>,
rwu_table: RWUTable,
@ -626,9 +600,10 @@ struct Liveness<'a, 'tcx> {
}
impl<'a, 'tcx> Liveness<'a, 'tcx> {
fn new(ir: &'a mut IrMaps<'tcx>, def_id: LocalDefId) -> Liveness<'a, 'tcx> {
let typeck_results = ir.tcx.typeck(def_id);
let param_env = ir.tcx.param_env(def_id);
fn new(ir: &'a mut IrMaps<'tcx>, body_owner: LocalDefId) -> Liveness<'a, 'tcx> {
let typeck_results = ir.tcx.typeck(body_owner);
let param_env = ir.tcx.param_env(body_owner);
let upvars = ir.tcx.upvars_mentioned(body_owner);
let closure_ln = ir.add_live_node(ClosureNode);
let exit_ln = ir.add_live_node(ExitNode);
@ -638,8 +613,10 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
Liveness {
ir,
body_owner,
typeck_results,
param_env,
upvars,
successors: IndexVec::from_elem_n(INVALID_NODE, num_live_nodes),
rwu_table: RWUTable::new(num_live_nodes * num_vars),
closure_ln,
@ -868,13 +845,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
self.rwu_table.assign_unpacked(idx, rwu);
}
fn compute(
&mut self,
fk: FnKind<'_>,
body: &hir::Body<'_>,
span: Span,
id: hir::HirId,
) -> LiveNode {
fn compute(&mut self, body: &hir::Body<'_>, span: Span, id: hir::HirId) -> LiveNode {
debug!("compute: using id for body, {:?}", body.value);
// # Liveness of captured variables
@ -893,12 +864,12 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
// if they are live on the entry to the closure, since only the closure
// itself can access them on subsequent calls.
if let Some(upvars) = self.ir.tcx.upvars_mentioned(self.ir.body_owner) {
if let Some(upvars) = self.upvars {
// Mark upvars captured by reference as used after closure exits.
for (&var_hir_id, upvar) in upvars.iter().rev() {
let upvar_id = ty::UpvarId {
var_path: ty::UpvarPath { hir_id: var_hir_id },
closure_expr_id: self.ir.body_owner,
closure_expr_id: self.body_owner,
};
match self.typeck_results.upvar_capture(upvar_id) {
ty::UpvarCapture::ByRef(_) => {
@ -912,9 +883,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
let succ = self.propagate_through_expr(&body.value, self.exit_ln);
match fk {
FnKind::Method(..) | FnKind::ItemFn(..) => return succ,
FnKind::Closure(..) => {}
if self.upvars.is_none() {
// Either not a closure, or closure without any captured variables.
// No need to determine liveness of captured variables, since there
// are none.
return succ;
}
let ty = self.typeck_results.node_type(id);
@ -926,7 +899,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
},
ty::Generator(..) => return succ,
_ => {
span_bug!(span, "type of closure expr {:?} is not a closure {:?}", id, ty,);
span_bug!(span, "{} has upvars so it should have a closure type: {:?}", id, ty);
}
};
@ -1565,7 +1538,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
}
fn warn_about_unused_upvars(&self, entry_ln: LiveNode) {
let upvars = match self.ir.tcx.upvars_mentioned(self.ir.body_owner) {
let upvars = match self.upvars {
None => return,
Some(upvars) => upvars,
};
@ -1573,7 +1546,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
let var = self.variable(var_hir_id, upvar.span);
let upvar_id = ty::UpvarId {
var_path: ty::UpvarPath { hir_id: var_hir_id },
closure_expr_id: self.ir.body_owner,
closure_expr_id: self.body_owner,
};
match self.typeck_results.upvar_capture(upvar_id) {
ty::UpvarCapture::ByValue(_) => {}

View File

@ -0,0 +1,38 @@
// Test for interaction between #[automatically_derived] attribute used by
// built-in derives and lints generated by liveness pass.
//
// edition:2018
// check-pass
#![warn(unused)]
pub trait T: Sized {
const N: usize;
fn t(&self) -> Self;
}
impl T for u32 {
const N: usize = {
let a = 0; // FIXME should warn about unused variable
4
};
fn t(&self) -> Self {
let b = 16; //~ WARN unused variable: `b`
0
}
}
#[automatically_derived]
impl T for i32 {
const N: usize = {
let c = 0;
4
};
fn t(&self) -> Self {
let d = 17;
0
}
}
fn main() {}

View File

@ -0,0 +1,15 @@
warning: unused variable: `b`
--> $DIR/liveness-derive.rs:20:13
|
LL | let b = 16;
| ^ help: if this is intentional, prefix it with an underscore: `_b`
|
note: the lint level is defined here
--> $DIR/liveness-derive.rs:6:9
|
LL | #![warn(unused)]
| ^^^^^^
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`
warning: 1 warning emitted