Skip to content

Commit

Permalink
Enable some Clippy conversion lints for wasmtime-cranelift (#9536)
Browse files Browse the repository at this point in the history
Similar to how `wasmtime::runtime` has a few off-by-default lints turned
on for it do the same for the compilation phase of `wasmtime-cranelift`.
This is intended to help weed out lossy `as` casts and instead steer
users to `from` or `try_from` conversions.
  • Loading branch information
alexcrichton authored Nov 1, 2024
1 parent e3aab7d commit 7be834f
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 20 deletions.
4 changes: 2 additions & 2 deletions cranelift/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ rust-version.workspace = true
workspace = true

[package.metadata.docs.rs]
# Ask Cargo to build docs with the feature `all-arch`
# Ask Cargo to build docs with the feature `all-arch`
features = ["all-arch"]

[dependencies]
Expand Down Expand Up @@ -62,7 +62,7 @@ default = ["std", "unwind", "host-arch", "timing"]
# The "std" feature enables use of libstd. The "core" feature enables use
# of some minimal std-like replacement libraries. At least one of these two
# features need to be enabled.
std = []
std = ["serde?/std"]

# The "core" feature used to enable a hashmap workaround, but is now
# deprecated (we (i) always use hashbrown, and (ii) don't support a
Expand Down
4 changes: 2 additions & 2 deletions crates/cranelift/src/compiled_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl CompiledFunction {
.into_iter()
.map(|&MachSrcLoc { start, end, loc }| (loc, start, (end - start)));
let instructions = if with_instruction_addresses {
collect_address_maps(len as u32, srclocs)
collect_address_maps(len.try_into().unwrap(), srclocs)
} else {
Default::default()
};
Expand All @@ -123,7 +123,7 @@ impl CompiledFunction {
start_srcloc,
end_srcloc,
body_offset: 0,
body_len: len as u32,
body_len: len.try_into().unwrap(),
};

self.metadata.address_map = address_map;
Expand Down
4 changes: 2 additions & 2 deletions crates/cranelift/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,8 +833,8 @@ impl FunctionCompiler<'_> {
let offset = data.original_position();
let len = data.bytes_remaining();
compiled_function.set_address_map(
offset as u32,
len as u32,
offset.try_into().unwrap(),
len.try_into().unwrap(),
tunables.generate_address_map,
);
}
Expand Down
5 changes: 5 additions & 0 deletions crates/cranelift/src/debug.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
//! Debug utils for WebAssembly using Cranelift.

// FIXME: this whole crate opts-in to these two noisier-than-default lints, but
// this module has lots of hits on this warning which aren't the easiest to
// resolve. Ideally all warnings would be resolved here though.
#![allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]

use crate::CompiledFunctionMetadata;
use cranelift_codegen::isa::TargetIsa;
use object::write::SymbolId;
Expand Down
2 changes: 1 addition & 1 deletion crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
.name_section
.func_names
.get(&func_index)
.map(|s| *s)
.copied()
}

/// Proof-carrying code: create a memtype describing an empty
Expand Down
4 changes: 4 additions & 0 deletions crates/cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
//! This crate provides an implementation of the `wasmtime_environ::Compiler`
//! and `wasmtime_environ::CompilerBuilder` traits.

// See documentation in crates/wasmtime/src/runtime.rs for why this is
// selectively enabled here.
#![warn(clippy::cast_possible_truncation, clippy::cast_sign_loss)]

