Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rust 2021] migrate to rust 2021 #1159

Merged
merged 10 commits into from
Aug 3, 2021
4 changes: 3 additions & 1 deletion common/allocators/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
cargo-features = ["edition2021"]

[package]
name = "common-allocators"
version = "0.1.0"
authors = ["Datafuse Authors <opensource@datafuselabs.com>"]
license = "Apache-2.0"
publish = false
edition = "2018"
edition = "2021"

[features]
# Enable jemalloc for binaries
Expand Down
4 changes: 3 additions & 1 deletion common/arrow/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
cargo-features = ["edition2021"]

[package]
name = "common-arrow"
version = "0.1.0"
authors = ["Datafuse Authors <opensource@datafuselabs.com>"]
license = "Apache-2.0"
publish = false
edition = "2018"
edition = "2021"

[features]
simd = ["arrow/simd"]
Expand Down
4 changes: 3 additions & 1 deletion common/building/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
cargo-features = ["edition2021"]

[package]
name = "common-building"
version = "0.1.0"
authors = ["Datafuse Authors <opensource@datafuselabs.com>"]
license = "Apache-2.0"
publish = false
edition = "2018"
edition = "2021"

[dependencies]
vergen = "5.1.13"
Expand Down
4 changes: 3 additions & 1 deletion common/datablocks/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
cargo-features = ["edition2021"]

[package]
name = "common-datablocks"
version = "0.1.0"
authors = ["Datafuse Authors <opensource@datafuselabs.com>"]
license = "Apache-2.0"
publish = false
edition = "2018"
edition = "2021"

