Skip to content

Commit

Permalink
Start using #[expect] instead of #[allow] (#9696)
Browse files Browse the repository at this point in the history
* Start using `#[expect]` instead of `#[allow]`

In Rust 1.81, our new MSRV, a new feature was added to Rust to use
`#[expect]` to control lint levels. This new lint annotation will
silence a lint but will itself cause a lint if it doesn't actually
silence anything. This is quite useful to ensure that annotations don't
get stale over time.

Another feature is the ability to use a `reason` directive on the
attribute with a string explaining why the attribute is there. This
string is then rendered in compiler messages if a warning or error
happens.

This commit migrates applies a few changes across the workspace:

* Some `#[allow]` are changed to `#[expect]` with a `reason`.
* Some `#[allow]` have a `reason` added if the lint conditionally fires
  (mostly related to macros).
* Some `#[allow]` are removed since the lint doesn't actually fire.
* The workspace configures `clippy::allow_attributes_without_reason = 'warn'`
  as a "ratchet" to prevent future regressions.
* Many crates are annotated to allow `allow_attributes_without_reason`
  during this transitionary period.

The end-state is that all crates should use
`#[expect(..., reason = "...")]` for any lint that unconditionally fires
but is expected. The `#[allow(..., reason = "...")]` lint should be used
for conditionally firing lints, primarily in macro-related code.
The `allow_attributes_without_reason = 'warn'` level is intended to be
permanent but the transitionary
`#[expect(clippy::allow_attributes_without_reason)]` crate annotations
to go away over time.

* Fix adapter build

prtest:full

* Fix one-core build of icache coherence

* Use `allow` for missing_docs

Work around rust-lang/rust#130021 which was fixed in Rust 1.83 and isn't
fixed for our MSRV at this time.

* More MSRV compat
  • Loading branch information
alexcrichton authored Dec 2, 2024
1 parent af476a5 commit 45b60bd
Show file tree
Hide file tree
Showing 69 changed files with 121 additions and 106 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ manual_strip = 'warn'
unnecessary_mut_passed = 'warn'
unnecessary_fallible_conversions = 'warn'
unnecessary_cast = 'warn'
allow_attributes_without_reason = 'warn'

[workspace.dependencies]
arbitrary = { version = "1.3.1" }
Expand Down
2 changes: 2 additions & 0 deletions benches/call.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

use criterion::measurement::WallTime;
use criterion::{criterion_group, criterion_main, BenchmarkGroup, Criterion};
use std::fmt::Debug;
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/meta/src/shared/instructions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(non_snake_case)]
#![expect(non_snake_case, reason = "DSL style here")]

