Merge #3312

3312: OUT_OF_BOUNDS_INDEXING false negative r=phansch a=JoshMcguigan

fixes #3102

Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>

3 files changed, 84 insertions, 46 deletions

diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs
index 984d725898d..f960ab5958c 100644
--- a/clippy_lints/src/indexing_slicing.rs
+++ b/clippy_lints/src/indexing_slicing.rs
@@ -108,19 +108,40 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
         if let ExprKind::Index(ref array, ref index) = &expr.node {
             let ty = cx.tables.expr_ty(array);
             if let Some(range) = higher::range(cx, index) {
+
                 // Ranged indexes, i.e. &x[n..m], &x[n..], &x[..n] and &x[..]
                 if let ty::Array(_, s) = ty.sty {
                     let size: u128 = s.assert_usize(cx.tcx).unwrap().into();
-                    // Index is a constant range.
-                    if let Some((start, end)) = to_const_range(cx, range, size) {
-                        if start > size || end > size {
+
+                    let const_range = to_const_range(cx, range, size);
+
+                    if let (Some(start), _) = const_range {
+                        if start > size {
                             utils::span_lint(
                                 cx,
                                 OUT_OF_BOUNDS_INDEXING,
-                                expr.span,
+                                range.start.map_or(expr.span, |start| start.span),
                                 "range is out of bounds",
                             );
+                            return;
                         }
+                    }
+
+                    if let (_, Some(end)) = const_range {
+                        if end > size {
+                            utils::span_lint(
+                                cx,
+                                OUT_OF_BOUNDS_INDEXING,
+                                range.end.map_or(expr.span, |end| end.span),
+                                "range is out of bounds",
+                            );
+                            return;
+                        }
+                    }
+
+                    if let (Some(_), Some(_)) = const_range {
+                        // early return because both start and end are constants
+                        // and we have proven above that they are in bounds
                         return;
                     }
                 }
@@ -161,20 +182,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
     }
 }
 
-/// Returns an option containing a tuple with the start and end (exclusive) of
-/// the range.
+/// Returns a tuple of options with the start and end (exclusive) values of
+/// the range. If the start or end is not constant, None is returned.
 fn to_const_range<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
     range: Range<'_>,
     array_size: u128,
-) -> Option<(u128, u128)> {
+) -> (Option<u128>, Option<u128>) {
     let s = range
         .start
         .map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c));
     let start = match s {
-        Some(Some(Constant::Int(x))) => x,
-        Some(_) => return None,
-        None => 0,
+        Some(Some(Constant::Int(x))) => Some(x),
+        Some(_) => None,
+        None => Some(0),
     };
 
     let e = range
@@ -182,13 +203,13 @@ fn to_const_range<'a, 'tcx>(
         .map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c));
     let end = match e {
         Some(Some(Constant::Int(x))) => if range.limits == RangeLimits::Closed {
-            x + 1
+            Some(x + 1)
         } else {
-            x
+            Some(x)
         },
-        Some(_) => return None,
-        None => array_size,
+        Some(_) => None,
+        None => Some(array_size),
     };
 
-    Some((start, end))
+    (start, end)
 }
diff --git a/tests/ui/indexing_slicing.rs b/tests/ui/indexing_slicing.rs
index 6a32eb87491..ff154091bb8 100644
--- a/tests/ui/indexing_slicing.rs
+++ b/tests/ui/indexing_slicing.rs
@@ -91,4 +91,9 @@ fn main() {
     x[M]; // Ok, should not produce stderr.
     v[N];
     v[M];
+
+    // issue 3102
+    let num = 1;
+    &x[num..10]; // should trigger out of bounds error
+    &x[10..num]; // should trigger out of bounds error
 }
diff --git a/tests/ui/indexing_slicing.stderr b/tests/ui/indexing_slicing.stderr
index 7d847c7a673..fafcb1bc485 100644
--- a/tests/ui/indexing_slicing.stderr
+++ b/tests/ui/indexing_slicing.stderr
@@ -48,18 +48,18 @@ error: slicing may panic.
    = help: Consider using `.get(n..)` or .get_mut(n..)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:30:6
