]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/eval_order_dependence.rs
modify code
[rust.git] / clippy_lints / src / eval_order_dependence.rs
index 762f64fe37ad6862f19edf05576a747ec76e158e..65599a0587d449ab2150abdb280442d808e6a48c 100644 (file)
@@ -1,24 +1,27 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_note};
 use clippy_utils::{get_parent_expr, path_to_local, path_to_local_id};
-use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
+use if_chain::if_chain;
+use rustc_hir::intravisit::{walk_expr, Visitor};
 use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Guard, HirId, Local, Node, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::hir::map::Map;
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for a read and a write to the same variable where
+    /// ### 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.
+    /// ### Why is this bad?
+    /// It is often confusing to read. As described [here](https://doc.rust-lang.org/reference/expressions.html?highlight=subexpression#evaluation-order-of-operands),
+    /// the operands of these expressions are evaluated before applying the effects of the expression.
     ///
-    /// **Known problems:** Code which intentionally depends on the evaluation
+    /// ### Known problems
+    /// Code which intentionally depends on the evaluation
     /// order, or which is correct for any evaluation order.
     ///
-    /// **Example:**
+    /// ### Example
     /// ```rust
     /// let mut x = 0;
     ///
     /// };
     /// let a = tmp + x;
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub EVAL_ORDER_DEPENDENCE,
-    complexity,
+    suspicious,
     "whether a variable read occurs before a write depends on sub-expression evaluation order"
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for diverging calls that are not match arms or
+    /// ### 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
+    /// ### 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
+    /// ### Known problems
+    /// Someone might want to use `some_bool || panic!()` as a
     /// shorthand.
     ///
-    /// **Example:**
+    /// ### Example
     /// ```rust,no_run
     /// # fn b() -> bool { true }
     /// # fn c() -> bool { true }
@@ -60,6 +67,7 @@
     /// let x = (a, b, c, panic!());
     /// // can simply be replaced by `panic!()`
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub DIVERGING_SUB_EXPRESSION,
     complexity,
     "whether an expression contains a diverging sub expression"
 impl<'tcx> LateLintPass<'tcx> for EvalOrderDependence {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         // Find a write to a local variable.
-        match expr.kind {
-            ExprKind::Assign(lhs, ..) | ExprKind::AssignOp(_, lhs, _) => {
-                if let Some(var) = path_to_local(lhs) {
-                    let mut visitor = ReadVisitor {
-                        cx,
-                        var,
-                        write_expr: expr,
-                        last_expr: expr,
-                    };
-                    check_for_unsequenced_reads(&mut visitor);
-                }
-            },
-            _ => {},
-        }
+        let var = if_chain! {
+            if let ExprKind::Assign(lhs, ..) | ExprKind::AssignOp(_, lhs, _) = expr.kind;
+            if let Some(var) = path_to_local(lhs);
+            if expr.span.desugaring_kind().is_none();
+            then { var } else { return; }
+        };
+        let mut visitor = ReadVisitor {
+            cx,
+            var,
+            write_expr: expr,
+            last_expr: expr,
+        };
+        check_for_unsequenced_reads(&mut visitor);
     }
     fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
         match stmt.kind {
@@ -110,7 +117,7 @@ fn maybe_walk_expr(&mut self, e: &'tcx Expr<'_>) {
                 self.visit_expr(e);
                 for arm in arms {
                     if let Some(Guard::If(if_expr)) = arm.guard {
-                        self.visit_expr(if_expr)
+                        self.visit_expr(if_expr);
                     }
                     // make sure top level arm expressions aren't linted
                     self.maybe_walk_expr(&*arm.body);
@@ -125,8 +132,6 @@ fn report_diverging_sub_expr(&mut self, e: &Expr<'_>) {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
-
     fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
         match e.kind {
             ExprKind::Continue(_) | ExprKind::Break(_, _) | ExprKind::Ret(_) => self.report_diverging_sub_expr(e),
@@ -135,7 +140,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
                 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().kind() {
+                        if self.cx.tcx.erase_late_bound_regions(sig).output().kind() == &ty::Never {
                             self.report_diverging_sub_expr(e);
                         }
                     },
@@ -159,9 +164,6 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
     fn visit_block(&mut self, _: &'tcx Block<'_>) {
         // don't continue over blocks, LateLintPass already does that
     }
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
-    }
 }
 
 /// Walks up the AST from the given write expression (`vis.write_expr`) looking
@@ -291,8 +293,6 @@ struct ReadVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for ReadVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
-
     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
         if expr.hir_id == self.last_expr.hir_id {
             return;
@@ -305,7 +305,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
                     self.cx,
                     EVAL_ORDER_DEPENDENCE,
                     expr.span,
-                    "unsequenced read of a variable",
+                    &format!("unsequenced read of `{}`", self.cx.tcx.hir().name(self.var)),
                     Some(self.write_expr.span),
                     "whether read occurs before this write depends on evaluation order",
                 );
@@ -335,9 +335,6 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
 
         walk_expr(self, expr);
     }
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
-    }
 }
 
 /// Returns `true` if `expr` is the LHS of an assignment, like `expr = ...`.