X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Feval_order_dependence.rs;h=7967d99d104aac2f98269e435f3406d082b13ef9;hb=6d1225981177587fbb68d9c4902c770c3dbaafb0;hp=8bd8461b119dd9010ad246769ff3f114d760ca9c;hpb=da7aebc342b7dae444912b98dedbab22bf2a224c;p=rust.git diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index 8bd8461b119..7967d99d104 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -1,77 +1,72 @@ use crate::utils::{get_parent_expr, span_lint, span_note_and_lint}; use if_chain::if_chain; +use rustc::declare_lint_pass; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::ty; -use rustc::{declare_tool_lint, lint_array}; -use syntax::ast; +use rustc_session::declare_tool_lint; -/// **What it does:** Checks for a read and a write to the same variable where -/// whether the read occurs before or after the write depends on the evaluation -/// order of sub-expressions. -/// -/// **Why is this bad?** It is often confusing to read. In addition, the -/// sub-expression evaluation order for Rust is not well documented. -/// -/// **Known problems:** Code which intentionally depends on the evaluation -/// order, or which is correct for any evaluation order. -/// -/// **Example:** -/// ```rust -/// let mut x = 0; -/// let a = { -/// x = 1; -/// 1 -/// } + x; -/// // Unclear whether a is 1 or 2. -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for a read and a write to the same variable where + /// whether the read occurs before or after the write depends on the evaluation + /// order of sub-expressions. + /// + /// **Why is this bad?** It is often confusing to read. In addition, the + /// sub-expression evaluation order for Rust is not well documented. + /// + /// **Known problems:** Code which intentionally depends on the evaluation + /// order, or which is correct for any evaluation order. + /// + /// **Example:** + /// ```rust + /// let mut x = 0; + /// let a = { + /// x = 1; + /// 1 + /// } + x; + /// // Unclear whether a is 1 or 2. + /// ``` pub EVAL_ORDER_DEPENDENCE, complexity, "whether a variable read occurs before a write depends on sub-expression evaluation order" } -/// **What it does:** Checks for diverging calls that are not match arms or -/// statements. -/// -/// **Why is this bad?** It is often confusing to read. In addition, the -/// sub-expression evaluation order for Rust is not well documented. -/// -/// **Known problems:** Someone might want to use `some_bool || panic!()` as a -/// shorthand. -/// -/// **Example:** -/// ```rust -/// let a = b() || panic!() || c(); -/// // `c()` is dead, `panic!()` is only called if `b()` returns `false` -/// let x = (a, b, c, panic!()); -/// // can simply be replaced by `panic!()` -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for diverging calls that are not match arms or + /// statements. + /// + /// **Why is this bad?** It is often confusing to read. In addition, the + /// sub-expression evaluation order for Rust is not well documented. + /// + /// **Known problems:** Someone might want to use `some_bool || panic!()` as a + /// shorthand. + /// + /// **Example:** + /// ```rust,no_run + /// # fn b() -> bool { true } + /// # fn c() -> bool { true } + /// let a = b() || panic!() || c(); + /// // `c()` is dead, `panic!()` is only called if `b()` returns `false` + /// let x = (a, b, c, panic!()); + /// // can simply be replaced by `panic!()` + /// ``` pub DIVERGING_SUB_EXPRESSION, complexity, "whether an expression contains a diverging sub expression" } -#[derive(Copy, Clone)] -pub struct EvalOrderDependence; - -impl LintPass for EvalOrderDependence { - fn get_lints(&self) -> LintArray { - lint_array!(EVAL_ORDER_DEPENDENCE, DIVERGING_SUB_EXPRESSION) - } -} +declare_lint_pass!(EvalOrderDependence => [EVAL_ORDER_DEPENDENCE, DIVERGING_SUB_EXPRESSION]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EvalOrderDependence { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { // Find a write to a local variable. - match expr.node { + match expr.kind { ExprKind::Assign(ref lhs, _) | ExprKind::AssignOp(_, ref lhs, _) => { - if let ExprKind::Path(ref qpath) = lhs.node { + if let ExprKind::Path(ref qpath) = lhs.kind { if let QPath::Resolved(_, ref path) = *qpath { if path.segments.len() == 1 { - if let def::Def::Local(var) = cx.tables.qpath_def(qpath, lhs.hir_id) { + if let def::Res::Local(var) = cx.tables.qpath_res(qpath, lhs.hir_id) { let mut visitor = ReadVisitor { cx, var, @@ -88,27 +83,26 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { } } fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) { - match stmt.node { - StmtKind::Expr(ref e, _) | StmtKind::Semi(ref e, _) => DivergenceVisitor { cx }.maybe_walk_expr(e), - StmtKind::Decl(ref d, _) => { - if let DeclKind::Local(ref local) = d.node { - if let Local { init: Some(ref e), .. } = **local { - DivergenceVisitor { cx }.visit_expr(e); - } + match stmt.kind { + StmtKind::Local(ref local) => { + if let Local { init: Some(ref e), .. } = **local { + DivergenceVisitor { cx }.visit_expr(e); } }, + StmtKind::Expr(ref e) | StmtKind::Semi(ref e) => DivergenceVisitor { cx }.maybe_walk_expr(e), + StmtKind::Item(..) => {}, } } } -struct DivergenceVisitor<'a, 'tcx: 'a> { +struct DivergenceVisitor<'a, 'tcx> { cx: &'a LateContext<'a, 'tcx>, } impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> { fn maybe_walk_expr(&mut self, e: &'tcx Expr) { - match e.node { - ExprKind::Closure(.., _) => {}, + match e.kind { + ExprKind::Closure(..) => {}, ExprKind::Match(ref e, ref arms, _) => { self.visit_expr(e); for arm in arms { @@ -131,14 +125,14 @@ fn report_diverging_sub_expr(&mut self, e: &Expr) { impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> { fn visit_expr(&mut self, e: &'tcx Expr) { - match e.node { + match e.kind { ExprKind::Continue(_) | ExprKind::Break(_, _) | ExprKind::Ret(_) => self.report_diverging_sub_expr(e), ExprKind::Call(ref func, _) => { let typ = self.cx.tables.expr_ty(func); - match typ.sty { + match typ.kind { ty::FnDef(..) | ty::FnPtr(_) => { let sig = typ.fn_sig(self.cx.tcx); - if let ty::Never = self.cx.tcx.erase_late_bound_regions(&sig).output().sty { + if let ty::Never = self.cx.tcx.erase_late_bound_regions(&sig).output().kind { self.report_diverging_sub_expr(e); } }, @@ -183,7 +177,7 @@ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { /// When such a read is found, the lint is triggered. fn check_for_unsequenced_reads(vis: &mut ReadVisitor<'_, '_>) { let map = &vis.cx.tcx.hir(); - let mut cur_id = vis.write_expr.id; + let mut cur_id = vis.write_expr.hir_id; loop { let parent_id = map.get_parent_node(cur_id); if parent_id == cur_id { @@ -221,11 +215,11 @@ enum StopEarly { } fn check_expr<'a, 'tcx>(vis: &mut ReadVisitor<'a, 'tcx>, expr: &'tcx Expr) -> StopEarly { - if expr.id == vis.last_expr.id { + if expr.hir_id == vis.last_expr.hir_id { return StopEarly::KeepGoing; } - match expr.node { + match expr.kind { ExprKind::Array(_) | ExprKind::Tup(_) | ExprKind::MethodCall(..) @@ -268,27 +262,23 @@ fn check_expr<'a, 'tcx>(vis: &mut ReadVisitor<'a, 'tcx>, expr: &'tcx Expr) -> St } fn check_stmt<'a, 'tcx>(vis: &mut ReadVisitor<'a, 'tcx>, stmt: &'tcx Stmt) -> StopEarly { - match stmt.node { - StmtKind::Expr(ref expr, _) | StmtKind::Semi(ref expr, _) => check_expr(vis, expr), - StmtKind::Decl(ref decl, _) => { - // If the declaration is of a local variable, check its initializer - // expression if it has one. Otherwise, keep going. - let local = match decl.node { - DeclKind::Local(ref local) => Some(local), - _ => None, - }; - local - .and_then(|local| local.init.as_ref()) - .map_or(StopEarly::KeepGoing, |expr| check_expr(vis, expr)) - }, + match stmt.kind { + StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => check_expr(vis, expr), + // If the declaration is of a local variable, check its initializer + // expression if it has one. Otherwise, keep going. + StmtKind::Local(ref local) => local + .init + .as_ref() + .map_or(StopEarly::KeepGoing, |expr| check_expr(vis, expr)), + _ => StopEarly::KeepGoing, } } /// A visitor that looks for reads from a variable. -struct ReadVisitor<'a, 'tcx: 'a> { +struct ReadVisitor<'a, 'tcx> { cx: &'a LateContext<'a, 'tcx>, - /// The id of the variable we're looking for. - var: ast::NodeId, + /// The ID of the variable we're looking for. + var: HirId, /// The expressions where the write to the variable occurred (for reporting /// in the lint). write_expr: &'tcx Expr, @@ -299,16 +289,16 @@ struct ReadVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx> Visitor<'tcx> for ReadVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr) { - if expr.id == self.last_expr.id { + if expr.hir_id == self.last_expr.hir_id { return; } - match expr.node { + match expr.kind { ExprKind::Path(ref qpath) => { if_chain! { if let QPath::Resolved(None, ref path) = *qpath; if path.segments.len() == 1; - if let def::Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id); + if let def::Res::Local(local_id) = self.cx.tables.qpath_res(qpath, expr.hir_id); if local_id == self.var; // Check that this is a read, not a write. if !is_in_assignment_position(self.cx, expr); @@ -339,7 +329,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr) { // ``` // // TODO: fix this - ExprKind::AddrOf(_, _) => { + ExprKind::AddrOf(_, _, _) => { return; } _ => {} @@ -352,11 +342,11 @@ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { } } -/// Returns true if `expr` is the LHS of an assignment, like `expr = ...`. +/// Returns `true` if `expr` is the LHS of an assignment, like `expr = ...`. fn is_in_assignment_position(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { if let Some(parent) = get_parent_expr(cx, expr) { - if let ExprKind::Assign(ref lhs, _) = parent.node { - return lhs.id == expr.id; + if let ExprKind::Assign(ref lhs, _) = parent.kind { + return lhs.hir_id == expr.hir_id; } } false