reversed_empty_ranges false positive

[nilgoyette]

As a user of ndarray, I can use negative indexing in slices, like I would do with NumPy

// A view of the image without borders
let block = image.slice(s![1..-1, 1..-1, 1..-1]);

Since Rust 1.45.0, I get this error when I run clippy: error: this range is empty so it will yield no values. This message is wrong: my code compiles and runs as intended. Clippy shouldn't only check if end <= start, it should also check if end is negative, or if it's a isize slice.

[flip1995]

TIL, Rust has negative indices in ranges 😮

cc @ebroto

[AB1908]

How can I try and reproduce this bug?

[nilgoyette]

ndarray = "0.13"

use ndarray::{arr1, s};
let a = arr1(&[1, 2, 3, 4, 5]);
println!("{}", a.slice(s![1..-1])); // Should print [2, 3, 4]

I just tested in a real project and I do see the reversed_empty_ranges error.

[AB1908]

Okay, I'll give this a shot.

[AB1908]

As per the discussion in the #clippy channel, the best way to fix this would be to add a check for whether the range is inside a macro.

[AB1908]

So...I kinda failed hard on this one. Hopefully, someone else picks it up.

[ebroto]

I've been looking into this, but it seems the fix is not as simple as it seemed in the discussion.

In the ndarray example, the ranges are used as macro arguments and used as-is in the macro body, so they don't strictly come from an expansion and the macro checks don't work. I don't know of an alternative approach.

I'm thinking about constraining this lint to contexts where the range literal is directly used in a method call to an iterator method (besides slice indexing and for loop iteration). I think that is relatively simple to do, but would left out cases like the following:

let r = (10..0);
r.for_each(...);

I believe trying to follow the local is error prone, especially if we don't want to use MIR.

Should I go for constraining the lint, or are there other solutions that I'm not aware?