OUT_OF_BOUNDS_INDEXING fix #3102 false negative

Author: JoshMcguigan

clippy_lints/src/indexing_slicing.rs

@@ -111,17 +111,43 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
                 // 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 {
-                            utils::span_lint(
-                                cx,
-                                OUT_OF_BOUNDS_INDEXING,
-                                expr.span,
-                                "range is out of bounds",
-                            );
-                        }
-                        return;
+
+                    match to_const_range(cx, range, size) {
+                        (None, None) => {},
+                        (Some(start), None) => {
+                            if start > size {
+                                utils::span_lint(
+                                    cx,
+                                    OUT_OF_BOUNDS_INDEXING,
+                                    expr.span,
+                                    "range is out of bounds",
+                                );
+                                return;
+                            }
+                        },
+                        (None, Some(end)) => {
+                            if end > size {
+                                utils::span_lint(
+                                    cx,
+                                    OUT_OF_BOUNDS_INDEXING,
+                                    expr.span,
+                                    "range is out of bounds",
+                                );
+                                return;
+                            }
+                        },
+                        (Some(start), Some(end)) => {
+                            if start > size || end > size {
+                                utils::span_lint(
+                                    cx,
+                                    OUT_OF_BOUNDS_INDEXING,
+                                    expr.span,
+                                    "range is out of bounds",
+                                );
+                            }
+                            // early return because both start and end are constant
+                            return;
+                        },
                     }
                 }
 
@@ -161,34 +187,34 @@ 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
         .end
         .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)
 }

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
 }

tests/ui/indexing_slicing.stderr

@@ -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:6
+   |
+97 |     &x[num..10]; // should trigger out of bounds error
+   |      ^^^^^^^^^^
+
+error: range is out of bounds
+  --> $DIR/indexing_slicing.rs:98:6
+   |
+98 |     &x[10..num]; // should trigger out of bounds error
+   |      ^^^^^^^^^^
+
+error: aborting due to 39 previous errors