Skip to content

Commit

Permalink
Merge #687
Browse files Browse the repository at this point in the history
687: Address clippy warning r=taiki-e a=erickt

Clippy doesn't like this pattern:

```
for i in 0..batch_size {
  ...
  batch_size = i;
  ...
}
```

because it can trick readers into thinking the loop condition might be
modified during iteration. This avoids the warning by explicitly making
a new variable for the end condition of the loop.

Co-authored-by: Erick Tryzelaar <etryzelaar@google.com>
  • Loading branch information
bors[bot] and erickt committed Apr 28, 2021
2 parents bb64382 + 5a14c61 commit 7928d11
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions crossbeam-deque/src/deque.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// TODO(@jeehoonkang): we mutates `batch_size` inside `for i in 0..batch_size {}`. It is difficult
// to read because we're mutating the range bound.
#![allow(clippy::mut_range_bound)]

use std::cell::{Cell, UnsafeCell};
use std::cmp;
use std::fmt;
Expand Down Expand Up @@ -760,7 +756,12 @@ impl<T> Stealer<T> {

// Steal a batch of tasks from the front one by one.
Flavor::Lifo => {
for i in 0..batch_size {
// This loop may modify the batch_size, which triggers a clippy lint warning.
// Use a new variable to avoid the warning, and to make it clear we aren't
// modifying the loop exit condition during iteration.
let original_batch_size = batch_size;

for i in 0..original_batch_size {
// If this is not the first steal, check whether the queue is empty.
if i > 0 {
// We've already got the current front index. Now execute the fence to
Expand Down Expand Up @@ -965,7 +966,12 @@ impl<T> Stealer<T> {
f = f.wrapping_add(1);

// Repeat the same procedure for the batch steals.
for i in 0..batch_size {
//
// This loop may modify the batch_size, which triggers a clippy lint warning.
// Use a new variable to avoid the warning, and to make it clear we aren't
// modifying the loop exit condition during iteration.
let original_batch_size = batch_size;
for i in 0..original_batch_size {
// We've already got the current front index. Now execute the fence to
// synchronize with other threads.
atomic::fence(Ordering::SeqCst);
Expand Down

0 comments on commit 7928d11

Please sign in to comment.