[dependencies] # In alphabetical order
# Workspace dependencies
Expand Down
5 changes: 4 additions & 1 deletion common/datablocks/src/kernels/data_block_sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ impl DataBlock {
.columns()
.iter()
.zip(rhs.columns().iter())
.map(|(a, b)| DataColumnCommon::merge_columns(a, b, indices))
.map(|(a, b)| {
let _ = &indices;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line used for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line used for?

see https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html

Auto gen by cargo fix. This is a conservative analysis: in many cases, these dummy lets can be safely removed and your program will work fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like a good style : (

Copy link
Member

@zhang2014 zhang2014 Jul 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can do it with macros? similar to '[&captures_var] (args) {} in C++`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can do it with macros? similar to '[&captures_var] (args) {} in C++`.

We can try to remove these dummy let manually. If it is really necessary, consider other options.

DataColumnCommon::merge_columns(a, b, indices)
})
.collect::<Result<Vec<_>>>()?;

Ok(DataBlock::create(lhs.schema().clone(), arrays))
Expand Down
1 change: 1 addition & 0 deletions common/datablocks/src/kernels/data_block_take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl DataBlock {
let columns = fields
.iter()
.map(|f| {
let _ = (&constant_columns, &indices);
let column = raw.try_column_by_name(f.name())?;
if constant_columns.contains(f.name()) {
let v = column.try_get(indices[0] as usize)?;
Expand Down
4 changes: 3 additions & 1 deletion common/datavalues/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
cargo-features = ["edition2021"]

[package]
name = "common-datavalues"
version = "0.1.0"
authors = ["Datafuse Authors <opensource@datafuselabs.com>"]
license = "Apache-2.0"
publish = false
edition = "2018"
edition = "2021"

[dependencies] # In alphabetical order
# Workspace dependencies
Expand Down
12 changes: 10 additions & 2 deletions common/datavalues/src/arrays/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,19 @@ impl Add<&str> for &DFUtf8Array {
Ok(match self.null_count() {
0 => self
.into_no_null_iter()
.map(|l| concat_strings(l, rhs))
.map(|l| {
let _ = &rhs;
concat_strings(l, rhs)
})
.collect(),
_ => self
.into_iter()
.map(|opt_l| opt_l.map(|l| concat_strings(l, rhs)))
.map(|opt_l| {
opt_l.map(|l| {
let _ = &rhs;
concat_strings(l, rhs)
})
})
.collect(),
})
}
Expand Down
1 change: 1 addition & 0 deletions common/datavalues/src/arrays/builders/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ where
builder.append_slice(values);
} else {
values.iter().enumerate().for_each(|(idx, v)| {
let _ = &builder;
if array.is_valid(idx) {
builder.append_value(*v);
} else {
Expand Down
71 changes: 53 additions & 18 deletions common/datavalues/src/arrays/kernels/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub unsafe fn take_no_null_primitive<T: DFNumericType>(
av.iter_mut()
.zip(index_values.iter())
.for_each(|(num, idx)| {
let _ = &array_values;
*num = *array_values.get_unchecked(*idx as usize);
});

Expand All @@ -62,6 +63,7 @@ pub unsafe fn take_no_null_primitive_iter_unchecked<
let mut av = AlignedVec::<T::Native>::with_capacity_len_aligned(data_len);

av.iter_mut().zip(indices_iter).for_each(|(num, idx)| {
let _ = &array_values;
*num = *array_values.get_unchecked(idx);
});
let arr = av.into_primitive_array::<T>(None);
Expand All @@ -79,7 +81,10 @@ pub fn take_no_null_primitive_iter<T: DFNumericType, I: IntoIterator<Item = usiz

let av = indices
.into_iter()
.map(|idx| array_values[idx])
.map(|idx| {
let _ = &array_values;
array_values[idx]
})
.collect::<AlignedVec<_>>();
let arr = av.into_primitive_array(None);

Expand All @@ -96,6 +101,7 @@ pub unsafe fn take_primitive_iter_unchecked<T: DFNumericType, I: IntoIterator<It
let array_values = arr.values();

let iter = indices.into_iter().map(|idx| {
let _ = (&arr, &array_values);
if arr.is_valid(idx) {
Some(*array_values.get_unchecked(idx))
} else {
Expand All @@ -117,6 +123,7 @@ pub fn take_primitive_iter<T: DFNumericType, I: IntoIterator<Item = usize>>(
let arr = indices
.into_iter()
.map(|idx| {
let _ = (&arr, &array_values);
if arr.is_valid(idx) {
Some(array_values[idx])
} else {
Expand All @@ -141,9 +148,12 @@ pub unsafe fn take_no_null_primitive_opt_iter_unchecked<
) -> Arc<PrimitiveArray<T>> {
let array_values = arr.values();

let iter = indices
.into_iter()
.map(|opt_idx| opt_idx.map(|idx| *array_values.get_unchecked(idx)));
let iter = indices.into_iter().map(|opt_idx| {
opt_idx.map(|idx| {
let _ = &array_values;
Copy link
Member

@sundy-li sundy-li Jul 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why rust 2021 turns to be more complicated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😭

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why rust 2021 turns to be more complicated.

I don't think it becomes more complicated.

This is a conservative analysis: in many cases, these dummy lets can be safely removed and your program will work fine.

Starting in Rust 2021, closures captures are more precise. Typically they will only capture the fields they use (in some cases, they might capture more than just what they use, see the Rust reference for full details).

*array_values.get_unchecked(idx)
})
});
let arr = PrimitiveArray::from_trusted_len_iter(iter);

Arc::new(arr)
Expand All @@ -164,6 +174,7 @@ pub unsafe fn take_primitive_opt_iter_unchecked<

let iter = indices.into_iter().map(|opt_idx| {
opt_idx.and_then(|idx| {
let _ = (&arr, &array_values);
if arr.is_valid(idx) {
Some(*array_values.get_unchecked(idx))
} else {
Expand Down Expand Up @@ -205,7 +216,10 @@ pub fn take_no_null_bool_iter<I: IntoIterator<Item = usize>>(
) -> Arc<BooleanArray> {
debug_assert_eq!(arr.null_count(), 0);

let iter = indices.into_iter().map(|idx| Some(arr.value(idx)));
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
Some(arr.value(idx))
});

Arc::new(iter.collect())
}
Expand All @@ -218,9 +232,10 @@ pub unsafe fn take_no_null_bool_iter_unchecked<I: IntoIterator<Item = usize>>(
indices: I,
) -> Arc<BooleanArray> {
debug_assert_eq!(arr.null_count(), 0);
let iter = indices
.into_iter()
.map(|idx| Some(arr.value_unchecked(idx)));
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
Some(arr.value_unchecked(idx))
});

Arc::new(iter.collect())
}
Expand All @@ -231,6 +246,7 @@ pub fn take_bool_iter<I: IntoIterator<Item = usize>>(
indices: I,
) -> Arc<BooleanArray> {
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
if arr.is_null(idx) {
None
} else {
Expand All @@ -249,6 +265,7 @@ pub unsafe fn take_bool_iter_unchecked<I: IntoIterator<Item = usize>>(
indices: I,
) -> Arc<BooleanArray> {
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
if arr.is_null(idx) {
None
} else {
Expand All @@ -268,6 +285,7 @@ pub unsafe fn take_bool_opt_iter_unchecked<I: IntoIterator<Item = Option<usize>>
) -> Arc<BooleanArray> {
let iter = indices.into_iter().map(|opt_idx| {
opt_idx.and_then(|idx| {
let _ = &arr;
if arr.is_null(idx) {
None
} else {
Expand All @@ -286,9 +304,12 @@ pub unsafe fn take_no_null_bool_opt_iter_unchecked<I: IntoIterator<Item = Option
arr: &BooleanArray,
indices: I,
) -> Arc<BooleanArray> {
let iter = indices
.into_iter()
.map(|opt_idx| opt_idx.map(|idx| arr.value_unchecked(idx)));
let iter = indices.into_iter().map(|opt_idx| {
opt_idx.map(|idx| {
let _ = &arr;
arr.value_unchecked(idx)
})
});

Arc::new(iter.collect())
}
Expand All @@ -299,9 +320,10 @@ pub unsafe fn take_no_null_utf8_iter_unchecked<I: IntoIterator<Item = usize>>(
arr: &StringArray,
indices: I,
) -> Arc<StringArray> {
let iter = indices
.into_iter()
.map(|idx| Some(arr.value_unchecked(idx)));
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
Some(arr.value_unchecked(idx))
});

Arc::new(iter.collect())
}
Expand All @@ -313,6 +335,7 @@ pub unsafe fn take_utf8_iter_unchecked<I: IntoIterator<Item = usize>>(
indices: I,
) -> Arc<StringArray> {
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
if arr.is_null(idx) {
None
} else {
Expand All @@ -329,9 +352,12 @@ pub unsafe fn take_no_null_utf8_opt_iter_unchecked<I: IntoIterator<Item = Option
arr: &StringArray,
indices: I,
) -> Arc<StringArray> {
let iter = indices
.into_iter()
.map(|opt_idx| opt_idx.map(|idx| arr.value_unchecked(idx)));
let iter = indices.into_iter().map(|opt_idx| {
opt_idx.map(|idx| {
let _ = &arr;
arr.value_unchecked(idx)
})
});

Arc::new(iter.collect())
}
Expand All @@ -344,6 +370,7 @@ pub unsafe fn take_utf8_opt_iter_unchecked<I: IntoIterator<Item = Option<usize>>
) -> Arc<StringArray> {
let iter = indices.into_iter().map(|opt_idx| {
opt_idx.and_then(|idx| {
let _ = &arr;
if arr.is_null(idx) {
None
} else {
Expand All @@ -359,7 +386,10 @@ pub fn take_no_null_utf8_iter<I: IntoIterator<Item = usize>>(
arr: &StringArray,
indices: I,
) -> Arc<StringArray> {
let iter = indices.into_iter().map(|idx| Some(arr.value(idx)));
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
Some(arr.value(idx))
});

Arc::new(iter.collect())
}
Expand All @@ -369,6 +399,7 @@ pub fn take_utf8_iter<I: IntoIterator<Item = usize>>(
indices: I,
) -> Arc<StringArray> {
let iter = indices.into_iter().map(|idx| {
let _ = &arr;
if arr.is_null(idx) {
None
} else {
Expand Down Expand Up @@ -413,6 +444,7 @@ pub unsafe fn take_utf8(arr: &StringArray, indices: &UInt32Array) -> Arc<StringA
.skip(1)
.enumerate()
.for_each(|(idx, offset)| {
let _ = (&indices, &arr);
let index = indices.value_unchecked(idx) as usize;
let s = arr.value_unchecked(index);
length_so_far += s.len() as i64;
Expand All @@ -432,6 +464,7 @@ pub unsafe fn take_utf8(arr: &StringArray, indices: &UInt32Array) -> Arc<StringA
.skip(1)
.enumerate()
.for_each(|(idx, offset)| {
let _ = (&indices, &arr);
if indices.is_valid(idx) {
let index = indices.value_unchecked(idx) as usize;
let s = arr.value_unchecked(index);
Expand All @@ -452,6 +485,7 @@ pub unsafe fn take_utf8(arr: &StringArray, indices: &UInt32Array) -> Arc<StringA

if indices.null_count() == 0 {
(0..data_len).for_each(|idx| {
let _ = (&indices, &arr);
let index = indices.value_unchecked(idx) as usize;
if arr.is_valid(index) {
let s = arr.value_unchecked(index);
Expand All @@ -462,6 +496,7 @@ pub unsafe fn take_utf8(arr: &StringArray, indices: &UInt32Array) -> Arc<StringA
});
} else {
(0..data_len).for_each(|idx| {
let _ = (&indices, &arr);
if indices.is_valid(idx) {
let index = indices.value_unchecked(idx) as usize;

Expand Down
10 changes: 8 additions & 2 deletions common/datavalues/src/arrays/ops/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ impl<'a> ArrayApply<'a, bool, bool> for DFBooleanArray {
{
self.apply_kernel_cast(|array| {
let av: AlignedVec<_> = (0..array.len())
.map(|idx| unsafe { f(array.value_unchecked(idx)) })
.map(|idx| {
let _ = &array;
unsafe { f(array.value_unchecked(idx)) }
})
.collect();
let null_bit_buffer = array.data_ref().null_buffer().cloned();
Arc::new(av.into_primitive_array::<S>(null_bit_buffer)) as ArrayRef
Expand Down Expand Up @@ -204,7 +207,10 @@ impl<'a> ArrayApply<'a, &'a str, Cow<'a, str>> for DFUtf8Array {
{
let arr = self.downcast_ref();
let av: AlignedVec<_> = (0..arr.len())
.map(|idx| unsafe { f(arr.value_unchecked(idx)) })
.map(|idx| {
let _ = &arr;
unsafe { f(arr.value_unchecked(idx)) }
})
.collect();

let null_bit_buffer = self.array.data_ref().null_buffer().cloned();
Expand Down
Loading