diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ea0fb88b09..84bbf87d29c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -211,6 +211,7 @@ All notable changes to this project will be documented in this file.
[`cyclomatic_complexity`]: https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity
[`deprecated_semver`]: https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver
[`derive_hash_xor_eq`]: https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq
+[`diverging_sub_expression`]: https://github.com/Manishearth/rust-clippy/wiki#diverging_sub_expression
[`doc_markdown`]: https://github.com/Manishearth/rust-clippy/wiki#doc_markdown
[`double_neg`]: https://github.com/Manishearth/rust-clippy/wiki#double_neg
[`drop_ref`]: https://github.com/Manishearth/rust-clippy/wiki#drop_ref
diff --git a/README.md b/README.md
index 83b9788fba5..ba0973b0e67 100644
--- a/README.md
+++ b/README.md
@@ -17,7 +17,7 @@ Table of contents:
## Lints
-There are 170 lints included in this crate:
+There are 171 lints included in this crate:
name | default | triggers on
---------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -50,6 +50,7 @@ name
[cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | functions that should be split up into multiple functions
[deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | use of `#[deprecated(since = "x")]` where x is not semver
[derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly
+[diverging_sub_expression](https://github.com/Manishearth/rust-clippy/wiki#diverging_sub_expression) | warn | whether an expression contains a diverging sub expression
[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | presence of `_`, `::` or camel-case outside backticks in documentation
[double_neg](https://github.com/Manishearth/rust-clippy/wiki#double_neg) | warn | `--x`, which is a double negation of `x` and not a pre-decrement as in C/C++
[drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | calls to `std::mem::drop` with a reference instead of an owned value
diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs
index 4465d31bff5..d6964c156eb 100644
--- a/clippy_lints/src/eval_order_dependence.rs
+++ b/clippy_lints/src/eval_order_dependence.rs
@@ -1,8 +1,9 @@
use rustc::hir::def_id::DefId;
use rustc::hir::intravisit::{Visitor, walk_expr};
use rustc::hir::*;
+use rustc::ty;
use rustc::lint::*;
-use utils::{get_parent_expr, span_note_and_lint};
+use utils::{get_parent_expr, span_note_and_lint, span_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
@@ -26,12 +27,32 @@ declare_lint! {
"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_lint! {
+ pub DIVERGING_SUB_EXPRESSION,
+ Warn,
+ "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)
+ lint_array!(EVAL_ORDER_DEPENDENCE, DIVERGING_SUB_EXPRESSION)
}
}
@@ -56,6 +77,75 @@ impl LateLintPass for EvalOrderDependence {
_ => {}
}
}
+ fn check_stmt(&mut self, cx: &LateContext, stmt: &Stmt) {
+ match stmt.node {
+ StmtExpr(ref e, _) | StmtSemi(ref e, _) => DivergenceVisitor(cx).maybe_walk_expr(e),
+ StmtDecl(ref d, _) => {
+ if let DeclLocal(ref local) = d.node {
+ if let Local { init: Some(ref e), .. } = **local {
+ DivergenceVisitor(cx).visit_expr(e);
+ }
+ }
+ },
+ }
+ }
+}
+
+struct DivergenceVisitor<'a, 'tcx: 'a>(&'a LateContext<'a, 'tcx>);
+
+impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
+ fn maybe_walk_expr(&mut self, e: &Expr) {
+ match e.node {
+ ExprClosure(..) => {},
+ ExprMatch(ref e, ref arms, _) => {
+ self.visit_expr(e);
+ for arm in arms {
+ if let Some(ref guard) = arm.guard {
+ self.visit_expr(guard);
+ }
+ // make sure top level arm expressions aren't linted
+ walk_expr(self, &*arm.body);
+ }
+ }
+ _ => walk_expr(self, e),
+ }
+ }
+ fn report_diverging_sub_expr(&mut self, e: &Expr) {
+ span_lint(
+ self.0,
+ DIVERGING_SUB_EXPRESSION,
+ e.span,
+ "sub-expression diverges",
+ );
+ }
+}
+
+impl<'a, 'tcx, 'v> Visitor<'v> for DivergenceVisitor<'a, 'tcx> {
+ fn visit_expr(&mut self, e: &'v Expr) {
+ // this match can be replaced by just the default arm, once
+ // https://github.com/rust-lang/rust/issues/35121 makes sure that
+ // ! is propagated properly
+ match e.node {
+ ExprAgain(_) |
+ ExprBreak(_) |
+ ExprRet(_) => self.report_diverging_sub_expr(e),
+ ExprCall(ref func, _) => match self.0.tcx.expr_ty(func).sty {
+ ty::TyFnDef(_, _, fn_ty) |
+ ty::TyFnPtr(fn_ty) => if let ty::TyNever = self.0.tcx.erase_late_bound_regions(&fn_ty.sig).output.sty {
+ self.report_diverging_sub_expr(e);
+ },
+ _ => {},
+ },
+ ExprMethodCall(..) => { /* TODO */ },
+ _ => if let ty::TyNever = self.0.tcx.expr_ty(e).sty {
+ self.report_diverging_sub_expr(e);
+ },
+ }
+ self.maybe_walk_expr(e);
+ }
+ fn visit_block(&mut self, _: &'v Block) {
+ // don't continue over blocks, LateLintPass already does that
+ }
}
/// Walks up the AST from the the given write expression (`vis.write_expr`)
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 36d466ed9c9..27842d6addd 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -332,6 +332,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
eq_op::EQ_OP,
escape::BOXED_LOCAL,
eta_reduction::REDUNDANT_CLOSURE,
+ eval_order_dependence::DIVERGING_SUB_EXPRESSION,
eval_order_dependence::EVAL_ORDER_DEPENDENCE,
format::USELESS_FORMAT,
formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
diff --git a/tests/compile-fail/diverging_sub_expression.rs b/tests/compile-fail/diverging_sub_expression.rs
new file mode 100644
index 00000000000..929ef911f43
--- /dev/null
+++ b/tests/compile-fail/diverging_sub_expression.rs
@@ -0,0 +1,14 @@
+#![feature(plugin, never_type)]
+#![plugin(clippy)]
+#![deny(diverging_sub_expression)]
+
+#[allow(empty_loop)]
+fn diverge() -> ! { loop {} }
+
+#[allow(unused_variables, unnecessary_operation)]
+fn main() {
+ let b = true;
+ b || diverge(); //~ ERROR sub-expression diverges
+ let y = (5, diverge(), 6); //~ ERROR sub-expression diverges
+ println!("{}", y.1); //~ ERROR sub-expression diverges
+}