Merge remote-tracking branch 'origin/master'

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
+* New [`mut_from_ref`] lint
+
 ## 0.0.114 — 2017-02-08
 * Rustup to rustc 1.17.0-nightly (c49d10207 2017-02-07)
 * Tests are now ui tests (testing the exact output of rustc)
@@ -370,6 +372,7 @@ All notable changes to this project will be documented in this file.
 [`mixed_case_hex_literals`]: https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals
 [`module_inception`]: https://github.com/Manishearth/rust-clippy/wiki#module_inception
 [`modulo_one`]: https://github.com/Manishearth/rust-clippy/wiki#modulo_one
+[`mut_from_ref`]: https://github.com/Manishearth/rust-clippy/wiki#mut_from_ref
 [`mut_mut`]: https://github.com/Manishearth/rust-clippy/wiki#mut_mut
 [`mutex_atomic`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic
 [`mutex_integer`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_integer

README.md

@@ -256,7 +256,7 @@ name
 [iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop)                                       | warn    | for-looping over `_.next()` which is probably not intended
 [iter_nth](https://github.com/Manishearth/rust-clippy/wiki#iter_nth)                                                   | warn    | using `.iter().nth()` on a standard library type with O(1) element access
 [iter_skip_next](https://github.com/Manishearth/rust-clippy/wiki#iter_skip_next)                                       | warn    | using `.skip(x).next()` on an iterator
-[large_enum_variant](https://github.com/Manishearth/rust-clippy/wiki#large_enum_variant)                               | warn    | large variants on an enum
+[large_enum_variant](https://github.com/Manishearth/rust-clippy/wiki#large_enum_variant)                               | warn    | large size difference between variants on an enum
 [len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty)                           | warn    | traits or impls with a public `len` method but no corresponding `is_empty` method
 [len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero)                                                   | warn    | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead
 [let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return)                                       | warn    | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block
@@ -279,6 +279,7 @@ name
 [mixed_case_hex_literals](https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals)                     | warn    | hex literals whose letter digits are not consistently upper- or lowercased
 [module_inception](https://github.com/Manishearth/rust-clippy/wiki#module_inception)                                   | warn    | modules that have the same name as their parent module
 [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one)                                               | warn    | taking a number modulo 1, which always returns 0
+[mut_from_ref](https://github.com/Manishearth/rust-clippy/wiki#mut_from_ref)                                           | warn    | fns that create mutable refs from immutable ref args
 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut)                                                     | allow   | usage of double-mut refs, e.g. `&mut &mut ...`
 [mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic)                                           | warn    | using a mutex where an atomic value could be used instead
 [mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer)                                         | allow   | using a mutex for an integer type

clippy_lints/src/large_enum_variant.rs

@@ -1,4 +1,4 @@
-//! lint when there are large variants on an enum
+//! lint when there is a large size difference between variants on an enum
 
 use rustc::lint::*;
 use rustc::hir::*;
@@ -7,7 +7,7 @@ use rustc::ty::layout::TargetDataLayout;
 use rustc::ty::TypeFoldable;
 use rustc::traits::Reveal;
 
-/// **What it does:** Checks for large variants on `enum`s.
+/// **What it does:** Checks for large size differences between variants on `enum`s.
 ///
 /// **Why is this bad?** Enum size is bounded by the largest variant. Having a large variant
 /// can penalize the memory layout of that enum.
@@ -24,17 +24,17 @@ use rustc::traits::Reveal;
 declare_lint! {
     pub LARGE_ENUM_VARIANT,
     Warn,
-    "large variants on an enum"
+    "large size difference between variants on an enum"
 }
 
 #[derive(Copy,Clone)]
 pub struct LargeEnumVariant {
-    maximum_variant_size_allowed: u64,
+    maximum_size_difference_allowed: u64,
 }
 
 impl LargeEnumVariant {
-    pub fn new(maximum_variant_size_allowed: u64) -> Self {
+    pub fn new(maximum_size_difference_allowed: u64) -> Self {
-        LargeEnumVariant { maximum_variant_size_allowed: maximum_variant_size_allowed }
+        LargeEnumVariant { maximum_size_difference_allowed: maximum_size_difference_allowed }
     }
 }
 
@@ -50,6 +50,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
         if let ItemEnum(ref def, _) = item.node {
             let ty = cx.tcx.item_type(did);
             let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
+
+            let mut smallest_variant: Option<(_, _)> = None;
+            let mut largest_variant: Option<(_, _)> = None;
+
             for (i, variant) in adt.variants.iter().enumerate() {
                 let data_layout = TargetDataLayout::parse(cx.sess());
                 cx.tcx.infer_ctxt((), Reveal::All).enter(|infcx| {
@@ -67,32 +71,57 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
                             }
                         })
                         .sum();
-                    if size > self.maximum_variant_size_allowed {
+
-                        span_lint_and_then(cx,
+                    let grouped = (size, (i, variant));
-                                           LARGE_ENUM_VARIANT,
+
-                                           def.variants[i].span,
+                    update_if(&mut smallest_variant, grouped, |a, b| b.0 <= a.0);
-                                           "large enum variant found",
+                    update_if(&mut largest_variant, grouped, |a, b| b.0 >= a.0);
-                                           |db| {
-                            if variant.fields.len() == 1 {
-                                let span = match def.variants[i].node.data {
-                                    VariantData::Struct(ref fields, _) |
-                                    VariantData::Tuple(ref fields, _) => fields[0].ty.span,
-                                    VariantData::Unit(_) => unreachable!(),
-                                };
-                                if let Some(snip) = snippet_opt(cx, span) {
-                                    db.span_suggestion(span,
-                                                       "consider boxing the large fields to reduce the total size of \
-                                                        the enum",
-                                                       format!("Box<{}>", snip));
-                                    return;
-                                }
-                            }
-                            db.span_help(def.variants[i].span,
-                                         "consider boxing the large fields to reduce the total size of the enum");
-                        });
-                    }
                 });
             }
+
+            if let (Some(smallest), Some(largest)) = (smallest_variant, largest_variant) {
+                let difference = largest.0 - smallest.0;
+
+                if difference > self.maximum_size_difference_allowed {
+                    let (i, variant) = largest.1;
+
+                    span_lint_and_then(cx,
+                                       LARGE_ENUM_VARIANT,
+                                       def.variants[i].span,
+                                       "large size difference between variants",
+                                       |db| {
+                        if variant.fields.len() == 1 {
+                            let span = match def.variants[i].node.data {
+                                VariantData::Struct(ref fields, _) |
+                                VariantData::Tuple(ref fields, _) => fields[0].ty.span,
+                                VariantData::Unit(_) => unreachable!(),
+                            };
+                            if let Some(snip) = snippet_opt(cx, span) {
+                                db.span_suggestion(span,
+                                                   "consider boxing the large fields to reduce the total size of the \
+                                                    enum",
+                                                   format!("Box<{}>", snip));
+                                return;
+                            }
+                        }
+                        db.span_help(def.variants[i].span,
+                                     "consider boxing the large fields to reduce the total size of the enum");
+                    });
+                }
+            }
+
         }
     }
 }
+
+fn update_if<T, F>(old: &mut Option<T>, new: T, f: F)
+    where F: Fn(&T, &T) -> bool
+{
+    if let Some(ref mut val) = *old {
+        if f(val, &new) {
+            *val = new;
+        }
+    } else {
+        *old = Some(new);
+    }
+}

clippy_lints/src/lib.rs

@@ -15,6 +15,7 @@
 #![allow(needless_lifetimes)]
 
 extern crate syntax;
+extern crate syntax_pos;
 #[macro_use]
 extern crate rustc;
 extern crate rustc_data_structures;
@@ -465,6 +466,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         precedence::PRECEDENCE,
         print::PRINT_WITH_NEWLINE,
         ptr::CMP_NULL,
+        ptr::MUT_FROM_REF,
         ptr::PTR_ARG,
         ranges::RANGE_STEP_BY_ZERO,
         ranges::RANGE_ZIP_WITH_LEN,

clippy_lints/src/ptr.rs

@@ -5,7 +5,9 @@ use rustc::hir::map::NodeItem;
 use rustc::lint::*;
 use rustc::ty;
 use syntax::ast::NodeId;
-use utils::{match_path, match_type, paths, span_lint};
+use syntax::codemap::Span;
+use syntax_pos::MultiSpan;
+use utils::{match_path, match_type, paths, span_lint, span_lint_and_then};
 
 /// **What it does:** This lint checks for function arguments of type `&String` or `&Vec` unless
 /// the references are mutable.
@@ -44,13 +46,32 @@ declare_lint! {
     "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead."
 }
 
+/// **What it does:** This lint checks for functions that take immutable references and return
+/// mutable ones.
+///
+/// **Why is this bad?** This is trivially unsound, as one can create two mutable references
+/// from the same (immutable!) source. This [error](https://github.com/rust-lang/rust/issues/39465)
+/// actually lead to an interim Rust release 1.15.1.
+///
+/// **Known problems:** To be on the conservative side, if there's at least one mutable reference
+/// with the output lifetime, this lint will not trigger. In practice, this case is unlikely anyway.
+///
+/// **Example:**
+/// ```rust
+/// fn foo(&Foo) -> &mut Bar { .. }
+/// ```
+declare_lint! {
+    pub MUT_FROM_REF,
+    Warn,
+    "fns that create mutable refs from immutable ref args"
+}
 
 #[derive(Copy,Clone)]
 pub struct PointerPass;
 
 impl LintPass for PointerPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(PTR_ARG, CMP_NULL)
+        lint_array!(PTR_ARG, CMP_NULL, MUT_FROM_REF)
     }
 }
 
@@ -111,6 +132,37 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) {
             }
         }
     }
