Skip to content

Commit f265209

Browse files
authored
chore: silence clippy 1.95 lint regressions across workspace (#18)
* chore: silence clippy 1.95 lint regressions across workspace Rust 1.95.0 promoted several lints to default-warn (and CI runs with `-D warnings`), causing a cascade of build failures across mlxcel-core and mlxcel-surgery that had nothing to do with their underlying logic. This commit makes the workspace lint-clean again under the new toolchain. Auto-fixed via `cargo clippy --fix`: - `manual_is_multiple_of` → `.is_multiple_of()` (pack3, quant3, utils) - `manual_div_ceil` → `.div_ceil()` (utils) - `unnecessary_cast` (quant, codebook, generate, etc.) - `useless_conversion` (generate) - `double_parens` (mtp/tests) Manually rewritten: - `needless_range_loop` → iterator + take/enumerate where the conversion was clear (utils, quant, quant3, sampling, boundary, turbo_tests). - `field_reassign_with_default` → struct-init form (generate, speculative). - `doc_overindented_list_items` → list-continuation indent normalised (speculative/mtp/generator). - `doc_lazy_continuation` → reworded the `softmax + cold-V` paragraph in turbo_tests so a leading `+` no longer parses as a markdown list marker. - `unusual_byte_groupings` → `0xC0FF_EE42` (pack3, quant). - `duplicated_attribute` → drop inner `#![cfg(test)]` where the parent module already gates with `#[cfg(test)]` (mlxcel-surgery). - Added a `# Safety` section to `attention_from_ptr` (layers). Targeted `#[allow(...)]` (intentional shape, not worth restructuring): - `too_many_arguments` on the attention-dispatch helpers in `layers.rs` and the `attention_sparse_v_turbo4{,_fused}` kernels: positional signatures intentionally mirror Metal/MLX call sites. - `needless_range_loop` on `forward_batched`: mixed indexing into `input_ids` slices and `batch_caches[i]` is clearer than enumerate. - `large_enum_variant` / `result_large_err` on the paged-cache adopt path: shrinking the variant would force callers through `Box` for no runtime gain on a cold path. - `type_complexity` on the `SyntheticTarget` test fixture's `rollback_events`: it is a one-off test type. `cargo clippy --workspace --all-targets -- -D warnings` is clean and `cargo test --workspace` matches the pre-existing baseline (the same two `multimodal::video` fd-variant tests fail on `main` too, due to runtime ffmpeg policy mismatches unrelated to this change). * chore: silence two more clippy errors gated by metal,accelerate features The earlier sweep ran without `--features metal,accelerate`, so the CI build (which uses that exact feature set) still tripped on two finds: - `quant.rs:422` unnecessary `as usize` cast on an already-usize index - `generate.rs:826` explicit `.into_iter()` inside `zip(...)` which already accepts any `IntoIterator` `cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings` is now clean. * chore: drop another useless into_iter in speculative_burst zip server::batch::speculative_burst::zip already accepts IntoIterator, so the explicit `.into_iter()` on rows_tokens is redundant. Missed in the prior sweep because the local check ran against a cached clippy result where this file was unchanged; `cargo clean` followed by a fresh `cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings` confirms the workspace is now clean.
1 parent a84284e commit f265209

18 files changed

Lines changed: 67 additions & 56 deletions

File tree

src/lib/mlxcel-core/src/cache/paged_detach.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ impl Drop for DetachedPagedCacheSet {
258258

259259
/// Internal parked-cache representation. Used by the pool so both dense and
260260
/// paged detached sets share a single [`DetachedHandle`] space.
261+
#[allow(clippy::large_enum_variant)]
261262
pub(super) enum ParkedCache {
262263
Dense(super::detach::DetachedCacheSet),
263264
Paged(DetachedPagedCacheSet),
@@ -434,6 +435,7 @@ impl CachePool {
434435
///
435436
/// Retained block pins are preserved across failure paths so the caller
436437
/// can still call [`CachePool::release_detached_paged`] manually or retry.
438+
#[allow(clippy::result_large_err)]
437439
pub fn adopt_paged_preserving(
438440
&mut self,
439441
model: &dyn crate::generate::LanguageModel,

src/lib/mlxcel-core/src/cache/turbo/boundary.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,9 +373,9 @@ mod tests {
373373
// Cross-check the single-layer helper against the bulk helper.
374374
let n = 16;
375375
let bulk = resolve_layer_modes(KVCacheMode::Turbo4Asym, n, 2);
376-
for i in 0..n {
376+
for (i, &expected) in bulk.iter().enumerate().take(n) {
377377
let single = resolve_layer_mode(KVCacheMode::Turbo4Asym, i, n, 2);
378-
assert_eq!(bulk[i], single, "layer {i} mismatch");
378+
assert_eq!(expected, single, "layer {i} mismatch");
379379
}
380380
}
381381
}

src/lib/mlxcel-core/src/cache/turbo/codebook.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ mod tests {
507507

508508
// Φ(−1) ≈ 0.15865525393145702
509509
let v = gaussian_cdf(-1.0);
510-
assert!((v - 0.158_655_253_931_457_0).abs() < 1e-12, "Φ(-1) = {v}");
510+
assert!((v - 0.158_655_253_931_457).abs() < 1e-12, "Φ(-1) = {v}");
511511
}
512512

513513
#[test]

src/lib/mlxcel-core/src/cache/turbo/pack3.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub const V_BIT_WIDTH_3: u8 = 3;
8787
#[inline]
8888
pub fn packed_bytes_per_token_3bit(head_dim: i32) -> i32 {
8989
debug_assert!(
90-
head_dim > 0 && (head_dim as usize) % COORDS_PER_GROUP == 0,
90+
head_dim > 0 && (head_dim as usize).is_multiple_of(COORDS_PER_GROUP),
9191
"packed_bytes_per_token_3bit: head_dim must be a positive multiple of 8; \
9292
got {head_dim}"
9393
);
@@ -109,7 +109,7 @@ pub fn packed_bytes_per_token_3bit(head_dim: i32) -> i32 {
109109
/// pipeline).
110110
pub fn pack_3bit_indices(indices: &[u8], out: &mut [u8]) {
111111
debug_assert!(
112-
indices.len() % COORDS_PER_GROUP == 0,
112+
indices.len().is_multiple_of(COORDS_PER_GROUP),
113113
"pack_3bit_indices: index count ({}) must be a multiple of {}",
114114
indices.len(),
115115
COORDS_PER_GROUP
@@ -147,7 +147,7 @@ pub fn pack_3bit_indices(indices: &[u8], out: &mut [u8]) {
147147
/// dequant path).
148148
pub fn unpack_3bit_indices(packed: &[u8], out: &mut [u8]) {
149149
debug_assert!(
150-
packed.len() % BYTES_PER_GROUP == 0,
150+
packed.len().is_multiple_of(BYTES_PER_GROUP),
151151
"unpack_3bit_indices: packed byte count ({}) must be a multiple of {}",
152152
packed.len(),
153153
BYTES_PER_GROUP
@@ -311,7 +311,7 @@ mod tests {
311311
#[test]
312312
fn round_trip_random_multi_group() {
313313
// 32 indices = 4 groups = 12 bytes packed. Use a deterministic LCG.
314-
let mut state: u32 = 0xC0FFEE_42;
314+
let mut state: u32 = 0xC0FF_EE42;
315315
let mut indices = vec![0u8; 32];
316316
for x in &mut indices {
317317
state = state.wrapping_mul(1_664_525).wrapping_add(1_013_904_223);

src/lib/mlxcel-core/src/cache/turbo/quant.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ fn quantize_into_packed(
419419
let idx_off = tok * coords_per_token;
420420
let mut sum_sq = 0.0_f32;
421421
for d_i in 0..coords_per_token {
422-
let c = centroids[indices[idx_off + d_i] as usize];
422+
let c = centroids[indices[idx_off + d_i]];
423423
sum_sq += c * c;
424424
}
425425
let y_hat_norm = sum_sq.sqrt();
@@ -874,8 +874,7 @@ mod tests {
874874
let hd_usize = head_dim as usize;
875875
let mut k_data: Vec<f32> = Vec::with_capacity(n_tokens * hd_usize);
876876
let token_scales = [0.5_f32, 1.5, 4.0, 12.0];
877-
for tok in 0..n_tokens {
878-
let scale = token_scales[tok];
877+
for &scale in token_scales.iter().take(n_tokens) {
879878
for _ in 0..head_dim {
880879
let mut acc = 0.0_f32;
881880
for _ in 0..6 {
@@ -981,8 +980,7 @@ mod tests {
981980
// norm storage, which is acceptable in production but would noise
982981
// up this test.
983982
let token_scales = [0.5_f32, 1.5, 4.0, 12.0];
984-
for tok in 0..4 {
985-
let scale = token_scales[tok];
983+
for &scale in &token_scales {
986984
for _ in 0..head_dim {
987985
// Pseudo-Gaussian via summed uniform bits → ~normal in [-1, 1].
988986
let mut acc = 0.0_f32;
@@ -1057,7 +1055,7 @@ mod tests {
10571055
let head_dim: i32 = 128;
10581056
let params = TurboQuantParams::new(head_dim as u32, 0xC1_DECAFu32);
10591057

1060-
let mut rng = Lcg32::new(0xC0FFEE_42);
1058+
let mut rng = Lcg32::new(0xC0FF_EE42);
10611059
let n_tokens = 4usize;
10621060
let hd_usize = head_dim as usize;
10631061
let mut v_data: Vec<f32> = Vec::with_capacity(n_tokens * hd_usize);

src/lib/mlxcel-core/src/cache/turbo/quant3.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ impl TurboQuantParams3 {
111111
"TurboQuantParams3::new: head_dim must be a non-zero power of two; got {head_dim}"
112112
);
113113
assert!(
114-
head_dim % 8 == 0,
114+
head_dim.is_multiple_of(8),
115115
"TurboQuantParams3::new: head_dim must be a multiple of 8 \
116116
for the 24-bit packing layout; got {head_dim}"
117117
);
@@ -373,8 +373,7 @@ mod tests {
373373
let mut state: u32 = 123;
374374
let mut v_data: Vec<f32> = Vec::with_capacity(4 * head_dim as usize);
375375
let token_scales = [0.5_f32, 1.5, 4.0, 12.0];
376-
for tok in 0..4 {
377-
let scale = token_scales[tok];
376+
for &scale in &token_scales {
378377
for _ in 0..head_dim {
379378
let mut acc = 0.0_f32;
380379
for _ in 0..6 {

src/lib/mlxcel-core/src/cache/turbo/sparse_v.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ pub fn compute_alive_mask(
380380
/// Used by: tests under `cache/turbo_tests.rs` (issue #480 unit tests).
381381
/// Production attention call sites continue to use the standard
382382
/// `attention()` path until the Metal kernel lands.
383+
#[allow(clippy::too_many_arguments)]
383384
pub fn attention_sparse_v_turbo4(
384385
q: &MlxArray,
385386
k: &MlxArray,
@@ -526,6 +527,7 @@ pub fn attention_sparse_v_turbo4(
526527
/// case.
527528
///
528529
/// Used by: [`KVCache::sparse_v_attention`] (issue #505).
530+
#[allow(clippy::too_many_arguments)]
529531
pub fn attention_sparse_v_turbo4_fused(
530532
q: &MlxArray,
531533
k: &MlxArray,

src/lib/mlxcel-core/src/cache/turbo_tests.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,8 @@ fn turbo4_asym_clone_handle_install_then_dequant_matches_pre_detach() {
364364
let v_data: Vec<f32> = (0..2 * head_dim)
365365
.map(|i| (((i * 7) % 13) as f32 / 13.0) - 0.5)
366366
.collect();
367-
let k = ffi::from_slice_f32(&k_data, &[1, 1, 2, head_dim as i32]);
368-
let v = ffi::from_slice_f32(&v_data, &[1, 1, 2, head_dim as i32]);
367+
let k = ffi::from_slice_f32(&k_data, &[1, 1, 2, head_dim]);
368+
let v = ffi::from_slice_f32(&v_data, &[1, 1, 2, head_dim]);
369369

370370
let (_k1, v1_out) = cache.update_and_fetch(k, v);
371371
let v1 = flatten_fp32(&v1_out);
@@ -901,13 +901,13 @@ mod boundary_v {
901901
fn typical_32_layer_split_protects_first_two_and_last_two() {
902902
let n = 32;
903903
let modes = resolve_layer_modes(KVCacheMode::Turbo4Asym, n, 2);
904-
for i in 0..n {
904+
for (i, &actual) in modes.iter().enumerate().take(n) {
905905
let expected = if i < 2 || i >= n - 2 {
906906
KVCacheMode::Fp16
907907
} else {
908908
KVCacheMode::Turbo4Asym
909909
};
910-
assert_eq!(modes[i], expected, "layer {i}");
910+
assert_eq!(actual, expected, "layer {i}");
911911
}
912912
}
913913

@@ -922,10 +922,10 @@ mod boundary_v {
922922
KVCacheMode::Turbo4Delegated,
923923
] {
924924
let bulk = resolve_layer_modes(mode, n, 2);
925-
for i in 0..n {
925+
for (i, &expected) in bulk.iter().enumerate().take(n) {
926926
let single = resolve_layer_mode(mode, i, n, 2);
927927
assert_eq!(
928-
bulk[i], single,
928+
expected, single,
929929
"{mode:?} layer {i}: bulk vs single helper disagree"
930930
);
931931
}
@@ -1414,8 +1414,8 @@ fn turbo3_asym_clone_handle_install_then_dequant_matches_pre_detach() {
14141414
let v_data: Vec<f32> = (0..2 * head_dim)
14151415
.map(|i| (((i * 7) % 13) as f32 / 13.0) - 0.5)
14161416
.collect();
1417-
let k = ffi::from_slice_f32(&k_data, &[1, 1, 2, head_dim as i32]);
1418-
let v = ffi::from_slice_f32(&v_data, &[1, 1, 2, head_dim as i32]);
1417+
let k = ffi::from_slice_f32(&k_data, &[1, 1, 2, head_dim]);
1418+
let v = ffi::from_slice_f32(&v_data, &[1, 1, 2, head_dim]);
14191419

14201420
let (_k1, v1_out) = cache.update_and_fetch(k, v);
14211421
let v1 = flatten_fp32(&v1_out);
@@ -1667,7 +1667,7 @@ fn turbo3_asym_layer_modes_apply_boundary_upgrade() {
16671667
assert_eq!(modes.len(), 8);
16681668
// First 2 + last 2 are upgraded to FP16; middle 4 stay Turbo3Asym.
16691669
for (i, m) in modes.iter().enumerate() {
1670-
if i < 2 || i >= 6 {
1670+
if !(2..6).contains(&i) {
16711671
assert_eq!(*m, KVCacheMode::Fp16, "layer {i} must be FP16 boundary");
16721672
} else {
16731673
assert_eq!(
@@ -2608,8 +2608,8 @@ fn delegated_dequant_sdpa_matches_reference_attention() {
26082608
/// Steel-envelope vs cold-only fused composition parity (issue #531).
26092609
///
26102610
/// `update_and_turbo4_delegated_attention` first tries the steel-envelope
2611-
/// kernel (issue #531, single Metal dispatch covering softmax + cold-V dequant
2612-
/// + hot-V accumulate). On the same hardware where the cold-only fused
2611+
/// kernel (issue #531, single Metal dispatch covering softmax, cold-V dequant,
2612+
/// and hot-V accumulate). On the same hardware where the cold-only fused
26132613
/// composition path (issue #528) already produces correct output, the
26142614
/// steel-envelope path must produce numerically equivalent output (within
26152615
/// FP16 round-off) — both paths funnel through the same MLX softmax algebra

src/lib/mlxcel-core/src/drafter/dflash/round_loop_batched.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@ mod tests {
714714
///
715715
/// `argmax_fn(row, position, prev_token) -> next_token`. Tests seed
716716
/// this so the accept pattern is deterministic.
717+
#[allow(clippy::type_complexity)]
717718
struct SyntheticTarget {
718719
argmax_fn: Box<SyntheticArgmaxFn>,
719720
concat_hidden_dim: i32,

src/lib/mlxcel-core/src/generate.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,7 @@ pub trait LanguageModel {
411411
/// amortizes weight-loading bandwidth across the batch.
412412
///
413413
/// Used by: BatchScheduler (server continuous batching)
414+
#[allow(clippy::needless_range_loop)]
414415
fn forward_batched(
415416
&self,
416417
input_ids: &MlxArray,
@@ -822,7 +823,7 @@ impl CxxGenerator {
822823
let n_layers = self.caches.len();
823824
let requested = crate::cache::turbo::boundary_v_layers_from_env();
824825
let layer_modes = crate::cache::turbo::resolve_layer_modes(nominal, n_layers, requested);
825-
for (cache, mode) in self.caches.iter_mut().zip(layer_modes.into_iter()) {
826+
for (cache, mode) in self.caches.iter_mut().zip(layer_modes) {
826827
cache.mode = mode;
827828
}
828829
}
@@ -2096,7 +2097,7 @@ mod tests {
20962097
// n=0 is the very first decode iteration after prefill. Clearing here
20972098
// would discard tensors needed for the pipelined next-step computation.
20982099
let n = 0_usize;
2099-
assert!(!(n % 256 == 0 && n > 0));
2100+
assert!(!(n.is_multiple_of(256) && n > 0));
21002101
}
21012102

21022103
#[test]
@@ -2259,8 +2260,10 @@ mod tests {
22592260
let caller_bias = make_bias(&[(99, -3.0)]);
22602261
let g = CxxGenerator::new(2).with_token_bias(cached);
22612262

2262-
let mut caller = SamplingConfig::default();
2263-
caller.token_bias = caller_bias;
2263+
let caller = SamplingConfig {
2264+
token_bias: caller_bias,
2265+
..SamplingConfig::default()
2266+
};
22642267
let composed = g.compose_sampling(&caller);
22652268

22662269
// Caller's explicit bias is preserved verbatim, cached bias is ignored.

0 commit comments

Comments
 (0)