From be13123a0fcf82d93911eaccdbcf41f86e3c0944 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 3 Oct 2023 01:29:05 +0200 Subject: [PATCH] fix: add more lints, fix miri --- Cargo.toml | 1 + .clippy.toml => clippy.toml | 0 src/aarch64.rs | 2 + src/error.rs | 4 +- src/lib.rs | 76 +++++++++++++++++++++++++++---------- src/portable_simd.rs | 17 ++++++--- src/serde.rs | 33 ++++++++-------- src/traits.rs | 13 +++++++ src/x86.rs | 2 + tests/test.rs | 27 +++++++++---- 10 files changed, 124 insertions(+), 51 deletions(-) rename .clippy.toml => clippy.toml (100%) diff --git a/Cargo.toml b/Cargo.toml index f0e80e1..df13d25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ keywords = ["hex", "bytes", "fmt"] documentation = "https://docs.rs/const-hex" homepage = "https://github.com/danipopes/const-hex" repository = "https://github.com/danipopes/const-hex" +exclude = [".github/", "benches/", "fuzz/", "tests/"] [package.metadata.docs.rs] all-features = true diff --git a/.clippy.toml b/clippy.toml similarity index 100% rename from .clippy.toml rename to clippy.toml diff --git a/src/aarch64.rs b/src/aarch64.rs index 1714cb8..75d5bca 100644 --- a/src/aarch64.rs +++ b/src/aarch64.rs @@ -1,3 +1,5 @@ +#![allow(unsafe_op_in_unsafe_fn)] + use crate::generic; use core::arch::aarch64::*; diff --git a/src/error.rs b/src/error.rs index 547e7ac..fb04a32 100644 --- a/src/error.rs +++ b/src/error.rs @@ -8,6 +8,7 @@ use core::fmt; pub enum FromHexError { /// An invalid character was found. Valid ones are: `0...9`, `a...f` /// or `A...F`. + #[allow(missing_docs)] InvalidHexCharacter { c: char, index: usize }, /// A hex string's length needs to be even, as two digits correspond to @@ -24,7 +25,8 @@ pub enum FromHexError { impl std::error::Error for FromHexError {} impl fmt::Display for FromHexError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { FromHexError::InvalidHexCharacter { c, index } => { write!(f, "Invalid character {c:?} at position {index}") diff --git a/src/lib.rs b/src/lib.rs index 54b347b..af7e61a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,9 +18,23 @@ #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] #![cfg_attr(feature = "nightly", feature(core_intrinsics, inline_const))] #![cfg_attr(feature = "portable-simd", feature(portable_simd))] +#![warn( + missing_copy_implementations, + missing_debug_implementations, + missing_docs, + unreachable_pub, + unsafe_op_in_unsafe_fn, + clippy::missing_const_for_fn, + clippy::missing_inline_in_public_items, + clippy::all, + rustdoc::all +)] +#![cfg_attr(not(test), warn(unused_crate_dependencies))] +#![deny(unused_must_use, rust_2018_idioms)] #![allow( clippy::cast_lossless, clippy::inline_always, + clippy::let_unit_value, clippy::must_use_candidate, clippy::wildcard_imports, unsafe_op_in_unsafe_fn, @@ -32,12 +46,17 @@ extern crate alloc; use cfg_if::cfg_if; +use core::fmt; use core::slice; use core::str; #[cfg(feature = "alloc")] use alloc::{string::String, vec::Vec}; +// `cpufeatures` may be unused when `force-generic` is enabled. +#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +use cpufeatures as _; + // The main encoding and decoding functions. cfg_if! { if #[cfg(feature = "force-generic")] { @@ -60,7 +79,7 @@ cfg_if! { // Otherwise, use our own with the more optimized implementation. cfg_if! { if #[cfg(feature = "hex")] { - pub extern crate hex; + pub use hex; #[doc(inline)] pub use hex::{FromHex, FromHexError, ToHex}; @@ -92,10 +111,12 @@ cfg_if! { // suggests that the function is unlikely to be called #[inline(always)] #[cold] + #[allow(clippy::missing_const_for_fn)] fn cold() {} #[inline(always)] #[allow(dead_code)] + #[allow(clippy::missing_const_for_fn)] fn likely(b: bool) -> bool { if !b { cold(); @@ -104,6 +125,7 @@ cfg_if! { } #[inline(always)] + #[allow(clippy::missing_const_for_fn)] fn unlikely(b: bool) -> bool { if b { cold(); @@ -161,12 +183,13 @@ pub const HEX_DECODE_LUT: &[u8; 256] = &make_decode_lut(); /// ``` #[must_use] #[repr(C)] +#[derive(Clone)] pub struct Buffer { // Workaround for Rust issue #76560: // https://github.com/rust-lang/rust/issues/76560 // This would ideally be `[u8; (N + PREFIX as usize) * 2]` prefix: [u8; 2], - bytes: [u16; N], + bytes: [[u8; 2]; N], } impl Default for Buffer { @@ -176,10 +199,10 @@ impl Default for Buffer { } } -impl Clone for Buffer { +impl fmt::Debug for Buffer { #[inline] - fn clone(&self) -> Self { - Self::new() + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("Buffer").field(&self.as_str()).finish() } } @@ -187,13 +210,18 @@ impl Buffer { /// The length of the buffer in bytes. pub const LEN: usize = (N + PREFIX as usize) * 2; + const ASSERT_SIZE: () = assert!(core::mem::size_of::() == 2 + N * 2, "invalid size"); + const ASSERT_ALIGNMENT: () = assert!(core::mem::align_of::() == 1, "invalid alignment"); + /// This is a cheap operation; you don't need to worry about reusing buffers /// for efficiency. #[inline] pub const fn new() -> Self { + let () = Self::ASSERT_SIZE; + let () = Self::ASSERT_ALIGNMENT; Self { prefix: if PREFIX { [b'0', b'x'] } else { [0, 0] }, - bytes: [0; N], + bytes: [[0; 2]; N], } } @@ -214,7 +242,8 @@ impl Buffer { let mut i = 0; while i < N { let (high, low) = byte2hex::(array[i]); - self.bytes[i] = u16::from_le_bytes([high, low]); + self.bytes[i][0] = high; + self.bytes[i][1] = low; i += 1; } self @@ -243,6 +272,7 @@ impl Buffer { /// /// If the slice is not exactly `N` bytes long. #[track_caller] + #[inline] pub fn format_slice>(&mut self, slice: T) -> &mut str { self.format_slice_inner::(slice.as_ref()) } @@ -254,6 +284,7 @@ impl Buffer { /// /// If the slice is not exactly `N` bytes long. #[track_caller] + #[inline] pub fn format_slice_upper>(&mut self, slice: T) -> &mut str { self.format_slice_inner::(slice.as_ref()) } @@ -320,7 +351,7 @@ impl Buffer { /// /// See Rust tracking issue [#76001](https://github.com/rust-lang/rust/issues/76001). #[inline] - pub fn as_byte_array(&self) -> &[u8; LEN] { + pub const fn as_byte_array(&self) -> &[u8; LEN] { maybe_const_assert!(LEN == Self::LEN, "`LEN` must be equal to `Self::LEN`"); // SAFETY: [u16; N] is layout-compatible with [u8; N * 2]. unsafe { &*self.as_ptr().cast::<[u8; LEN]>() } @@ -366,6 +397,7 @@ impl Buffer { /// # Safety /// /// See [`as_mut_bytes`](Buffer::as_mut_bytes). + #[inline] pub unsafe fn buffer(&mut self) -> &mut [u8] { unsafe { slice::from_raw_parts_mut(self.bytes.as_mut_ptr().cast(), N * 2) } } @@ -376,11 +408,7 @@ impl Buffer { /// function returns, or else it will end up pointing to garbage. #[inline] pub const fn as_ptr(&self) -> *const u8 { - if PREFIX { - self.prefix.as_ptr() - } else { - self.bytes.as_ptr().cast::() - } + unsafe { (self as *const Self).cast::().add(!PREFIX as usize * 2) } } /// Returns an unsafe mutable pointer to the slice's buffer. @@ -389,11 +417,7 @@ impl Buffer { /// function returns, or else it will end up pointing to garbage. #[inline] pub fn as_mut_ptr(&mut self) -> *mut u8 { - if PREFIX { - self.prefix.as_mut_ptr() - } else { - self.bytes.as_mut_ptr().cast::() - } + unsafe { (self as *mut Self).cast::().add(!PREFIX as usize * 2) } } } @@ -432,6 +456,7 @@ pub const fn const_encode( /// # Ok(()) /// # } /// ``` +#[inline] pub fn encode_to_slice>(input: T, output: &mut [u8]) -> Result<(), FromHexError> { encode_to_slice_inner::(input.as_ref(), output) } @@ -453,6 +478,7 @@ pub fn encode_to_slice>(input: T, output: &mut [u8]) -> Result<() /// # Ok(()) /// # } /// ``` +#[inline] pub fn encode_to_slice_upper>( input: T, output: &mut [u8], @@ -474,6 +500,7 @@ pub fn encode_to_slice_upper>( /// assert_eq!(const_hex::encode([1, 2, 3, 15, 16]), "0102030f10"); /// ``` #[cfg(feature = "alloc")] +#[inline] pub fn encode>(data: T) -> String { encode_inner::(data.as_ref()) } @@ -489,6 +516,7 @@ pub fn encode>(data: T) -> String { /// assert_eq!(const_hex::encode_upper([1, 2, 3, 15, 16]), "0102030F10"); /// ``` #[cfg(feature = "alloc")] +#[inline] pub fn encode_upper>(data: T) -> String { encode_inner::(data.as_ref()) } @@ -504,6 +532,7 @@ pub fn encode_upper>(data: T) -> String { /// assert_eq!(const_hex::encode_prefixed([1, 2, 3, 15, 16]), "0x0102030f10"); /// ``` #[cfg(feature = "alloc")] +#[inline] pub fn encode_prefixed>(data: T) -> String { encode_inner::(data.as_ref()) } @@ -519,6 +548,7 @@ pub fn encode_prefixed>(data: T) -> String { /// assert_eq!(const_hex::encode_upper_prefixed([1, 2, 3, 15, 16]), "0x0102030F10"); /// ``` #[cfg(feature = "alloc")] +#[inline] pub fn encode_upper_prefixed>(data: T) -> String { encode_inner::(data.as_ref()) } @@ -551,6 +581,7 @@ pub fn encode_upper_prefixed>(data: T) -> String { /// assert!(const_hex::decode("foo").is_err()); /// ``` #[cfg(feature = "alloc")] +#[inline] pub fn decode>(input: T) -> Result, FromHexError> { fn decode_inner(input: &[u8]) -> Result, FromHexError> { if unlikely(input.len() % 2 != 0) { @@ -589,6 +620,7 @@ pub fn decode>(input: T) -> Result, FromHexError> { /// const_hex::decode_to_slice("0x6b697769", &mut bytes).unwrap(); /// assert_eq!(&bytes, b"kiwi"); /// ``` +#[inline] pub fn decode_to_slice>(input: T, output: &mut [u8]) -> Result<(), FromHexError> { fn decode_to_slice_inner(input: &[u8], output: &mut [u8]) -> Result<(), FromHexError> { if unlikely(input.len() % 2 != 0) { @@ -644,8 +676,10 @@ mod generic { pub(super) unsafe fn encode(input: &[u8], output: *mut u8) { for (i, byte) in input.iter().enumerate() { let (high, low) = byte2hex::(*byte); - output.add(i * 2).write(high); - output.add(i * 2 + 1).write(low); + unsafe { + output.add(i * 2).write(high); + output.add(i * 2 + 1).write(low); + } } } @@ -657,7 +691,7 @@ mod generic { pub(super) unsafe fn decode(input: &[u8], output: &mut [u8]) -> Result<(), FromHexError> { macro_rules! next { ($var:ident, $i:expr) => { - let hex = *input.get_unchecked($i); + let hex = unsafe { *input.get_unchecked($i) }; let $var = HEX_DECODE_LUT[hex as usize]; if unlikely($var == u8::MAX) { return Err(FromHexError::InvalidHexCharacter { diff --git a/src/portable_simd.rs b/src/portable_simd.rs index a035684..3cdbc75 100644 --- a/src/portable_simd.rs +++ b/src/portable_simd.rs @@ -13,7 +13,8 @@ pub(super) unsafe fn encode(input: &[u8], output: *mut u8) { let mut i = 0; let (prefix, chunks, suffix) = input.as_simd::(); - generic::encode::(prefix, output); + // SAFETY: ensured by caller. + unsafe { generic::encode::(prefix, output) }; i += prefix.len() * 2; let hex_table = u8x16::from_array(*crate::get_chars_table::()); @@ -30,13 +31,17 @@ pub(super) unsafe fn encode(input: &[u8], output: *mut u8) { let (hex_lo, hex_hi) = u8x16::interleave(hi, lo); // Store result into the output buffer. - hex_lo.copy_to_slice(slice::from_raw_parts_mut(output.add(i), CHUNK_SIZE)); - i += CHUNK_SIZE; - hex_hi.copy_to_slice(slice::from_raw_parts_mut(output.add(i), CHUNK_SIZE)); - i += CHUNK_SIZE; + // SAFETY: ensured by caller. + unsafe { + hex_lo.copy_to_slice(slice::from_raw_parts_mut(output.add(i), CHUNK_SIZE)); + i += CHUNK_SIZE; + hex_hi.copy_to_slice(slice::from_raw_parts_mut(output.add(i), CHUNK_SIZE)); + i += CHUNK_SIZE; + } } - generic::encode::(suffix, output.add(i)); + // SAFETY: ensured by caller. + unsafe { generic::encode::(suffix, output.add(i)) }; } pub(super) use generic::decode; diff --git a/src/serde.rs b/src/serde.rs index efdfc7f..96c461b 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -1,19 +1,19 @@ //! Hex encoding with [`serde`]. -#[cfg_attr( - feature = "alloc", - doc = r#" -# Examples +//! +//! # Examples +//! +//! ``` +//! # #[cfg(feature = "alloc")] { +//! use serde::{Serialize, Deserialize}; +//! +//! #[derive(Serialize, Deserialize)] +//! struct Foo { +//! #[serde(with = "const_hex")] +//! bar: Vec, +//! } +//! # } +//! ``` -``` -use serde::{Serialize, Deserialize}; - -#[derive(Serialize, Deserialize)] -struct Foo { - #[serde(with = "const_hex")] - bar: Vec, -} -```"# -)] use crate::FromHex; use core::fmt; use core::marker::PhantomData; @@ -30,6 +30,7 @@ mod serialize { /// is always even, each byte in data is always encoded using two hex digits. /// Thus, the resulting string contains exactly twice as many bytes as the input /// data. + #[inline] pub fn serialize(data: T, serializer: S) -> Result where S: Serializer, @@ -41,6 +42,7 @@ mod serialize { /// Serializes `data` as hex string using uppercase characters. /// /// Apart from the characters' casing, this works exactly like [`serialize`]. + #[inline] pub fn serialize_upper(data: T, serializer: S) -> Result where S: Serializer, @@ -57,6 +59,7 @@ pub use serialize::{serialize, serialize_upper}; /// /// Both, upper and lower case characters are valid in the input string and can /// even be mixed (e.g. `f9b4ca`, `F9B4CA` and `f9B4Ca` are all valid strings). +#[inline] pub fn deserialize<'de, D, T>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -72,7 +75,7 @@ where { type Value = T; - fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("a hex encoded string") } diff --git a/src/traits.rs b/src/traits.rs index e70716e..cbe2204 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -77,10 +77,12 @@ fn encode_to_iter, const UPPER: bool>(source: &[u8]) #[allow(deprecated)] impl> ToHex for T { + #[inline] fn encode_hex>(&self) -> U { encode_to_iter::<_, false>(self.as_ref()) } + #[inline] fn encode_hex_upper>(&self) -> U { encode_to_iter::<_, true>(self.as_ref()) } @@ -100,6 +102,7 @@ impl> ToHex for T { /// # Ok::<(), const_hex::FromHexError>(()) /// ``` pub trait FromHex: Sized { + /// The associated error which can be returned from parsing. type Error; /// Creates an instance of type `Self` from the given hex string, or fails @@ -114,6 +117,7 @@ pub trait FromHex: Sized { impl FromHex for Box { type Error = U::Error; + #[inline] fn from_hex>(hex: T) -> Result { Ok(Box::new(FromHex::from_hex(hex.as_ref())?)) } @@ -127,6 +131,7 @@ where { type Error = ::Error; + #[inline] fn from_hex>(hex: T) -> Result { Ok(Cow::Owned(FromHex::from_hex(hex.as_ref())?)) } @@ -136,6 +141,7 @@ where impl FromHex for Rc { type Error = U::Error; + #[inline] fn from_hex>(hex: T) -> Result { Ok(Rc::new(FromHex::from_hex(hex.as_ref())?)) } @@ -145,6 +151,7 @@ impl FromHex for Rc { impl FromHex for Arc { type Error = U::Error; + #[inline] fn from_hex>(hex: T) -> Result { Ok(Arc::new(FromHex::from_hex(hex.as_ref())?)) } @@ -154,6 +161,7 @@ impl FromHex for Arc { impl FromHex for Vec { type Error = crate::FromHexError; + #[inline] fn from_hex>(hex: T) -> Result { crate::decode(hex.as_ref()) } @@ -163,6 +171,7 @@ impl FromHex for Vec { impl FromHex for Vec { type Error = crate::FromHexError; + #[inline] fn from_hex>(hex: T) -> Result { let vec = crate::decode(hex.as_ref())?; // SAFETY: transmuting `u8` to `i8` is safe. @@ -174,6 +183,7 @@ impl FromHex for Vec { impl FromHex for Box<[u8]> { type Error = crate::FromHexError; + #[inline] fn from_hex>(hex: T) -> Result { >::from_hex(hex).map(Vec::into_boxed_slice) } @@ -183,6 +193,7 @@ impl FromHex for Box<[u8]> { impl FromHex for Box<[i8]> { type Error = crate::FromHexError; + #[inline] fn from_hex>(hex: T) -> Result { >::from_hex(hex).map(Vec::into_boxed_slice) } @@ -191,6 +202,7 @@ impl FromHex for Box<[i8]> { impl FromHex for [u8; N] { type Error = crate::FromHexError; + #[inline] fn from_hex>(hex: T) -> Result { let mut buf = [0u8; N]; crate::decode_to_slice(hex.as_ref(), &mut buf)?; @@ -201,6 +213,7 @@ impl FromHex for [u8; N] { impl FromHex for [i8; N] { type Error = crate::FromHexError; + #[inline] fn from_hex>(hex: T) -> Result { let mut buf = [0u8; N]; crate::decode_to_slice(hex.as_ref(), &mut buf)?; diff --git a/src/x86.rs b/src/x86.rs index 480fa93..f7165cf 100644 --- a/src/x86.rs +++ b/src/x86.rs @@ -1,3 +1,5 @@ +#![allow(unsafe_op_in_unsafe_fn)] + use crate::generic; #[cfg(target_arch = "x86")] diff --git a/tests/test.rs b/tests/test.rs index d0d3f3c..0dde37c 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -3,8 +3,19 @@ use const_hex::Buffer; #[test] -#[cfg_attr(miri, ignore)] // false positive -fn prefix() { +fn buffer_fmt() { + let mut buffer = Buffer::<256, true>::new(); + buffer.format(&ALL); + let s = format!("{buffer:?}"); + let pre = "Buffer(\"0x"; + let post = "\")"; + assert_eq!(&s[..pre.len()], pre); + assert_eq!(&s[s.len() - post.len()..], post); + assert_lower(&s[pre.len()..s.len() - post.len()]); +} + +#[test] +fn buffer_prefix() { let mut buffer = Buffer::<256, true>::new(); let s = buffer.format(&ALL); assert_eq!(&s[..2], "0x"); @@ -12,41 +23,41 @@ fn prefix() { } #[test] -fn array_lower() { +fn buffer_array_lower() { let mut buffer = Buffer::<256>::new(); let s = buffer.format(&ALL); assert_lower(s); } #[test] -fn array_upper() { +fn buffer_array_upper() { let mut buffer = Buffer::<256>::new(); let s = buffer.format_upper(&ALL); assert_upper(s); } #[test] -fn slice_lower() { +fn buffer_slice_lower() { let mut buffer = Buffer::<256>::new(); let s = buffer.format_slice(ALL); assert_lower(s); } #[test] -fn slice_upper() { +fn buffer_slice_upper() { let mut buffer = Buffer::<256>::new(); let s = buffer.format_slice_upper(ALL); assert_upper(s); } #[test] -fn const_lower() { +fn buffer_const_lower() { const BUFFER: Buffer<256> = Buffer::new().const_format(&ALL); assert_lower(BUFFER.as_str()); } #[test] -fn const_upper() { +fn buffer_const_upper() { const BUFFER: Buffer<256> = Buffer::new().const_format_upper(&ALL); assert_upper(BUFFER.as_str()); }