+
+    if let FunctionRetTy::Return(ref ty) = decl.output {
+        if let Some((out, MutMutable, _)) = get_rptr_lm(ty) {
+            let mut immutables = vec![];
+            for (_, ref mutbl, ref argspan) in
+                decl.inputs
+                    .iter()
+                    .filter_map(|ty| get_rptr_lm(ty))
+                    .filter(|&(lt, _, _)| lt.name == out.name) {
+                if *mutbl == MutMutable {
+                    return;
+                }
+                immutables.push(*argspan);
+            }
+            if immutables.is_empty() {
+                return;
+            }
+            span_lint_and_then(cx, MUT_FROM_REF, ty.span, "mutable borrow from immutable input(s)", |db| {
+                let ms = MultiSpan::from_spans(immutables);
+                db.span_note(ms, "immutable borrow here");
+            });
+        }
+    }
+}
+
+fn get_rptr_lm(ty: &Ty) -> Option<(&Lifetime, Mutability, Span)> {
+    if let Ty_::TyRptr(ref lt, ref m) = ty.node {
+        Some((lt, m.mutbl, ty.span))
+    } else {
+        None
+    }
 }
 
 fn is_null_path(expr: &Expr) -> bool {

src/main.rs

@@ -1,7 +1,6 @@
 // error-pattern:yummy
 #![feature(box_syntax)]
 #![feature(rustc_private)]
-#![feature(static_in_const)]
 
 #![allow(unknown_lints, missing_docs_in_private_items)]
 

tests/ui/large_enum_variant.rs

@@ -8,18 +8,17 @@
 enum LargeEnum {
     A(i32),
     B([i32; 8000]),
-
-
 }
 
-enum GenericEnum<T> {
+enum GenericEnumOk<T> {
+    A(i32),
+    B([T; 8000]),
+}
+
+enum GenericEnum2<T> {
     A(i32),
     B([i32; 8000]),
-
+    C(T, [i32; 8000]),
-
-    C([T; 8000]),
-    D(T, [i32; 8000]),
-
 }
 
 trait SomeTrait {
@@ -27,27 +26,32 @@ trait SomeTrait {
 }
 
 enum LargeEnumGeneric<A: SomeTrait> {
-    Var(A::Item), // regression test, this used to ICE
+    Var(A::Item),
 }
 
-enum AnotherLargeEnum {
+enum LargeEnum2 {
     VariantOk(i32, u32),
     ContainingLargeEnum(LargeEnum),
-
+}
-
+enum LargeEnum3 {
     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
-
     VoidVariant,
     StructLikeLittle { x: i32, y: i32 },
+}
+
+enum LargeEnum4 {
+    VariantOk(i32, u32),
     StructLikeLarge { x: [i32; 8000], y: i32 },
-
-    StructLikeLarge2 {
-        x:
-        [i32; 8000]
-
-    },
 }
 
-fn main() {
+enum LargeEnum5 {
-
+    VariantOk(i32, u32),
+    StructLikeLarge2 { x: [i32; 8000] },
 }
+
+enum LargeEnumOk {
+    LargeA([i32; 8000]),
+    LargeB([i32; 8001]),
+}
+
+fn main() {}

tests/ui/large_enum_variant.stderr

@@ -1,4 +1,4 @@
-error: large enum variant found
+error: large size difference between variants
   --> $DIR/large_enum_variant.rs:10:5
    |
 10 |     B([i32; 8000]),
@@ -12,73 +12,59 @@ note: lint level defined here
 help: consider boxing the large fields to reduce the total size of the enum
    |     B(Box<[i32; 8000]>),
 
-error: large enum variant found
+error: large size difference between variants
-  --> $DIR/large_enum_variant.rs:17:5
-   |
-17 |     B([i32; 8000]),
-   |     ^^^^^^^^^^^^^^
-   |
-help: consider boxing the large fields to reduce the total size of the enum
-   |     B(Box<[i32; 8000]>),
-
-error: large enum variant found
   --> $DIR/large_enum_variant.rs:21:5
    |
-21 |     D(T, [i32; 8000]),
+21 |     C(T, [i32; 8000]),
    |     ^^^^^^^^^^^^^^^^^
    |
 help: consider boxing the large fields to reduce the total size of the enum
   --> $DIR/large_enum_variant.rs:21:5
    |
-21 |     D(T, [i32; 8000]),
+21 |     C(T, [i32; 8000]),
    |     ^^^^^^^^^^^^^^^^^
 
-error: large enum variant found
+error: large size difference between variants
-  --> $DIR/large_enum_variant.rs:35:5
+  --> $DIR/large_enum_variant.rs:34:5
    |
-35 |     ContainingLargeEnum(LargeEnum),
+34 |     ContainingLargeEnum(LargeEnum),
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 help: consider boxing the large fields to reduce the total size of the enum
    |     ContainingLargeEnum(Box<LargeEnum>),
 
-error: large enum variant found
+error: large size difference between variants
-  --> $DIR/large_enum_variant.rs:38:5
+  --> $DIR/large_enum_variant.rs:37:5
    |
-38 |     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
+37 |     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 help: consider boxing the large fields to reduce the total size of the enum
-  --> $DIR/large_enum_variant.rs:38:5
+  --> $DIR/large_enum_variant.rs:37:5
    |
-38 |     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
+37 |     ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: large enum variant found
+error: large size difference between variants
-  --> $DIR/large_enum_variant.rs:42:5
-   |
-42 |     StructLikeLarge { x: [i32; 8000], y: i32 },
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: consider boxing the large fields to reduce the total size of the enum
-  --> $DIR/large_enum_variant.rs:42:5
-   |
-42 |     StructLikeLarge { x: [i32; 8000], y: i32 },
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: large enum variant found
   --> $DIR/large_enum_variant.rs:44:5
    |
-44 |       StructLikeLarge2 {
+44 |     StructLikeLarge { x: [i32; 8000], y: i32 },
-   |  _____^ starting here...
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-45 | |         x:
-46 | |         [i32; 8000]
-47 | |
-48 | |     },
-   | |_____^ ...ending here
    |
 help: consider boxing the large fields to reduce the total size of the enum
-   |         Box<[i32; 8000]>
+  --> $DIR/large_enum_variant.rs:44:5
+   |
+44 |     StructLikeLarge { x: [i32; 8000], y: i32 },
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 7 previous errors
+error: large size difference between variants
+  --> $DIR/large_enum_variant.rs:49:5
+   |
+49 |     StructLikeLarge2 { x: [i32; 8000] },
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider boxing the large fields to reduce the total size of the enum
+   |     StructLikeLarge2 { x: Box<[i32; 8000]> },
+
+error: aborting due to 6 previous errors
 

tests/ui/mut_from_ref.rs

@@ -0,0 +1,48 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+#![allow(unused)]
+#![deny(mut_from_ref)]
+
+struct Foo;
+
+impl Foo {
+    fn this_wont_hurt_a_bit(&self) -> &mut Foo {
+        unimplemented!()
+    }
+}
+
+trait Ouch {
+    fn ouch(x: &Foo) -> &mut Foo;
+}
+
+impl Ouch for Foo {
+    fn ouch(x: &Foo) -> &mut Foo {
+        unimplemented!()
+    }
+}
+
+fn fail(x: &u32) -> &mut u16 {
+    unimplemented!()
+}
+
+fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 {
+    unimplemented!()
+}
+
+fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
+    unimplemented!()
+}
+
+// this is OK, because the result borrows y
+fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 {
+    unimplemented!()
+}
+
+// this is also OK, because the result could borrow y
+fn also_works<'a>(x: &'a u32, y: &'a mut u32) -> &'a mut u32 {
+    unimplemented!()
+}
+
+fn main() {
+    //TODO
+}

tests/ui/mut_from_ref.stderr

@@ -0,0 +1,67 @@
+error: mutable borrow from immutable input(s)
+ --> $DIR/mut_from_ref.rs:9:39
+  |
+9 |     fn this_wont_hurt_a_bit(&self) -> &mut Foo {
+  |                                       ^^^^^^^^
+  |
+note: lint level defined here
+ --> $DIR/mut_from_ref.rs:4:9
+  |
+4 | #![deny(mut_from_ref)]
+  |         ^^^^^^^^^^^^
+note: immutable borrow here
+ --> $DIR/mut_from_ref.rs:9:29
+  |
+9 |     fn this_wont_hurt_a_bit(&self) -> &mut Foo {
+  |                             ^^^^^
+
+error: mutable borrow from immutable input(s)
+  --> $DIR/mut_from_ref.rs:15:25
+   |
+15 |     fn ouch(x: &Foo) -> &mut Foo;
+   |                         ^^^^^^^^
+   |
+note: immutable borrow here
+  --> $DIR/mut_from_ref.rs:15:16
+   |
+15 |     fn ouch(x: &Foo) -> &mut Foo;
+   |                ^^^^
+
+error: mutable borrow from immutable input(s)
+  --> $DIR/mut_from_ref.rs:24:21
+   |
+24 | fn fail(x: &u32) -> &mut u16 {
+   |                     ^^^^^^^^
+   |
+note: immutable borrow here
+  --> $DIR/mut_from_ref.rs:24:12
+   |
+24 | fn fail(x: &u32) -> &mut u16 {
+   |            ^^^^
+
+error: mutable borrow from immutable input(s)
+  --> $DIR/mut_from_ref.rs:28:50
+   |
+28 | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 {
+   |                                                  ^^^^^^^^^^^
+   |
+note: immutable borrow here
+  --> $DIR/mut_from_ref.rs:28:25
+   |
+28 | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 {
+   |                         ^^^^^^^
+
+error: mutable borrow from immutable input(s)
+  --> $DIR/mut_from_ref.rs:32:67
+   |
+32 | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
+   |                                                                   ^^^^^^^^^^^
+   |
+note: immutable borrow here
+  --> $DIR/mut_from_ref.rs:32:27
+   |
+32 | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
+   |                           ^^^^^^^     ^^^^^^^
+
+error: aborting due to 5 previous errors
+