use crate::cdsl::instructions::{
AllInstructions, InstructionBuilder as Inst, InstructionGroupBuilder,
Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// built for one platform we don't have to worry too much about trimming
// everything down.
#![cfg_attr(not(feature = "all-arch"), allow(dead_code))]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

#[allow(unused_imports)] // #[macro_use] is required for no_std
#[macro_use]
Expand Down
12 changes: 6 additions & 6 deletions cranelift/entity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,29 +145,29 @@ macro_rules! entity_impl {

impl $entity {
/// Create a new instance from a `u32`.
#[allow(dead_code)]
#[allow(dead_code, reason = "macro-generated code")]
#[inline]
pub fn from_u32(x: u32) -> Self {
debug_assert!(x < $crate::__core::u32::MAX);
$entity(x)
}

/// Return the underlying index value as a `u32`.
#[allow(dead_code)]
#[allow(dead_code, reason = "macro-generated code")]
#[inline]
pub fn as_u32(self) -> u32 {
self.0
}

/// Return the raw bit encoding for this instance.
#[allow(dead_code)]
#[allow(dead_code, reason = "macro-generated code")]
#[inline]
pub fn as_bits(self) -> u32 {
self.0
}

/// Create a new instance from the raw bit encoding.
#[allow(dead_code)]
#[allow(dead_code, reason = "macro-generated code")]
#[inline]
pub fn from_bits(x: u32) -> Self {
$entity(x)
Expand Down Expand Up @@ -225,7 +225,7 @@ macro_rules! entity_impl {

impl $entity {
/// Create a new instance from a `u32`.
#[allow(dead_code)]
#[allow(dead_code, reason = "macro-generated code")]
#[inline]
pub fn from_u32(x: u32) -> Self {
debug_assert!(x < $crate::__core::u32::MAX);
Expand All @@ -234,7 +234,7 @@ macro_rules! entity_impl {
}

/// Return the underlying index value as a `u32`.
#[allow(dead_code)]
#[allow(dead_code, reason = "macro-generated code")]
#[inline]
pub fn as_u32(self) -> u32 {
let $arg = self;
Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! available filetest commands.

#![deny(missing_docs)]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

pub use crate::function_runner::TestFileCompiler;
use crate::runner::TestRunner;
Expand Down
1 change: 1 addition & 0 deletions cranelift/frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@

#![deny(missing_docs)]
#![no_std]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

#[allow(unused_imports)] // #[macro_use] is required for no_std
#[macro_use]
Expand Down
2 changes: 2 additions & 0 deletions cranelift/interpreter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
//!
//! This module is a project for interpreting Cranelift IR.

#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

pub mod address;
pub mod environment;
pub mod frame;
Expand Down
1 change: 1 addition & 0 deletions cranelift/isle/isle/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![doc = include_str!("../README.md")]
#![deny(missing_docs)]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

macro_rules! declare_id {
(
Expand Down
1 change: 1 addition & 0 deletions cranelift/module/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![deny(missing_docs)]
#![no_std]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

#[cfg(not(feature = "std"))]
#[macro_use]
Expand Down
1 change: 1 addition & 0 deletions cranelift/reader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! testing Cranelift, but is not essential for a JIT compiler.

#![deny(missing_docs)]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

pub use crate::error::{Location, ParseError, ParseResult};
pub use crate::isaspec::{parse_option, parse_options, IsaSpec, ParseOptionError};
Expand Down
2 changes: 1 addition & 1 deletion cranelift/src/bugpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ impl<'a> CrashCheckContext<'a> {
}
}

#[cfg_attr(test, allow(unreachable_code))]
#[cfg_attr(test, expect(unreachable_code, reason = "test-specific code"))]
fn check_for_crash(&mut self, func: &Function) -> CheckResult {
self.context.clear();

Expand Down
2 changes: 1 addition & 1 deletion crates/c-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//! but otherwise an accompanying `wasmtime.h` API is provided which is more
//! specific to Wasmtime and has fewer gymnastics to implement.

#![allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
#![expect(non_camel_case_types, reason = "matching C style, not Rust")]

pub use wasmtime;

Expand Down
2 changes: 1 addition & 1 deletion crates/cli-flags/src/opt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ macro_rules! wasmtime_option_group {
}

#[derive(Clone, Debug,PartialEq)]
#[allow(non_camel_case_types)]
#[expect(non_camel_case_types, reason = "macro-generated code")]
enum $option {
$(
$opt($payload),
Expand Down
1 change: 1 addition & 0 deletions crates/component-macro/tests/codegen.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(dead_code)]
#![expect(clippy::allow_attributes_without_reason, reason = "crate not migrated")]

macro_rules! gentest {
($id:ident $name:tt $path:tt) => {
Expand Down
6 changes: 5 additions & 1 deletion crates/cranelift/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
// 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)]
#![expect(
clippy::cast_possible_truncation,
clippy::cast_sign_loss,
reason = "haven't had a chance to fix these yet"
)]

use crate::CompiledFunctionMetadata;
use core::fmt;
Expand Down
3 changes: 1 addition & 2 deletions crates/cranelift/src/debug/transform/expression.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(trivial_numeric_casts)]

use super::address_transform::AddressTransform;
use crate::debug::ModuleMemoryOffset;
use crate::translate::get_vmctx_value_label;
Expand Down Expand Up @@ -796,6 +794,7 @@ impl std::fmt::Debug for JumpTargetMarker {
}

#[cfg(test)]
#[expect(trivial_numeric_casts, reason = "macro-generated code")]
mod tests {
use super::{
compile_expression, AddressTransform, CompiledExpression, CompiledExpressionPart,
Expand Down
2 changes: 1 addition & 1 deletion crates/cranelift/src/debug/transform/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn replace_pointer_type(
macro_rules! add_tag {
($parent_id:ident, $tag:expr => $die:ident as $die_id:ident { $($a:path = $v:expr),* }) => {
let $die_id = comp_unit.add($parent_id, $tag);
#[allow(unused_variables)]
#[allow(unused_variables, reason = "sometimes not used below")]
let $die = comp_unit.get_mut($die_id);
$( $die.set($a, $v); )*
};
Expand Down
3 changes: 0 additions & 3 deletions crates/cranelift/src/debug/write_debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@ use cranelift_codegen::isa::{
use gimli::write::{Address, Dwarf, EndianVec, FrameTable, Result, Sections, Writer};
use gimli::{RunTimeEndian, SectionId};

#[allow(missing_docs)]
pub struct DwarfSection {
pub name: &'static str,
pub body: Vec<u8>,
pub relocs: Vec<DwarfSectionReloc>,
}

#[allow(missing_docs)]
#[derive(Clone)]
pub struct DwarfSectionReloc {
pub target: DwarfSectionRelocTarget,
Expand All @@ -24,7 +22,6 @@ pub struct DwarfSectionReloc {
pub size: u8,
}

#[allow(missing_docs)]
#[derive(Clone)]
pub enum DwarfSectionRelocTarget {
Func(usize),
Expand Down
2 changes: 1 addition & 1 deletion crates/cranelift/src/obj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ struct UnwindInfoBuilder<'a> {
// platforms. Note that all of these specifiers here are relative to a "base
// address" which we define as the base of where the text section is eventually
// loaded.
#[allow(non_camel_case_types)]
#[expect(non_camel_case_types, reason = "matching Windows style, not Rust")]
struct RUNTIME_FUNCTION {
begin: u32,
end: u32,
Expand Down
2 changes: 1 addition & 1 deletion crates/environ/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ macro_rules! declare_indexes {
)*
) => {
$( #[$this_attr] )*
#[allow(missing_docs)]
#[allow(missing_docs, reason = "macro-generated")]
pub const fn $this_name() -> Self {
Self($index)
}
Expand Down
10 changes: 5 additions & 5 deletions crates/environ/src/compile/module_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub struct FunctionBodyData<'a> {
}

#[derive(Debug, Default)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct DebugInfoData<'a> {
pub dwarf: Dwarf<'a>,
pub name_section: NameSection<'a>,
Expand All @@ -131,21 +131,21 @@ pub struct DebugInfoData<'a> {
pub debug_tu_index: gimli::DebugTuIndex<Reader<'a>>,
}

#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing")]
pub type Dwarf<'input> = gimli::Dwarf<Reader<'input>>;

type Reader<'input> = gimli::EndianSlice<'input, gimli::LittleEndian>;

#[derive(Debug, Default)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct NameSection<'a> {
pub module_name: Option<&'a str>,
pub func_names: HashMap<FuncIndex, &'a str>,
pub locals_names: HashMap<FuncIndex, HashMap<u32, &'a str>>,
}

#[derive(Debug, Default)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct WasmFileInfo {
pub path: Option<PathBuf>,
pub code_section_offset: u64,
Expand All @@ -154,7 +154,7 @@ pub struct WasmFileInfo {
}

#[derive(Debug)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct FunctionMetadata {
pub params: Box<[WasmValType]>,
pub locals: Box<[(u32, WasmValType)]>,
Expand Down
20 changes: 10 additions & 10 deletions crates/environ/src/component/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ use std::hash::Hash;
use std::ops::Index;
use wasmparser::component_types::ComponentCoreModuleTypeId;

/// High-level representation of a component as a "data-flow graph".
#[derive(Default)]
#[allow(missing_docs)]
pub struct ComponentDfg {
/// Same as `Component::import_types`
pub import_types: PrimaryMap<ImportIndex, (String, TypeDef)>,
Expand Down Expand Up @@ -153,7 +153,7 @@ pub enum SideEffect {
macro_rules! id {
($(pub struct $name:ident(u32);)*) => ($(
#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "tedious to document")]
pub struct $name(u32);
cranelift_entity::entity_impl!($name);
)*)
Expand All @@ -169,7 +169,7 @@ id! {
}

/// Same as `info::InstantiateModule`
#[allow(missing_docs)]
#[allow(missing_docs, reason = "tedious to document variants")]
pub enum Instance {
Static(StaticModuleIndex, Box<[CoreDef]>),
Import(
Expand All @@ -179,7 +179,7 @@ pub enum Instance {
}

/// Same as `info::Export`
#[allow(missing_docs)]
#[allow(missing_docs, reason = "tedious to document variants")]
pub enum Export {
LiftedFunction {
ty: TypeFuncIndex,
Expand All @@ -203,7 +203,7 @@ pub enum Export {

/// Same as `info::CoreDef`, except has an extra `Adapter` variant.
#[derive(Debug, Clone, Hash, Eq, PartialEq)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "tedious to document variants")]
pub enum CoreDef {
Export(CoreExport<EntityIndex>),
InstanceFlags(RuntimeComponentInstanceIndex),
Expand All @@ -230,14 +230,14 @@ where

/// Same as `info::CoreExport`
#[derive(Debug, Clone, Hash, Eq, PartialEq)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct CoreExport<T> {
pub instance: InstanceId,
pub item: ExportItem<T>,
}

impl<T> CoreExport<T> {
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing function")]
pub fn map_index<U>(self, f: impl FnOnce(T) -> U) -> CoreExport<U> {
CoreExport {
instance: self.instance,
Expand All @@ -251,7 +251,7 @@ impl<T> CoreExport<T> {

/// Same as `info::Trampoline`
#[derive(Clone, PartialEq, Eq, Hash)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub enum Trampoline {
LowerImport {
import: RuntimeImportIndex,
Expand All @@ -277,7 +277,7 @@ pub enum Trampoline {

/// Same as `info::CanonicalOptions`
#[derive(Clone, Hash, Eq, PartialEq)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct CanonicalOptions {
pub instance: RuntimeComponentInstanceIndex,
pub string_encoding: StringEncoding,
Expand All @@ -287,7 +287,7 @@ pub struct CanonicalOptions {
}

/// Same as `info::Resource`
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing fields")]
pub struct Resource {
pub rep: WasmValType,
pub dtor: Option<CoreDef>,
Expand Down
6 changes: 3 additions & 3 deletions crates/environ/src/component/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ pub struct CanonicalOptions {
// `extern "C" fn()` function argument which is called from cranelift-compiled
// code so we must know the representation of this.
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing variants")]
#[repr(u8)]
pub enum StringEncoding {
Utf8,
Expand All @@ -469,7 +469,7 @@ pub enum StringEncoding {
///
/// Note that each transcoding operation may have a unique signature depending
/// on the precise operation.
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing variants")]
#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
pub enum Transcode {
Copy(FixedEncoding),
Expand Down Expand Up @@ -525,7 +525,7 @@ impl Transcode {
}

#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, Serialize, Deserialize)]
#[allow(missing_docs)]
#[allow(missing_docs, reason = "self-describing variants")]
pub enum FixedEncoding {
Utf8,
Utf16,
Expand Down
Loading

0 comments on commit 45b60bd

Please sign in to comment.