Skip to content

Commit

Permalink
Auto merge of rust-lang#11859 - y21:issue11856, r=blyxyas
Browse files Browse the repository at this point in the history
[`missing_asserts_for_indexing`]: work with bodies instead of blocks separately

Fixes rust-lang#11856

Before this change, this lint would check blocks independently of each other, which means that it misses `assert!()`s from parent blocks.
```rs
// check_block
assert!(x.len() > 1);

{
  // check_block
  // no assert here
  let _ = x[0] + x[1];
}
```

This PR changes it to work with bodies rather than individual blocks. That means that a function will be checked in one go and we can remember if an `assert!` occurred anywhere.

Eventually it would be nice to have a more control flow-aware analysis, possibly by rewriting it as a MIR lint, but that's more complicated and I wanted this fixed first.

changelog: [`missing_asserts_for_indexing`]: accept `assert!`s from parent blocks
  • Loading branch information
bors committed Nov 24, 2023
2 parents c3243f9 + 553857b commit 96eab06
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 4 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/missing_asserts_for_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use clippy_utils::{eq_expr_value, hash_expr, higher};
use rustc_ast::{LitKind, RangeLimits};
use rustc_data_structures::unhash::UnhashMap;
use rustc_errors::{Applicability, Diagnostic};
use rustc_hir::{BinOp, Block, Expr, ExprKind, UnOp};
use rustc_hir::{BinOp, Block, Body, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Spanned;
Expand Down Expand Up @@ -390,10 +390,10 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
}

impl LateLintPass<'_> for MissingAssertsForIndexing {
fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
let mut map = UnhashMap::default();

for_each_expr(block, |expr| {
for_each_expr(body.value, |expr| {
check_index(cx, expr, &mut map);
check_assert(cx, expr, &mut map);
ControlFlow::<!, ()>::Continue(())
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/missing_asserts_for_indexing_unfixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,26 @@ fn index_struct_different_fields(f: &Foo<'_>) {
let _ = f.v[0] + f.v2[1];
}

fn shadowing() {
let x: &[i32] = &[1];
assert!(x.len() > 1);

let x: &[i32] = &[1];
let _ = x[0] + x[1];
//~^ ERROR: indexing into a slice multiple times without an `assert`
}

pub fn issue11856(values: &[i32]) -> usize {
let mut ascending = Vec::new();
for w in values.windows(2) {
assert!(w.len() > 1);
if w[0] < w[1] {
ascending.push((w[0], w[1]));
} else {
ascending.push((w[1], w[0]));
}
}
ascending.len()
}

fn main() {}
21 changes: 20 additions & 1 deletion tests/ui/missing_asserts_for_indexing_unfixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -160,5 +160,24 @@ LL | let _ = f.v[0] + f.v[1];
| ^^^^^^
= note: asserting the length before indexing will elide bounds checks

error: aborting due to 7 previous errors
error: indexing into a slice multiple times without an `assert`
--> $DIR/missing_asserts_for_indexing_unfixable.rs:54:13
|
LL | let _ = x[0] + x[1];
| ^^^^^^^^^^^
|
= help: consider asserting the length before indexing: `assert!(x.len() > 1);`
note: slice indexed here
--> $DIR/missing_asserts_for_indexing_unfixable.rs:54:13
|
LL | let _ = x[0] + x[1];
| ^^^^
note: slice indexed here
--> $DIR/missing_asserts_for_indexing_unfixable.rs:54:20
|
LL | let _ = x[0] + x[1];
| ^^^^
= note: asserting the length before indexing will elide bounds checks

error: aborting due to 8 previous errors

0 comments on commit 96eab06

Please sign in to comment.