Skip to content

Commit

Permalink
Selectively enable clippy::cast_sign_loss
Browse files Browse the repository at this point in the history
This would have fixed bytecodealliance#7558 so try to head off future issues with that
by warning against this situation in a few crates. This lint is still
quite noisy though for Cranelift for example so it's not worthwhile at
this time to enable it for the whole workspace.
  • Loading branch information
alexcrichton committed Nov 20, 2023
1 parent e351a2b commit ff4722c
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 15 deletions.
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,16 @@ edition = "2021"
rust-version = "1.72.0"

[workspace.lints.rust]
# Turn on some lints which are otherwise allow-by-default in rustc.
unused_extern_crates = 'warn'
trivial_numeric_casts = 'warn'
unstable_features = 'warn'
unused_import_braces = 'warn'

[workspace.lints.clippy]
# The default set of lints in Clippy is viewed as "too noisy" right now so
# they're all turned off by default. Selective lints are then enabled below as
# necessary.
all = 'allow'

[workspace.dependencies]
Expand Down
2 changes: 2 additions & 0 deletions cranelift/entity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ mod map;
mod primary;
mod set;
mod sparse;
mod unsigned;

pub use self::boxed_slice::BoxedSlice;
pub use self::iter::{Iter, IterMut};
Expand All @@ -211,6 +212,7 @@ pub use self::map::SecondaryMap;
pub use self::primary::PrimaryMap;
pub use self::set::EntitySet;
pub use self::sparse::{SparseMap, SparseMapValue, SparseSet};
pub use self::unsigned::Unsigned;

/// A collection of tests to ensure that use of the different `entity_impl!` forms will generate
/// `EntityRef` implementations that behave the same way.
Expand Down
71 changes: 71 additions & 0 deletions cranelift/entity/src/unsigned.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/// Helper trait used to add `unsigned()` methods to primitive signed integer
/// types.
///
/// The purpose of this trait is to signal the intent that the sign bit of a
/// signed integer is intended to be discarded and the value is instead
/// understood to be a "bag of bits" where the conversion to an unsigned number
/// is intended to be lossless. This can be used for example when converting a
/// signed integer into a larger width with zero-extension.
pub trait Unsigned {
/// The unsigned integer for this type which has the same width.
type Unsigned;

/// View this signed integer as an unsigned integer of the same width.
///
/// All bits are preserved.
fn unsigned(self) -> Self::Unsigned;
}

impl Unsigned for i8 {
type Unsigned = u8;

#[inline]
fn unsigned(self) -> u8 {
self as u8
}
}

impl Unsigned for i16 {
type Unsigned = u16;

#[inline]
fn unsigned(self) -> u16 {
self as u16
}
}

impl Unsigned for i32 {
type Unsigned = u32;

#[inline]
fn unsigned(self) -> u32 {
self as u32
}
}

impl Unsigned for i64 {
type Unsigned = u64;

#[inline]
fn unsigned(self) -> u64 {
self as u64
}
}

impl Unsigned for i128 {
type Unsigned = u128;

#[inline]
fn unsigned(self) -> u128 {
self as u128
}
}

impl Unsigned for isize {
type Unsigned = usize;

#[inline]
fn unsigned(self) -> usize {
self as usize
}
}
1 change: 1 addition & 0 deletions crates/environ/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! linear memories.

#![deny(missing_docs)]
#![warn(clippy::cast_sign_loss)]