use cranelift_codegen::{
binemit,
cursor::FuncCursor,
Expand Down
8 changes: 4 additions & 4 deletions crates/cranelift/src/obj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use object::{Architecture, SectionKind, SymbolFlags, SymbolKind, SymbolScope};
use std::collections::HashMap;
use std::ops::Range;
use wasmtime_environ::obj::LibCall;
use wasmtime_environ::Compiler;
use wasmtime_environ::{Compiler, Unsigned};

const TEXT_SECTION_NAME: &[u8] = b".text";

Expand Down Expand Up @@ -341,9 +341,9 @@ impl<'a> UnwindInfoBuilder<'a> {
let requires_extended_counts = code_words > (1 << 5);
let encoded_function_len = function_len / 4;
assert!(encoded_function_len < (1 << 18), "function too large");
let mut word1 = encoded_function_len as u32;
let mut word1 = u32::try_from(encoded_function_len).unwrap();
if !requires_extended_counts {
word1 |= (code_words as u32) << 27;
word1 |= u32::from(code_words) << 27;
}
let unwind_address = self.windows_xdata.len();
self.windows_xdata.extend_from_slice(&word1.to_le_bytes());
Expand Down Expand Up @@ -548,7 +548,7 @@ impl<'a> UnwindInfoBuilder<'a> {
// unwinders just use this constant for a relative addition with the
// address of the FDE, which means that the sign doesn't actually
// matter.
let fde = unwind_info.to_fde(Address::Constant(actual_offset as u64));
let fde = unwind_info.to_fde(Address::Constant(actual_offset.unsigned()));
table.add_fde(cie_id, fde);
}
let endian = match compiler.triple().endianness().unwrap() {
Expand Down
11 changes: 4 additions & 7 deletions crates/cranelift/src/translate/code_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ use crate::translate::state::{ControlStackFrame, ElseData, FuncTranslationState}
use crate::translate::translation_utils::{
block_with_params, blocktype_params_results, f32_translation, f64_translation,
};
use core::{i32, u32};
use cranelift_codegen::ir::condcodes::{FloatCC, IntCC};
use cranelift_codegen::ir::immediates::Offset32;
use cranelift_codegen::ir::types::*;
Expand All @@ -93,8 +92,8 @@ use std::collections::{hash_map, HashMap};
use std::vec::Vec;
use wasmparser::{FuncValidator, MemArg, Operator, WasmModuleResources};
use wasmtime_environ::{
wasm_unsupported, DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, TableIndex,
TypeIndex, WasmRefType, WasmResult,
wasm_unsupported, DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, Signed,
TableIndex, TypeIndex, Unsigned, WasmRefType, WasmResult,
};

/// Given a `Reachability<T>`, unwrap the inner `T` or, when unreachable, set
Expand Down Expand Up @@ -930,7 +929,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
}
/****************************** Nullary Operators ************************************/
Operator::I32Const { value } => {
state.push1(builder.ins().iconst(I32, *value as u32 as i64))
state.push1(builder.ins().iconst(I32, i64::from(value.unsigned())));
}
Operator::I64Const { value } => state.push1(builder.ins().iconst(I64, *value)),
Operator::F32Const { value } => {
Expand Down Expand Up @@ -3289,9 +3288,7 @@ fn align_atomic_addr<FE: FuncEnvironment + ?Sized>(
let effective_addr = if memarg.offset == 0 {
addr
} else {
builder
.ins()
.iadd_imm(addr, i64::from(memarg.offset as i32))
builder.ins().iadd_imm(addr, memarg.offset.signed())
};
debug_assert!(loaded_bytes.is_power_of_two());
let misalignment = builder
Expand Down
4 changes: 2 additions & 2 deletions crates/cranelift/src/translate/func_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,6 @@ fn parse_function_body<FE: FuncEnvironment + ?Sized>(
/// Get the current source location from a reader.
fn cur_srcloc(reader: &BinaryReader) -> ir::SourceLoc {
// We record source locations as byte code offsets relative to the beginning of the file.
// This will wrap around if byte code is larger than 4 GB.
ir::SourceLoc::new(reader.original_position() as u32)
// This will panic if bytecode is larger than 4 GB.
ir::SourceLoc::new(reader.original_position().try_into().unwrap())
}
2 changes: 2 additions & 0 deletions crates/wasmtime/src/runtime/vm.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Runtime library support for Wasmtime.

#![deny(missing_docs)]
// See documentation in crates/wasmtime/src/runtime.rs for why this is
// selectively enabled here.
#![warn(clippy::cast_sign_loss)]

use crate::prelude::*;
Expand Down

0 comments on commit 7be834f

Please sign in to comment.