fix workaround false positive clippy lint reversed_empty_ranges - #1279
fix workaround false positive clippy lint reversed_empty_ranges#1279ModProg wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
Can we hold off on removing the deprecated zip until it can be replaced by std::iter::zip? That's after all the whole point of it, to use the function version :slightly_smiling_face: (Function zip is the best 🙂 ) |
Sorry, something went wrong.
|
Yeah no problem, only did so because the ci was failing because of it. |
Sorry, something went wrong.
| ] | ||
| { | ||
| #[allow(clippy::reversed_empty_ranges)] | ||
| let slice = $crate::s![@parse |
There was a problem hiding this comment.
Did you try using #![allow(clippy::reversed_empty_ranges)] to allow this for the block and thereby avoiding the temporary binding? (Tail expressions do influence lifetime extension of temporaries and hence I would like to avoid changing this if it can be avoided.)
Sorry, something went wrong.
There was a problem hiding this comment.
attributes on expressions are experimental
Wouldn't work, as it needs a statement to apply the attribute
Sorry, something went wrong.
There was a problem hiding this comment.
Note the exclamation mark. This is similar to the form used at the beginning of a module to apply a directive to the whole module. So this would apply to everything inside the block, which in this case is just a single expression.
Sorry, something went wrong.
There was a problem hiding this comment.
($($t:tt)*) => {
{
#![allow(clippy::reversed_empty_ranges)]
$crate::s![@parse
::core::marker::PhantomData::<$crate::Ix0>,
::core::marker::PhantomData::<$crate::Ix0>,
[]
$($t)*
]
}
};error[E0658]: attributes on expressions are experimental
--> /mnt/data-ssd/modprog/Development/Rust/ndarray/src/slice.rs:871:13
|
775 | macro_rules! s(
| -------------- in this expansion of `s!`
...
871 | #![allow(clippy::reversed_empty_ranges)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
::: tests/array.rs:202:16
|
202 | let info = s![1.., 1, NewAxis, ..;2];
| ------------------------- in this macro invocation
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
Sorry, something went wrong.
There was a problem hiding this comment.
Ok, so this did work. Sorry for that.
Could you give the diff
diff --git a/src/slice.rs b/src/slice.rs
index 0146d6db..53763ae2 100644
--- a/src/slice.rs
+++ b/src/slice.rs
@@ -779,7 +779,7 @@ macro_rules! s(
r => {
let in_dim = $crate::SliceNextDim::next_in_dim(&r, $in_dim);
let out_dim = $crate::SliceNextDim::next_out_dim(&r, $out_dim);
- #[allow(unsafe_code)]
+ #[allow(unsafe_code, clippy::reversed_empty_ranges)]
unsafe {
$crate::SliceInfo::new_unchecked(
[$($stack)* $crate::s!(@convert r, $s)],
@@ -796,7 +796,7 @@ macro_rules! s(
r => {
let in_dim = $crate::SliceNextDim::next_in_dim(&r, $in_dim);
let out_dim = $crate::SliceNextDim::next_out_dim(&r, $out_dim);
- #[allow(unsafe_code)]
+ #[allow(unsafe_code, clippy::reversed_empty_ranges)]
unsafe {
$crate::SliceInfo::new_unchecked(
[$($stack)* $crate::s!(@convert r)],a try? It does work for me without forcing the slice info into a local binding.
If it does, could use this approach and rebase this on the current master branch?
Sorry, something went wrong.
There was a problem hiding this comment.
That didn't work, but I found a different solution, using two blocks to convince rust to allow an attribute on an expression.
Sorry, something went wrong.
|
This lint is no longer triggered after #1196 was merged. |
Sorry, something went wrong.
As the rust-clippy issue rust-lang/rust-clippy#5808 doesn't seam to make much progress I propose to just allow the lint for s![] macro invocations for now.
The let = ... is necessary, because attributes are not allowed on expressions
currently (rust-lang/rust#15701).