mod address_map;
mod builtin;
Expand Down
10 changes: 5 additions & 5 deletions crates/environ/src/module_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::module::{
use crate::{
DataIndex, DefinedFuncIndex, ElemIndex, EntityIndex, EntityType, FuncIndex, GlobalIndex,
GlobalInit, MemoryIndex, ModuleTypesBuilder, PrimaryMap, SignatureIndex, TableIndex,
TableInitialValue, Tunables, TypeConvert, TypeIndex, WasmError, WasmFuncType, WasmHeapType,
WasmResult, WasmType,
TableInitialValue, Tunables, TypeConvert, TypeIndex, Unsigned, WasmError, WasmFuncType,
WasmHeapType, WasmResult, WasmType,
};
use cranelift_entity::packed_option::ReservedValue;
use std::borrow::Cow;
Expand Down Expand Up @@ -487,7 +487,7 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
let table_index = TableIndex::from_u32(table_index.unwrap_or(0));
let mut offset_expr_reader = offset_expr.get_binary_reader();
let (base, offset) = match offset_expr_reader.read_operator()? {
Operator::I32Const { value } => (None, value as u32),
Operator::I32Const { value } => (None, value.unsigned()),
Operator::GlobalGet { global_index } => {
(Some(GlobalIndex::from_u32(global_index)), 0)
}
Expand Down Expand Up @@ -607,8 +607,8 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
let memory_index = MemoryIndex::from_u32(memory_index);
let mut offset_expr_reader = offset_expr.get_binary_reader();
let (base, offset) = match offset_expr_reader.read_operator()? {
Operator::I32Const { value } => (None, value as u64),
Operator::I64Const { value } => (None, value as u64),
Operator::I32Const { value } => (None, value.unsigned().into()),
Operator::I64Const { value } => (None, value.unsigned()),
Operator::GlobalGet { global_index } => {
(Some(GlobalIndex::from_u32(global_index)), 0)
}
Expand Down
3 changes: 2 additions & 1 deletion crates/runtime/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Runtime library support for Wasmtime.

#![deny(missing_docs)]
#![warn(clippy::cast_sign_loss)]

use anyhow::{Error, Result};
use std::fmt;
Expand Down Expand Up @@ -240,7 +241,7 @@ pub fn page_size() -> usize {

#[cfg(unix)]
fn get_page_size() -> usize {
unsafe { libc::sysconf(libc::_SC_PAGESIZE) as usize }
unsafe { libc::sysconf(libc::_SC_PAGESIZE).try_into().unwrap() }
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/runtime/src/libcalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use std::mem;
use std::ptr::{self, NonNull};
use std::time::{Duration, Instant};
use wasmtime_environ::{
DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, TableIndex, Trap,
DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, TableIndex, Trap, Unsigned,
};
#[cfg(feature = "wmemcheck")]
use wasmtime_wmemcheck::AccessError::{
Expand Down Expand Up @@ -233,7 +233,7 @@ unsafe fn table_grow(
};
Ok(match instance.table_grow(table_index, delta, element)? {
Some(r) => r,
None => -1_i32 as u32,
None => (-1_i32).unsigned(),
})
}

Expand Down
2 changes: 1 addition & 1 deletion crates/runtime/src/traphandlers/macos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
//! function declarations. Many bits and pieces are copied or translated from
//! the SpiderMonkey implementation and it should pass all the tests!

#![allow(non_snake_case)]
#![allow(non_snake_case, clippy::cast_sign_loss)]

use crate::traphandlers::{tls, wasmtime_longjmp};
use mach::exception_types::*;
Expand Down
8 changes: 4 additions & 4 deletions crates/runtime/src/vmcontext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::ptr::NonNull;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::u32;
pub use vm_host_func_context::{VMArrayCallHostFuncContext, VMNativeCallHostFuncContext};
use wasmtime_environ::{DefinedMemoryIndex, VMCONTEXT_MAGIC};
use wasmtime_environ::{DefinedMemoryIndex, Unsigned, VMCONTEXT_MAGIC};

/// A function pointer that exposes the array calling convention.
///
Expand Down Expand Up @@ -1038,7 +1038,7 @@ impl ValRaw {
// `wasmtime` crate. Otherwise though all `ValRaw` constructors are
// otherwise constrained to guarantee that the initial 64-bits are
// always initialized.
ValRaw::u64((i as u32).into())
ValRaw::u64(i.unsigned().into())
}

/// Creates a WebAssembly `i64` value
Expand Down Expand Up @@ -1112,13 +1112,13 @@ impl ValRaw {
/// Gets the WebAssembly `i32` value
#[inline]
pub fn get_u32(&self) -> u32 {
self.get_i32() as u32
self.get_i32().unsigned()
}

/// Gets the WebAssembly `i64` value
#[inline]
pub fn get_u64(&self) -> u64 {
self.get_i64() as u64
self.get_i64().unsigned()
}

/// Gets the WebAssembly `f32` value
Expand Down
3 changes: 3 additions & 0 deletions crates/wasi-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
//! `WasiCtx::builder(...)` function. The
//! `wasi_cap_std_sync::WasiCtxBuilder::new()` function uses this public
//! interface to plug in its own implementations of each of these resources.

#![warn(clippy::cast_sign_loss)]

pub mod clocks;
mod ctx;
pub mod dir;
Expand Down
4 changes: 3 additions & 1 deletion crates/wasi-common/src/snapshots/preview_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,9 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
let whence = match whence {
types::Whence::Cur => SeekFrom::Current(offset),
types::Whence::End => SeekFrom::End(offset),
types::Whence::Set => SeekFrom::Start(offset as u64),
types::Whence::Set => {
SeekFrom::Start(offset.try_into().map_err(|_| Error::invalid_argument())?)
}
};
let newoffset = self
.table()
Expand Down
2 changes: 2 additions & 0 deletions crates/wasi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
//! Individual snapshots are available through
//! `wasmtime_wasi::snapshots::preview_{0, 1}::Wasi::new(&Store, Rc<RefCell<WasiCtx>>)`.

#![warn(clippy::cast_sign_loss)]

#[cfg(feature = "preview2")]
pub mod preview2;

Expand Down
4 changes: 3 additions & 1 deletion crates/wasi/src/preview2/preview1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1565,7 +1565,9 @@ impl<
let position = position.clone();
drop(t);
let pos = match whence {
types::Whence::Set if offset >= 0 => offset as _,
types::Whence::Set if offset >= 0 => {
offset.try_into().map_err(|_| types::Errno::Inval)?
}
types::Whence::Cur => position
.load(Ordering::Relaxed)
.checked_add_signed(offset)
Expand Down

0 comments on commit ff4722c

Please sign in to comment.