+  --> $DIR/indexing_slicing.rs:30:11
    |
 30 |     &x[..=4];
-   |      ^^^^^^^
+   |           ^
    |
    = note: `-D clippy::out-of-bounds-indexing` implied by `-D warnings`
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:31:6
+  --> $DIR/indexing_slicing.rs:31:11
    |
 31 |     &x[1..5];
-   |      ^^^^^^^
+   |           ^
 
 error: slicing may panic.
   --> $DIR/indexing_slicing.rs:32:6
@@ -70,34 +70,34 @@ error: slicing may panic.
    = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:32:6
+  --> $DIR/indexing_slicing.rs:32:8
    |
 32 |     &x[5..][..10]; // Two lint reports, one for [5..] and another for [..10].
-   |      ^^^^^^
+   |        ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:33:6
+  --> $DIR/indexing_slicing.rs:33:8
    |
 33 |     &x[5..];
-   |      ^^^^^^
+   |        ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:34:6
+  --> $DIR/indexing_slicing.rs:34:10
    |
 34 |     &x[..5];
-   |      ^^^^^^
+   |          ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:35:6
+  --> $DIR/indexing_slicing.rs:35:8
    |
 35 |     &x[5..].iter().map(|x| 2 * x).collect::<Vec<i32>>();
-   |      ^^^^^^
+   |        ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:36:6
+  --> $DIR/indexing_slicing.rs:36:12
    |
 36 |     &x[0..=4];
-   |      ^^^^^^^^
+   |            ^
 
 error: slicing may panic.
   --> $DIR/indexing_slicing.rs:37:6
@@ -148,46 +148,46 @@ error: slicing may panic.
    = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:60:6
+  --> $DIR/indexing_slicing.rs:60:12
    |
 60 |     &empty[1..5];
-   |      ^^^^^^^^^^^
+   |            ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:61:6
+  --> $DIR/indexing_slicing.rs:61:16
    |
 61 |     &empty[0..=4];
-   |      ^^^^^^^^^^^^
+   |                ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:62:6
+  --> $DIR/indexing_slicing.rs:62:15
    |
 62 |     &empty[..=4];
-   |      ^^^^^^^^^^^
+   |               ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:63:6
+  --> $DIR/indexing_slicing.rs:63:12
    |
 63 |     &empty[1..];
-   |      ^^^^^^^^^^
+   |            ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:64:6
+  --> $DIR/indexing_slicing.rs:64:14
    |
 64 |     &empty[..4];
-   |      ^^^^^^^^^^
+   |              ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:65:6
+  --> $DIR/indexing_slicing.rs:65:16
    |
 65 |     &empty[0..=0];
-   |      ^^^^^^^^^^^^
+   |                ^
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:66:6
+  --> $DIR/indexing_slicing.rs:66:15
    |
 66 |     &empty[..=0];
-   |      ^^^^^^^^^^^
+   |               ^
 
 error: indexing may panic.
   --> $DIR/indexing_slicing.rs:74:5
@@ -230,10 +230,10 @@ error: slicing may panic.
    = help: Consider using `.get(..n)`or `.get_mut(..n)` instead
 
 error: range is out of bounds
-  --> $DIR/indexing_slicing.rs:78:6
+  --> $DIR/indexing_slicing.rs:78:8
    |
 78 |     &x[10..][..100]; // Two lint reports, one for [10..] and another for [..100].
-   |      ^^^^^^^
+   |        ^^
 
 error: slicing may panic.
   --> $DIR/indexing_slicing.rs:79:6
@@ -267,5 +267,17 @@ error: indexing may panic.
    |
    = help: Consider using `.get(n)` or `.get_mut(n)` instead
 
-error: aborting due to 37 previous errors
+error: range is out of bounds
+  --> $DIR/indexing_slicing.rs:97:13
+   |
+97 |     &x[num..10]; // should trigger out of bounds error
+   |             ^^
+
+error: range is out of bounds
+  --> $DIR/indexing_slicing.rs:98:8
+   |
+98 |     &x[10..num]; // should trigger out of bounds error
+   |        ^^
+
+error: aborting due to 39 previous errors