From 7da0d30c434eee0ec1bb46197d461876963ad25b Mon Sep 17 00:00:00 2001 From: Lachlan Deakin Date: Thu, 28 Nov 2024 17:33:44 +1100 Subject: [PATCH] Fix `unsafe_op_in_unsafe_fn` lint - Enable `unsafe_op_in_unsafe_fn` for all crates in workspace - Fix some non-exhaustive safety invariants - Start adding safety docs to internal unsafe - Apply some other fmt/clippy inconsistencies noticed in rust 2024 --- CHANGELOG.md | 3 + Cargo.toml | 1 + zarrs/src/array.rs | 33 +++++----- zarrs/src/array/array_async_readable.rs | 66 +++++++++++-------- zarrs/src/array/array_sync_readable.rs | 51 +++++++++----- zarrs/src/array/chunk_grid.rs | 30 ++++----- zarrs/src/array/chunk_grid/rectangular.rs | 24 ++++--- .../codec/array_to_bytes/bytes/bytes_codec.rs | 2 +- .../bytes/bytes_partial_decoder.rs | 4 +- .../array/codec/array_to_bytes/codec_chain.rs | 36 +++++----- .../pcodec/pcodec_partial_decoder.rs | 2 +- .../array_to_bytes/sharding/sharding_codec.rs | 33 ++++++---- zarrs/src/array_subset.rs | 26 +++++--- .../iterators/contiguous_indices_iterator.rs | 7 +- .../contiguous_linearised_indices_iterator.rs | 7 +- zarrs/src/lib.rs | 4 +- zarrs/src/node.rs | 4 +- zarrs_storage/CHANGELOG.md | 3 + zarrs_storage/Cargo.toml | 2 +- zarrs_storage/src/byte_range.rs | 14 ++-- 20 files changed, 210 insertions(+), 142 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88547b34..d7b1df2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Bump `zfp-sys` to 0.3.0 +### Fixed +- Fix `unsafe_op_in_unsafe_fn` in lint + ## [0.18.0] - 2024-11-23 ### Announcements diff --git a/Cargo.toml b/Cargo.toml index 25789eb5..4563b4ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ unused_variables = "warn" dead_code = "warn" missing_docs = "warn" unreachable_pub = "warn" +unsafe_op_in_unsafe_fn = "warn" [workspace.lints.clippy] pedantic = { level = "warn", priority = -1 } diff --git a/zarrs/src/array.rs b/zarrs/src/array.rs index c45ce191..b63679d7 100644 --- a/zarrs/src/array.rs +++ b/zarrs/src/array.rs @@ -785,22 +785,23 @@ impl Array { /// # Errors /// Returns a [`ArrayMetadataV2ToV3ConversionError`] if the metadata is not compatible with Zarr V3 metadata. pub fn to_v3(self) -> Result { - if let ArrayMetadata::V2(metadata) = self.metadata { - let metadata: ArrayMetadata = array_metadata_v2_to_v3(&metadata)?.into(); - Ok(Self { - storage: self.storage, - path: self.path, - data_type: self.data_type, - chunk_grid: self.chunk_grid, - chunk_key_encoding: self.chunk_key_encoding, - fill_value: self.fill_value, - codecs: self.codecs, - storage_transformers: self.storage_transformers, - dimension_names: self.dimension_names, - metadata, - }) - } else { - Ok(self) + match self.metadata { + ArrayMetadata::V2(metadata) => { + let metadata: ArrayMetadata = array_metadata_v2_to_v3(&metadata)?.into(); + Ok(Self { + storage: self.storage, + path: self.path, + data_type: self.data_type, + chunk_grid: self.chunk_grid, + chunk_key_encoding: self.chunk_key_encoding, + fill_value: self.fill_value, + codecs: self.codecs, + storage_transformers: self.storage_transformers, + dimension_names: self.dimension_names, + metadata, + }) + } + ArrayMetadata::V3(_) => Ok(self), } } } diff --git a/zarrs/src/array/array_async_readable.rs b/zarrs/src/array/array_async_readable.rs index f9f7b6c3..79ca0f72 100644 --- a/zarrs/src/array/array_async_readable.rs +++ b/zarrs/src/array/array_async_readable.rs @@ -360,25 +360,29 @@ impl Array { if let Some(chunk_encoded) = chunk_encoded { let chunk_encoded: Vec = chunk_encoded.into(); let chunk_representation = self.chunk_array_representation(chunk_indices)?; - self.codecs() - .decode_into( - Cow::Owned(chunk_encoded), - &chunk_representation, + unsafe { + self.codecs() + .decode_into( + Cow::Owned(chunk_encoded), + &chunk_representation, + output, + output_shape, + output_subset, + options, + ) + .map_err(ArrayError::CodecError) + } + } else { + unsafe { + copy_fill_value_into( + self.data_type(), + self.fill_value(), output, output_shape, output_subset, - options, ) .map_err(ArrayError::CodecError) - } else { - copy_fill_value_into( - self.data_type(), - self.fill_value(), - output, - output_shape, - output_subset, - ) - .map_err(ArrayError::CodecError) + } } } @@ -790,14 +794,16 @@ impl Array { && chunk_subset.shape() == chunk_representation.shape_u64() { // Fast path if `chunk_subset` encompasses the whole chunk - self.async_retrieve_chunk_into( - chunk_indices, - output, - output_shape, - output_subset, - options, - ) - .await + unsafe { + self.async_retrieve_chunk_into( + chunk_indices, + output, + output_shape, + output_subset, + options, + ) + .await + } } else { let storage_handle = Arc::new(StorageHandle::new(self.storage.clone())); let storage_transformer = self @@ -809,13 +815,15 @@ impl Array { self.chunk_key(chunk_indices), )); - Ok(self - .codecs - .clone() - .async_partial_decoder(input_handle, &chunk_representation, options) - .await? - .partial_decode_into(chunk_subset, output, output_shape, output_subset, options) - .await?) + unsafe { + self.codecs + .clone() + .async_partial_decoder(input_handle, &chunk_representation, options) + .await? + .partial_decode_into(chunk_subset, output, output_shape, output_subset, options) + .await?; + } + Ok(()) } } diff --git a/zarrs/src/array/array_sync_readable.rs b/zarrs/src/array/array_sync_readable.rs index 1ab6012c..200e0ba6 100644 --- a/zarrs/src/array/array_sync_readable.rs +++ b/zarrs/src/array/array_sync_readable.rs @@ -481,8 +481,8 @@ impl Array { if let Some(chunk_encoded) = chunk_encoded { let chunk_encoded: Vec = chunk_encoded.into(); let chunk_representation = self.chunk_array_representation(chunk_indices)?; - self.codecs() - .decode_into( + unsafe { + self.codecs().decode_into( Cow::Owned(chunk_encoded), &chunk_representation, output, @@ -490,15 +490,18 @@ impl Array { output_subset, options, ) - .map_err(ArrayError::CodecError) + } + .map_err(ArrayError::CodecError) } else { - copy_fill_value_into( - self.data_type(), - self.fill_value(), - output, - output_shape, - output_subset, - ) + unsafe { + copy_fill_value_into( + self.data_type(), + self.fill_value(), + output, + output_shape, + output_subset, + ) + } .map_err(ArrayError::CodecError) } } @@ -845,7 +848,15 @@ impl Array { && chunk_subset.shape() == chunk_representation.shape_u64() { // Fast path if `chunk_subset` encompasses the whole chunk - self.retrieve_chunk_into(chunk_indices, output, output_shape, output_subset, options) + unsafe { + self.retrieve_chunk_into( + chunk_indices, + output, + output_shape, + output_subset, + options, + ) + } } else { let storage_handle = Arc::new(StorageHandle::new(self.storage.clone())); let storage_transformer = self @@ -856,11 +867,19 @@ impl Array { self.chunk_key(chunk_indices), )); - Ok(self - .codecs - .clone() - .partial_decoder(input_handle, &chunk_representation, options)? - .partial_decode_into(chunk_subset, output, output_shape, output_subset, options)?) + unsafe { + self.codecs + .clone() + .partial_decoder(input_handle, &chunk_representation, options)? + .partial_decode_into( + chunk_subset, + output, + output_shape, + output_subset, + options, + )?; + } + Ok(()) } } diff --git a/zarrs/src/array/chunk_grid.rs b/zarrs/src/array/chunk_grid.rs index 8a83fc9d..79ff9ee7 100644 --- a/zarrs/src/array/chunk_grid.rs +++ b/zarrs/src/array/chunk_grid.rs @@ -372,7 +372,7 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { chunk_indices.len() == self.dimensionality() && array_shape.len() == self.dimensionality() && self.grid_shape(array_shape).is_ok_and(|chunk_grid_shape| { - chunk_grid_shape.map_or(false, |chunk_grid_shape| { + chunk_grid_shape.is_some_and(|chunk_grid_shape| { std::iter::zip(chunk_indices, chunk_grid_shape) .all(|(index, shape)| shape == 0 || *index < shape) }) @@ -388,7 +388,7 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { /// See [`ChunkGridTraits::chunk_origin`]. /// /// # Safety - /// The length of `chunk_indices` must match the dimensionality of the chunk grid. + /// The length of `chunk_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn chunk_origin_unchecked( &self, chunk_indices: &[u64], @@ -398,7 +398,7 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { /// See [`ChunkGridTraits::chunk_shape`]. /// /// # Safety - /// The length of `chunk_indices` must match the dimensionality of the chunk grid. + /// The length of `chunk_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn chunk_shape_unchecked( &self, chunk_indices: &[u64], @@ -408,7 +408,7 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { /// See [`ChunkGridTraits::chunk_shape_u64`]. /// /// # Safety - /// The length of `chunk_indices` must match the dimensionality of the chunk grid. + /// The length of `chunk_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn chunk_shape_u64_unchecked( &self, chunk_indices: &[u64], @@ -418,7 +418,7 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { /// See [`ChunkGridTraits::chunk_indices`]. /// /// # Safety - /// The length of `array_indices` must match the dimensionality of the chunk grid. + /// The length of `array_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn chunk_indices_unchecked( &self, array_indices: &[u64], @@ -428,7 +428,7 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { /// See [`ChunkGridTraits::chunk_element_indices`]. /// /// # Safety - /// The length of `array_indices` must match the dimensionality of the chunk grid. + /// The length of `array_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn chunk_element_indices_unchecked( &self, array_indices: &[u64], @@ -438,21 +438,21 @@ pub trait ChunkGridTraits: core::fmt::Debug + Send + Sync { /// See [`ChunkGridTraits::subset`]. /// /// # Safety - /// The length of `chunk_indices` must match the dimensionality of the chunk grid. + /// The length of `chunk_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn subset_unchecked( &self, chunk_indices: &[u64], array_shape: &[u64], ) -> Option { debug_assert_eq!(self.dimensionality(), chunk_indices.len()); - if let (Some(chunk_origin), Some(chunk_shape)) = ( - self.chunk_origin_unchecked(chunk_indices, array_shape), - self.chunk_shape_u64_unchecked(chunk_indices, array_shape), - ) { - Some(ArraySubset::new_with_start_shape_unchecked( - chunk_origin, - chunk_shape, - )) + let chunk_origin = + // SAFETY: The length of `chunk_indices` and `array_shape` matches the dimensionality of the chunk grid + unsafe { self.chunk_origin_unchecked(chunk_indices, array_shape) }; + let chunk_shape = + // SAFETY: The length of `chunk_indices` and `array_shape` matches the dimensionality of the chunk grid + unsafe { self.chunk_shape_u64_unchecked(chunk_indices, array_shape) }; + if let (Some(chunk_origin), Some(chunk_shape)) = (chunk_origin, chunk_shape) { + Some(unsafe { ArraySubset::new_with_start_shape_unchecked(chunk_origin, chunk_shape) }) } else { None } diff --git a/zarrs/src/array/chunk_grid/rectangular.rs b/zarrs/src/array/chunk_grid/rectangular.rs index 958ee043..3cef4628 100644 --- a/zarrs/src/array/chunk_grid/rectangular.rs +++ b/zarrs/src/array/chunk_grid/rectangular.rs @@ -130,7 +130,8 @@ impl ChunkGridTraits for RectangularChunkGrid { RectangularChunkGridDimension::Varying(s) => { let last_default = OffsetSize { offset: 0, - size: NonZeroU64::new_unchecked(1), + // SAFETY: 1 is non-zero + size: unsafe { NonZeroU64::new_unchecked(1) }, }; let last = s.last().unwrap_or(&last_default); if *array_shape == last.offset + last.size.get() { @@ -222,7 +223,8 @@ impl ChunkGridTraits for RectangularChunkGrid { RectangularChunkGridDimension::Varying(offsets_sizes) => { let last_default = OffsetSize { offset: 0, - size: NonZeroU64::new_unchecked(1), + // SAFETY: 1 is non-zero + size: unsafe { NonZeroU64::new_unchecked(1) }, }; let last = offsets_sizes.last().unwrap_or(&last_default); if *index < last.offset + last.size.get() { @@ -242,19 +244,21 @@ impl ChunkGridTraits for RectangularChunkGrid { .collect() } + /// # Safety + /// The length of `array_indices` and `array_shape` must match the dimensionality of the chunk grid. unsafe fn chunk_element_indices_unchecked( &self, array_indices: &[u64], array_shape: &[u64], ) -> Option { - let chunk_indices = self.chunk_indices_unchecked(array_indices, array_shape); + let chunk_indices = unsafe { self.chunk_indices_unchecked(array_indices, array_shape) }; chunk_indices.and_then(|chunk_indices| { - self.chunk_origin_unchecked(&chunk_indices, array_shape) - .map(|chunk_start| { - std::iter::zip(array_indices, &chunk_start) - .map(|(i, s)| i - s) - .collect() - }) + // SAFETY: The length of chunk_indices matches the dimensionality of the chunk grid + unsafe { self.chunk_origin_unchecked(&chunk_indices, array_shape) }.map(|chunk_start| { + std::iter::zip(array_indices, &chunk_start) + .map(|(i, s)| i - s) + .collect() + }) }) } @@ -268,7 +272,7 @@ impl ChunkGridTraits for RectangularChunkGrid { RectangularChunkGridDimension::Fixed(_) => true, RectangularChunkGridDimension::Varying(offsets_sizes) => offsets_sizes .last() - .map_or(false, |last| *array_index < last.offset + last.size.get()), + .is_some_and(|last| *array_index < last.offset + last.size.get()), } }, ) diff --git a/zarrs/src/array/codec/array_to_bytes/bytes/bytes_codec.rs b/zarrs/src/array/codec/array_to_bytes/bytes/bytes_codec.rs index e6c827e8..f6cff4a3 100644 --- a/zarrs/src/array/codec/array_to_bytes/bytes/bytes_codec.rs +++ b/zarrs/src/array/codec/array_to_bytes/bytes/bytes_codec.rs @@ -74,7 +74,7 @@ impl BytesCodec { return Err(CodecError::UnsupportedDataType( decoded_representation.data_type().clone(), super::IDENTIFIER.to_string(), - )) + )); } DataTypeSize::Fixed(data_type_size) => { let array_size = decoded_representation.num_elements() * data_type_size as u64; diff --git a/zarrs/src/array/codec/array_to_bytes/bytes/bytes_partial_decoder.rs b/zarrs/src/array/codec/array_to_bytes/bytes/bytes_partial_decoder.rs index 85f5e989..9a71646e 100644 --- a/zarrs/src/array/codec/array_to_bytes/bytes/bytes_partial_decoder.rs +++ b/zarrs/src/array/codec/array_to_bytes/bytes/bytes_partial_decoder.rs @@ -56,7 +56,7 @@ impl ArrayPartialDecoderTraits for BytesPartialDecoder<'_> { return Err(CodecError::UnsupportedDataType( self.data_type().clone(), super::IDENTIFIER.to_string(), - )) + )); } DataTypeSize::Fixed(data_type_size) => { // Get byte ranges @@ -166,7 +166,7 @@ impl AsyncArrayPartialDecoderTraits for AsyncBytesPartialDecoder { return Err(CodecError::UnsupportedDataType( self.data_type().clone(), super::IDENTIFIER.to_string(), - )) + )); } DataTypeSize::Fixed(data_type_size) => array_subset .byte_ranges(&chunk_shape, data_type_size) diff --git a/zarrs/src/array/codec/array_to_bytes/codec_chain.rs b/zarrs/src/array/codec/array_to_bytes/codec_chain.rs index c1b605a9..3a0678b8 100644 --- a/zarrs/src/array/codec/array_to_bytes/codec_chain.rs +++ b/zarrs/src/array/codec/array_to_bytes/codec_chain.rs @@ -325,14 +325,16 @@ impl ArrayToBytesCodecTraits for CodecChain { if self.bytes_to_bytes.is_empty() && self.array_to_array.is_empty() { // Fast path if no bytes to bytes or array to array codecs - return self.array_to_bytes.decode_into( - bytes, - array_representations.last().unwrap(), - output, - output_shape, - output_subset, - options, - ); + return unsafe { + self.array_to_bytes.decode_into( + bytes, + array_representations.last().unwrap(), + output, + output_shape, + output_subset, + options, + ) + }; } // bytes->bytes @@ -345,14 +347,16 @@ impl ArrayToBytesCodecTraits for CodecChain { if self.array_to_array.is_empty() { // Fast path if no array to array codecs - return self.array_to_bytes.decode_into( - bytes, - array_representations.last().unwrap(), - output, - output_shape, - output_subset, - options, - ); + return unsafe { + self.array_to_bytes.decode_into( + bytes, + array_representations.last().unwrap(), + output, + output_shape, + output_subset, + options, + ) + }; } // bytes->array diff --git a/zarrs/src/array/codec/array_to_bytes/pcodec/pcodec_partial_decoder.rs b/zarrs/src/array/codec/array_to_bytes/pcodec/pcodec_partial_decoder.rs index 2d0fc84d..21a34b4b 100644 --- a/zarrs/src/array/codec/array_to_bytes/pcodec/pcodec_partial_decoder.rs +++ b/zarrs/src/array/codec/array_to_bytes/pcodec/pcodec_partial_decoder.rs @@ -93,7 +93,7 @@ fn do_partial_decode<'a>( return Err(CodecError::UnsupportedDataType( data_type.clone(), super::IDENTIFIER.to_string(), - )) + )); } }; } diff --git a/zarrs/src/array/codec/array_to_bytes/sharding/sharding_codec.rs b/zarrs/src/array/codec/array_to_bytes/sharding/sharding_codec.rs index 85b6a66e..3411ca40 100644 --- a/zarrs/src/array/codec/array_to_bytes/sharding/sharding_codec.rs +++ b/zarrs/src/array/codec/array_to_bytes/sharding/sharding_codec.rs @@ -445,9 +445,11 @@ impl ArrayToBytesCodecTraits for ShardingCodec { ); let shard_offset = usize::try_from(index * data_type_size as u64).unwrap(); - output - .index_mut(shard_offset..shard_offset + fv.len()) - .copy_from_slice(fv); + unsafe { + output + .index_mut(shard_offset..shard_offset + fv.len()) + .copy_from_slice(fv); + } } } else { unreachable!(); @@ -461,14 +463,16 @@ impl ArrayToBytesCodecTraits for ShardingCodec { let offset: usize = offset.try_into().unwrap(); let size: usize = size.try_into().unwrap(); let encoded_chunk = &encoded_shard[offset..offset + size]; - self.inner_codecs.decode_into( - Cow::Borrowed(encoded_chunk), - &chunk_representation, - output, - output_shape, - &output_subset_chunk, - &options, - )?; + unsafe { + self.inner_codecs.decode_into( + Cow::Borrowed(encoded_chunk), + &chunk_representation, + output, + output_shape, + &output_subset_chunk, + &options, + )?; + } }; Ok::<_, CodecError>(()) @@ -738,7 +742,7 @@ impl ShardingCodec { &options, )?; { - let shard_slice = unsafe { crate::vec_spare_capacity_to_mut_slice(&mut shard) }; + let shard_slice = crate::vec_spare_capacity_to_mut_slice(&mut shard); match self.index_location { ShardingIndexLocation::Start => { shard_slice[..encoded_array_index.len()].copy_from_slice(&encoded_array_index); @@ -749,7 +753,7 @@ impl ShardingCodec { } } } - + // SAFETY: all elements have been initialised unsafe { shard.set_len(shard_length) }; shard.shrink_to_fit(); Ok(shard) @@ -884,7 +888,7 @@ impl ShardingCodec { options, )?; { - let shard_slice = unsafe { crate::vec_spare_capacity_to_mut_slice(&mut shard) }; + let shard_slice = crate::vec_spare_capacity_to_mut_slice(&mut shard); match self.index_location { ShardingIndexLocation::Start => { shard_slice[..encoded_array_index.len()].copy_from_slice(&encoded_array_index); @@ -895,6 +899,7 @@ impl ShardingCodec { } } } + // SAFETY: all elements have been initialised unsafe { shard.set_len(shard_length) }; Ok(shard) } diff --git a/zarrs/src/array_subset.rs b/zarrs/src/array_subset.rs index f0b8c1f2..ac7b8cde 100644 --- a/zarrs/src/array_subset.rs +++ b/zarrs/src/array_subset.rs @@ -316,7 +316,9 @@ impl ArraySubset { element_size: usize, ) -> Vec { let mut byte_ranges: Vec = Vec::new(); - let contiguous_indices = self.contiguous_linearised_indices_unchecked(array_shape); + // SAFETY: The length of array_shape matches the dimensionality + let contiguous_indices = + unsafe { self.contiguous_linearised_indices_unchecked(array_shape) }; let byte_length = contiguous_indices.contiguous_elements_usize() * element_size; for array_index in &contiguous_indices { let byte_index = array_index * element_size as u64; @@ -373,7 +375,9 @@ impl ArraySubset { let mut elements_subset = Vec::with_capacity(num_elements); let elements_subset_slice = crate::vec_spare_capacity_to_mut_slice(&mut elements_subset); let mut subset_offset = 0; - let contiguous_elements = self.contiguous_linearised_indices_unchecked(array_shape); + // SAFETY: `array_shape` is encapsulated by an array with `array_shape`. + let contiguous_elements = + unsafe { self.contiguous_linearised_indices_unchecked(array_shape) }; let element_length = contiguous_elements.contiguous_elements_usize(); for array_index in &contiguous_elements { let element_offset = usize::try_from(array_index).unwrap(); @@ -408,10 +412,11 @@ impl ArraySubset { /// Returns an iterator over the indices of elements within the subset. /// /// # Safety - /// `array_shape` must match the dimensionality and encapsulate this array subset. + /// `array_shape` must encapsulate this array subset. #[must_use] pub unsafe fn linearised_indices_unchecked(&self, array_shape: &[u64]) -> LinearisedIndices { - LinearisedIndices::new_unchecked(self.clone(), array_shape.to_vec()) + // SAFETY: array_shape encapsulated this array subset + unsafe { LinearisedIndices::new_unchecked(self.clone(), array_shape.to_vec()) } } /// Returns an iterator over the indices of contiguous elements within the subset. @@ -429,10 +434,11 @@ impl ArraySubset { /// Returns an iterator over the indices of contiguous elements within the subset. /// /// # Safety - /// The length of `array_shape` must match the array subset dimensionality. + /// `array_shape` must encapsulate this array subset. #[must_use] pub unsafe fn contiguous_indices_unchecked(&self, array_shape: &[u64]) -> ContiguousIndices { - ContiguousIndices::new_unchecked(self, array_shape) + // SAFETY: array_shape encapsulated this array subset + unsafe { ContiguousIndices::new_unchecked(self, array_shape) } } /// Returns an iterator over the linearised indices of contiguous elements within the subset. @@ -450,13 +456,14 @@ impl ArraySubset { /// Returns an iterator over the linearised indices of contiguous elements within the subset. /// /// # Safety - /// The length of `array_shape` must match the array subset dimensionality. + /// `array_shape` must encapsulate this array subset. #[must_use] pub unsafe fn contiguous_linearised_indices_unchecked( &self, array_shape: &[u64], ) -> ContiguousLinearisedIndices { - ContiguousLinearisedIndices::new_unchecked(self, array_shape.to_vec()) + // SAFETY: array_shape encapsulated this array subset + unsafe { ContiguousLinearisedIndices::new_unchecked(self, array_shape.to_vec()) } } /// Returns the [`Chunks`] with `chunk_shape` in the array subset which can be iterated over. @@ -482,7 +489,8 @@ impl ArraySubset { /// The length of `chunk_shape` must match the array subset dimensionality. #[must_use] pub unsafe fn chunks_unchecked(&self, chunk_shape: &[NonZeroU64]) -> Chunks { - Chunks::new_unchecked(self, chunk_shape) + // SAFETY: the dimensionality of chunk_shape matches the dimensionality. + unsafe { Chunks::new_unchecked(self, chunk_shape) } } /// Return the overlapping subset between this array subset and `subset_other`. diff --git a/zarrs/src/array_subset/iterators/contiguous_indices_iterator.rs b/zarrs/src/array_subset/iterators/contiguous_indices_iterator.rs index 2cec7b6f..8f0e3d9c 100644 --- a/zarrs/src/array_subset/iterators/contiguous_indices_iterator.rs +++ b/zarrs/src/array_subset/iterators/contiguous_indices_iterator.rs @@ -84,9 +84,12 @@ impl ContiguousIndices { shape_out_i.write(subset_size); } } + // SAFETY: each element is initialised unsafe { shape_out.set_len(array_shape.len()) }; - let subset_contiguous_start = - ArraySubset::new_with_start_shape_unchecked(subset.start().to_vec(), shape_out); + // SAFETY: The length of shape_out matches the subset dimensionality + let subset_contiguous_start = unsafe { + ArraySubset::new_with_start_shape_unchecked(subset.start().to_vec(), shape_out) + }; // let inner = subset_contiguous_start.iter_indices(); Self { subset_contiguous_start, diff --git a/zarrs/src/array_subset/iterators/contiguous_linearised_indices_iterator.rs b/zarrs/src/array_subset/iterators/contiguous_linearised_indices_iterator.rs index c3ac9263..28f1af2a 100644 --- a/zarrs/src/array_subset/iterators/contiguous_linearised_indices_iterator.rs +++ b/zarrs/src/array_subset/iterators/contiguous_linearised_indices_iterator.rs @@ -53,8 +53,11 @@ impl ContiguousLinearisedIndices { /// `array_shape` must encapsulate `subset`. #[must_use] pub unsafe fn new_unchecked(subset: &ArraySubset, array_shape: Vec) -> Self { - let inner = subset.contiguous_indices_unchecked(&array_shape); - Self { inner, array_shape } + // SAFETY: The length of array_shape matches the array subset dimensionality + unsafe { + let inner = subset.contiguous_indices_unchecked(&array_shape); + Self { inner, array_shape } + } } /// Return the number of starting indices (i.e. the length of the iterator). diff --git a/zarrs/src/lib.rs b/zarrs/src/lib.rs index aa9822b8..ea9699dd 100644 --- a/zarrs/src/lib.rs +++ b/zarrs/src/lib.rs @@ -204,9 +204,9 @@ pub use zarrs_filesystem as filesystem; pub use storage::byte_range; /// Get a mutable slice of the spare capacity in a vector. -#[allow(dead_code)] -unsafe fn vec_spare_capacity_to_mut_slice(vec: &mut Vec) -> &mut [T] { +fn vec_spare_capacity_to_mut_slice(vec: &mut Vec) -> &mut [T] { let spare_capacity = vec.spare_capacity_mut(); + // SAFETY: `spare_capacity` is valid for both reads and writes for len * mem::size_of::() many bytes, and it is properly aligned unsafe { std::slice::from_raw_parts_mut( spare_capacity.as_mut_ptr().cast::(), diff --git a/zarrs/src/node.rs b/zarrs/src/node.rs index 4d052469..ad609233 100644 --- a/zarrs/src/node.rs +++ b/zarrs/src/node.rs @@ -92,7 +92,7 @@ impl Node { | NodeMetadata::Group(GroupMetadata::V3(_)) => return Ok(metadata), NodeMetadata::Array(ArrayMetadata::V2(_)) | NodeMetadata::Group(GroupMetadata::V2(_)) => { - return Err(NodeCreateError::MetadataVersionMismatch) + return Err(NodeCreateError::MetadataVersionMismatch); } } } @@ -154,7 +154,7 @@ impl Node { | NodeMetadata::Group(GroupMetadata::V3(_)) => return Ok(metadata), NodeMetadata::Array(ArrayMetadata::V2(_)) | NodeMetadata::Group(GroupMetadata::V2(_)) => { - return Err(NodeCreateError::MetadataVersionMismatch) + return Err(NodeCreateError::MetadataVersionMismatch); } } } diff --git a/zarrs_storage/CHANGELOG.md b/zarrs_storage/CHANGELOG.md index 830325a7..9d5232fb 100644 --- a/zarrs_storage/CHANGELOG.md +++ b/zarrs_storage/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Fix `unsafe_op_in_unsafe_fn` in lint + ## [0.3.0] - 2024-11-15 ### Added diff --git a/zarrs_storage/Cargo.toml b/zarrs_storage/Cargo.toml index 145111b0..4873e178 100644 --- a/zarrs_storage/Cargo.toml +++ b/zarrs_storage/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "zarrs_storage" -version = "0.3.0" +version = "0.3.1" authors = ["Lachlan Deakin "] edition = "2021" rust-version = "1.77" diff --git a/zarrs_storage/src/byte_range.rs b/zarrs_storage/src/byte_range.rs index 99165e72..7e1f86e3 100644 --- a/zarrs_storage/src/byte_range.rs +++ b/zarrs_storage/src/byte_range.rs @@ -268,12 +268,18 @@ pub unsafe fn extract_byte_ranges_concat_unchecked( for byte_range in byte_ranges { let start = usize::try_from(byte_range.start(bytes.len() as u64)).unwrap(); let byte_range_len = usize::try_from(byte_range.length(bytes.len() as u64)).unwrap(); - out_slice - .index_mut(offset..offset + byte_range_len) - .copy_from_slice(&bytes[start..start + byte_range_len]); + // SAFETY: the slices are non-overlapping + unsafe { + out_slice + .index_mut(offset..offset + byte_range_len) + .copy_from_slice(&bytes[start..start + byte_range_len]); + } offset += byte_range_len; } - out.set_len(out_size); + // SAFETY: each element is initialised + unsafe { + out.set_len(out_size); + } out }