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.

Sign up for GitHub

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

fewer clippy warnings #11

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fewer clippy warnings #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51f749d
fewer clippy warnings
andrewgazelka Nov 9, 2022
d146e84
fix requests
andrewgazelka Nov 9, 2022
d469047
fix unused import lint
andrewgazelka Mar 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "bit-struct"
version = "0.3.0"
version = "0.4.0"
edition = "2021"
description = "Define structs which have fields which are assigned to individual bits, not bytes"
repository = "https://github.com/parallel-systems/bit-struct"
Expand Down
32 changes: 29 additions & 3 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,13 @@ pub struct GetSet<'a, P, T, const START: usize, const STOP: usize> {

impl<'a, P, T, const START: usize, const STOP: usize> GetSet<'a, P, T, START, STOP> {
/// The bit offset at which this `GetSet` instance starts
#[must_use]
pub const fn start(&self) -> usize {
START
}

/// The bit offset at which this `GetSet` instance ends
#[must_use]
pub const fn stop(&self) -> usize {
STOP
}
Expand Down Expand Up @@ -206,6 +208,7 @@ impl<
> GetSet<'a, P, T, START, STOP>
{
/// Get the property this `GetSet` points at
#[must_use]
pub fn get(&self) -> T {
let section = self.get_raw();
// Safety:
Expand All @@ -216,13 +219,15 @@ impl<

/// Returns true if the memory this `GetSet` points at is a valid
/// representation of `T`
#[must_use]
pub fn is_valid(&self) -> bool {
let section = self.get_raw();
T::is_valid(section)
}

/// Get the raw bits being pointed at, without type conversion nor any form
/// of validation
#[must_use]
pub fn get_raw(&self) -> P {
let parent = *self.parent;
let mask = self.mask();
Expand Down Expand Up @@ -338,9 +343,12 @@ macro_rules! bit_struct_impl {

impl $name {

/// Creates an empty struct. This may or may not be valid
/// Creates an empty struct.
///
/// # Safety
/// This is safe if the bit-struct can be represented by a zero.
pub unsafe fn empty() -> Self {
unsafe { Self::from_unchecked(<$kind as $crate::BitStructZero>::bs_zero()) }
Self::from_unchecked(<$kind as $crate::BitStructZero>::bs_zero())
}

#[doc = concat!("Returns a valid representation for [`", stringify!($name), "`] where all values are")]
Expand Down Expand Up @@ -373,6 +381,7 @@ macro_rules! bit_struct_impl {
/// A bit struct which has a zero value we can get
pub trait BitStructZero: Zero {
/// Get a zero value for this bit struct
#[must_use]
fn bs_zero() -> Self {
Self::zero()
}
Expand Down Expand Up @@ -540,6 +549,8 @@ macro_rules! bit_struct {
impl $crate::BitStruct<{$(<$actual as $crate::ValidCheck<$kind>>::ALWAYS_VALID &&)* true}> for $name {
type Kind = $kind;

/// # Safety
/// - Creates the bit-struct without checking whether `inner` is a valid representation.
unsafe fn from_unchecked(inner: $kind) -> Self {
andrewgazelka marked this conversation as resolved.
Show resolved Hide resolved
Self(unsafe {$crate::UnsafeStorage::new_unsafe(inner)})
}
Expand All @@ -548,12 +559,16 @@ macro_rules! bit_struct {
#[allow(clippy::used_underscore_binding)]
impl $name {

/// # Safety
/// - Creates the bit-struct without checking whether `inner` is a valid representation.
unsafe fn from_unchecked(inner: $kind) -> Self {
Self(unsafe {$crate::UnsafeStorage::new_unsafe(inner)})
}

#[allow(clippy::too_many_arguments)]
pub fn new($($field: $actual),*) -> Self {
// SAFETY:
// - This is implemented automatically by the bit-struct crate.
let mut res = unsafe { Self::from_unchecked(<$kind as $crate::BitStructZero>::bs_zero()) };
$(
res.$field().set($field);
Expand Down Expand Up @@ -607,6 +622,7 @@ macro_rules! count_idents {
/// assert_eq!(bits(5), 3);
/// assert_eq!(bits(32), 6);
/// ```
#[must_use]
pub const fn bits(num: usize) -> usize {
/// Helper function for [`bits`]
const fn helper(count: usize, on: usize) -> usize {
Expand Down Expand Up @@ -637,6 +653,7 @@ macro_rules! enum_impl {
};
(VALID_CORE $name: ident: [$($kind: ty),*]) => {
$(
#[allow(clippy::undocumented_unsafe_blocks, clippy::use_self)]
unsafe impl $crate::ValidCheck<$kind> for $name {
const ALWAYS_VALID: bool = <Self as $crate::ValidCheck<u8>>::ALWAYS_VALID;
fn is_valid(value: $kind) -> bool {
Expand All @@ -653,6 +670,7 @@ macro_rules! enum_impl {
};
(VALID_BIT_STRUCT $name: ident: [$($kind: ty),*]) => {
$(
#[allow(clippy::undocumented_unsafe_blocks, clippy::use_self)]
unsafe impl $crate::ValidCheck<$kind> for $name {
const ALWAYS_VALID: bool = <Self as $crate::ValidCheck<u8>>::ALWAYS_VALID;
fn is_valid(value: $kind) -> bool {
Expand Down Expand Up @@ -709,14 +727,19 @@ macro_rules! enum_impl {
),*
}

/// # Safety
/// - This is implemented automatically by the bit-struct crate.
#[allow(clippy::use_self)]
unsafe impl $crate::BitCount for $name {
const COUNT: usize = $crate::bits($crate::count_idents!(0, [$($field),*]));
}

#[allow(clippy::use_self)]
impl $name {
const VARIANT_COUNT: usize = $crate::enum_impl!(COUNT $fst_field $(,$field)*);
}

#[allow(clippy::undocumented_unsafe_blocks, clippy::use_self)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be better to document these with a safety comment than put a clippy allowance (unless you tried that and it didn't work?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't seem to work for some odd reason. If you want to allocate time to try it and it works for you, I wouldn't mind. It seemed clippy::use_self also had false positives—I couldn't find anywhere I could use Self, but I wouldn't be surprised if I am just dumb.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was mainly referring to the clippy::undocumented_unsafe_blocks allowance, which you should be able to resolve by putting a safety comment on this unsafe impl and the others like it.

Also, with the clippy::use_self, it seems like you missed the spot where the usage is happening because I'm still seeing that lint trigger with the allowances you added in.

unsafe impl $crate::ValidCheck<u8> for $name {
const ALWAYS_VALID: bool = Self::VARIANT_COUNT.count_ones() == 1;
fn is_valid(value: u8) -> bool {
Expand All @@ -730,6 +753,7 @@ macro_rules! enum_impl {

$crate::enum_impl!(FROM_IMPLS $name);

#[allow(clippy::use_self)]
impl Default for $name {
fn default() -> Self {
Self::$default
Expand Down Expand Up @@ -762,6 +786,7 @@ macro_rules! enum_impl {
),*
}

#[allow(clippy::use_self)]
impl Default for $name {
fn default() -> Self {
Self::$fst_field
Expand All @@ -772,11 +797,12 @@ macro_rules! enum_impl {
const VARIANT_COUNT: usize = $crate::enum_impl!(COUNT $fst_field $(,$field)*);
}

#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl $crate::BitCount for $name {
const COUNT: usize = $crate::bits($crate::count_idents!(0, [$($field),*]));
}


#[allow(clippy::undocumented_unsafe_blocks)]
unsafe impl $crate::ValidCheck<u8> for $name {
const ALWAYS_VALID: bool = Self::VARIANT_COUNT.count_ones() == 1;

Expand Down
29 changes: 18 additions & 11 deletions src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
//! New integer types used in this crate, and trait implementations for those
//! types

use super::*;
use super::{
serde, Add, BitAnd, BitAndAssign, BitCount, BitOr, BitOrAssign, BitXor, BitXorAssign,
BitsFitIn, Bounded, Debug, Display, Div, FieldStorage, Mul, Num, One, Rem, Shl, ShlAssign, Shr,
ShrAssign, Sub, ValidCheck, Zero,
};
use serde::{Deserializer, Serializer};

/// Assert that the given type is valid for any representation thereof
Expand Down Expand Up @@ -54,6 +58,7 @@ impl_field_storage!(
(u64, Self),
(u128, Self),
);
/// helper macro to implement signed types
macro_rules! impl_signed_field_storage {
($(($type:ty, $base:ty)),+ $(,)?) => {
$(
Expand Down Expand Up @@ -156,7 +161,7 @@ macro_rules! new_signed_types {
/// Create a new $name from value
/// # Safety
/// - value must fit within the number of bits defined in the type
pub const unsafe fn new_unchecked(value: $signed) -> Self {
#[must_use] pub const unsafe fn new_unchecked(value: $signed) -> Self {
let unsigned_value = value as $inner;
if value >= 0 {
Self(unsigned_value)
Expand All @@ -171,7 +176,7 @@ macro_rules! new_signed_types {
/// Create a new $name from value
/// # Safety
/// - value must fit within the number of bits defined in the type
pub fn new(value: $signed) -> Option<Self> {
#[must_use] pub fn new(value: $signed) -> Option<Self> {
if (Self::MIN..=Self::MAX).contains(&value) {
// SAFETY:
// We've just checked that this is safe to call
Expand All @@ -189,7 +194,7 @@ macro_rules! new_signed_types {
pub const MIN: $signed = -(Self::MAX_UNSIGNED as $signed) - 1;

/// Get the value stored in here, as a signed integer
pub const fn value(self) -> $signed {
#[must_use] pub const fn value(self) -> $signed {
match self.0 >> ($count - 1) {
0 => self.0 as $signed,
_ => {
Expand Down Expand Up @@ -467,14 +472,14 @@ macro_rules! new_unsigned_types {
///
/// # Safety
/// The value must be valid value of the given type.
pub const unsafe fn new_unchecked(value: $inner) -> Self {
#[must_use] pub const unsafe fn new_unchecked(value: $inner) -> Self {
Self(value)
}

#[doc = concat!("Create a new ", stringify!($name), " from an inner value")]
///
/// This method checks that the inner value is valid, and return `None` if it isn't.
pub fn new(value: $inner) -> Option<Self> {
#[must_use] pub fn new(value: $inner) -> Option<Self> {
if (Self::MIN..=Self::MAX).contains(&value) {
// SAFETY:
// We've checked that this is safe to do in the above `if`
Expand All @@ -485,7 +490,7 @@ macro_rules! new_unsigned_types {
}

/// Get the stored value
pub const fn value(self) -> $inner {
#[must_use] pub const fn value(self) -> $inner {
self.0
}
}
Expand Down Expand Up @@ -646,7 +651,7 @@ macro_rules! byte_from_impls {
/// The size of byte array equal to the underlying storage for this value
const SUPER_BYTES: usize = ::core::mem::size_of::<$super_kind>();
/// Convert from an array of bytes, in big-endian order
pub fn from_be_bytes(bytes: [u8; Self::ARR_SIZE]) -> Self {
#[must_use] pub fn from_be_bytes(bytes: [u8; Self::ARR_SIZE]) -> Self {
let mut res_bytes = [0_u8; Self::SUPER_BYTES];
for (set, &get) in res_bytes.iter_mut().rev().zip(bytes.iter().rev()) {
*set = get;
Expand All @@ -655,7 +660,7 @@ macro_rules! byte_from_impls {
}

/// Convert `self` into an array of bytes, in big-endian order
pub fn to_be_bytes(self) -> [u8; Self::ARR_SIZE] {
#[must_use] pub fn to_be_bytes(self) -> [u8; Self::ARR_SIZE] {
let mut res = [0; Self::ARR_SIZE];
let inner_bytes = self.0.to_be_bytes();
for (&get, set) in inner_bytes.iter().rev().zip(res.iter_mut().rev()) {
Expand All @@ -665,7 +670,7 @@ macro_rules! byte_from_impls {
}

/// Convert from an array of bytes, in little-endian order
pub fn from_le_bytes(bytes: [u8; Self::ARR_SIZE]) -> Self {
#[must_use] pub fn from_le_bytes(bytes: [u8; Self::ARR_SIZE]) -> Self {
let mut res_bytes = [0_u8; Self::SUPER_BYTES];
for (set, &get) in res_bytes.iter_mut().zip(bytes.iter()) {
*set = get;
Expand All @@ -674,7 +679,7 @@ macro_rules! byte_from_impls {
}

/// Convert `self` into an array of bytes, in little-endian order
pub fn to_le_bytes(self) -> [u8; Self::ARR_SIZE] {
#[must_use] pub fn to_le_bytes(self) -> [u8; Self::ARR_SIZE] {
let mut res = [0; Self::ARR_SIZE];
let inner_bytes = self.0.to_le_bytes();
for (&get, set) in inner_bytes.iter().zip(res.iter_mut()) {
Expand Down Expand Up @@ -724,13 +729,15 @@ impl u1 {
/// Implement `BitsFitIn` for the given pair of types, using the given method
macro_rules! bits_fit_in_impl {
($basety:ty => $target:ty : from) => {
#[allow(clippy::use_self)]
impl BitsFitIn<$target> for $basety {
fn fit(self) -> $target {
self.inner_raw().into()
}
}
};
($basety:ty => $target:ty : new_unchecked) => {
#[allow(clippy::use_self)]
impl BitsFitIn<$target> for $basety {
fn fit(self) -> $target {
// Safety:
Expand Down