Doctest fixes (from #139) + unblock i686 cross-tests + thumbv6m nostd build#141
Merged
Merged
Conversation
1. crystal_encoder::distill — remove unused import `Role` 2. nibble::nibble_propagate_bfs — remove unused import `nibble_unpack` 3. udf_kernels::udf_sigma_classify — swap band/threshold args in doctest assertions (distance=100 with threshold=1000 → Foveal, not distance=1000 with threshold=100 → Reject) All 3 fail under CI's RUSTFLAGS="-D warnings". Now: 335 doctests pass, 0 fail, 7 ignored. Co-authored-by: AdaWorldAPI <AdaWorldAPI@users.noreply.github.com>
dtolnay/rust-toolchain installs the target for the matrix toolchain (stable), but rust-toolchain.toml overrides cargo to use 1.94.1 which doesn't have thumbv6m-none-eabi installed. Add explicit rustup target add to ensure the pinned toolchain has the cross-compile target. Co-authored-by: AdaWorldAPI <AdaWorldAPI@users.noreply.github.com>
Two pre-existing latent CI failures surfaced on PR #139 that are NOT introduced by it but block the same merge gate: (1) i686-unknown-linux-gnu cross-tests — duplicate scalar mask types The `impl_float_type!` macro at src/simd.rs:562 already emits `pub struct $mask(pub $mask_prim);` from its `mask_prim` parameter, so calling it with `F32Mask8Scalar` / `F64Mask4Scalar` defines those structs. Lines 768-771 had explicit re-declarations of the same structs ("Unused mask types for AVX2 scalars" comment notwithstanding — the macro had been emitting them since PR #49). On x86_64 the entire scalar module is `#[cfg(not(target_arch = "x86_64"))]` so the duplication is invisible. On i686 (32-bit x86) the cfg matches → both definitions compile → 8 errors: E0428 × 2 (duplicate type names) E0119 × 6 (conflicting Copy / Clone / Debug derive impls) Fix: delete the 4 explicit re-declarations. Macro continues to emit both mask types as before; the scalar module compiles cleanly on i686. (2) thumbv6m-none-eabi nostd build — blake3 transitive dep pulls std `cargo rustc --target=thumbv6m-none-eabi --no-default-features --features portable-atomic-critical-section` failed with: error[E0463]: can't find crate for `std` note: `std` is required by `constant_time_eq` because it does not declare `#![no_std]` Root cause: `blake3` was declared unconditional in Cargo.toml (`blake3 = { version = "1" }`) — pulled into every build including `--no-default-features`. blake3 itself supports no_std but its transitive dep `constant_time_eq` does not declare `#![no_std]`, so the dep tree pulls std unconditionally. The Cargo.toml comment on the `hpc-extras` feature already documents "blake3 hashing" as part of that feature group; fix is to make the declaration match the documented intent: - `blake3 = { version = "1", optional = true }` - Add `dep:blake3` to the `hpc-extras` feature list `hpc-extras` is in `default = ["std", "hpc-extras"]`, so all default builds still get blake3 (zero behavior change). Only `--no-default- features` builds drop it — which is exactly what the thumbv6m nostd job runs. The 8 hpc/* files that use blake3 (plane, seal, merkle_tree, vsa, spo_bundle, crystal_encoder, compression_curves, deepnsm) need no cfg gating — the existing module-level cfg conditions already exclude them from the no_std build. Verification (local, 2026-05-13): - `cargo check --no-default-features --features portable-atomic-critical-section --lib` clean - `cargo clippy --no-deps` (default features) clean - Standard build unaffected (default includes hpc-extras → blake3 still in dep tree) Both fixes are pre-existing latent bugs (simd duplicates since PR #49, blake3 unconditional since the workspace was set up); PR #139 exposes them via the new CI matrix entries (cross-tests + nostd).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f3a43f372
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # burn-ndarray that only need the core array layer can disable this with | ||
| # `default-features = false` (and re-enable `std` explicitly if needed). | ||
| hpc-extras = ["std", "dep:p64", "dep:fractal", "fractal/std"] | ||
| hpc-extras = ["std", "dep:p64", "dep:fractal", "fractal/std", "dep:blake3"] |
There was a problem hiding this comment.
When consumers use the documented configuration default-features = false and re-enable std, this line leaves blake3 disabled even though src/lib.rs still compiles pub mod hpc under feature = "std", and src/hpc/mod.rs unconditionally exposes plane, seal, merkle_tree, and vsa. Those modules contain direct blake3::... references, so cargo check --no-default-features --features std fails with unresolved crate errors; either std needs to enable dep:blake3 or those modules need matching cfg gates.
Useful? React with 👍 / 👎.
…rray + rustfmt Three additional CI failures surfaced by PR #141 (all pre-existing latent issues, none caused by the original migration): (1) s390x-unknown-linux-gnu cross-tests — x86-only inline asm leaks `src/hpc/amx_matmul.rs` + `src/hpc/bf16_tile_gemm.rs` (its wrapper) + `src/simd_amx.rs` all use `asm!` with `rcx` / `rax` register names. AMX is an Intel-only ISA (Sapphire Rapids+); the registers don't exist on s390x / aarch64 / wasm32 / etc. and the asm parser rejects them at compile time. Fix: gate all three module declarations behind `#[cfg(target_arch = "x86_64")]`. On x86_64 CI runners (most) they compile normally and runtime gating via `amx_available()` already prevents execution on CPUs without AMX. On non-x86 targets they're skipped entirely. External consumer audit clean — only `bf16_tile_gemm` uses `amx_matmul`, and only `amx_matmul` uses `simd_amx`. No cascade gating needed. (2) thumbv6m-none-eabi nostd — criterion dev-dep tree leaks into workspace cross-build CI runs `cargo rustc --target=thumbv6m-none-eabi --no-default-features --features portable-atomic-critical-section` from the workspace root. Without `-p ndarray` scoping, cargo evaluates the whole workspace's dep graph (including dev-deps from ndarray-rand / serialization-tests / numeric-tests). The bench migration in PR #140 added `criterion 0.5` as a dev-dep; criterion transitively pulls `serde_core` (which doesn't declare `#![no_std]`) and `getrandom` (which has its own no_std-incompatible paths) into the dep tree. The library `ndarray` itself builds cleanly on thumbv6m no-default- features (verified: `cargo check -p ndarray --target=thumbv6m-none-eabi --no-default-features --features portable-atomic-critical-section` is clean). The CI command just needs scoping. Fix: add `-p ndarray` to the cargo rustc invocation in the nostd CI job so dev-dep evaluation is limited to the library's own deps. (3) cargo fmt --all --check failures Bench files migrated to criterion in PR #140 used the workspace's prior-style "brace on next line" formatting (`fn foo(c: &mut Criterion) \n{`). Stable rustfmt 1.94.1 (pinned per CLAUDE.md) wants "brace on same line" (`fn foo(c: &mut Criterion) {`). Plus single-statement closures inlined. Fix: run `cargo fmt --all` (no manual changes needed). 13 bench files + examples/life.rs touched with mechanical formatting changes only; no semantic changes. Verification (local, 2026-05-13): - `cargo clippy --no-deps` (default) → clean - `cargo check --no-default-features --features portable-atomic-critical-section --lib` → clean - `cargo rustc -p ndarray --target=thumbv6m-none-eabi --no-default-features --features portable-atomic-critical-section` → clean (CI command as updated) - `cargo fmt --all --check` → clean Pre-existing latent bugs (amx asm registers since the module was added, criterion dev-dep regression introduced by PR #140 itself); fixes land on the same PR series that exposed them.
i686-unknown-linux-gnu cross-tests (which run under -D warnings) failed:
error: variants `Avx512` and `Avx2` are never constructed
--> src/backend/native.rs:17:5
= note: `-D dead-code` implied by `-D warnings`
Two `enum Tier { Avx512, Avx2, ..., Scalar }` definitions exist, one in
`src/backend/native.rs` and one in `src/simd.rs`. Both `detect_tier()`
blocks gate the AVX-detection paths behind `#[cfg(target_arch =
"x86_64")]`; the NEON-detection (simd.rs only) is gated behind
`#[cfg(target_arch = "aarch64")]`. On i686 / wasm32 / etc., none of
those gates match → only `Scalar` is ever constructed → dead-code
lint fires on the other variants.
Fix: `#[allow(dead_code)]` on both Tier enums. The variants ARE needed
on x86_64 and aarch64 builds (the cfg blocks produce them via runtime
feature detection); they just don't reach instantiation under the i686
target_arch.
The src/simd.rs fix is preemptive — only src/backend/native.rs was
flagged in the CI error, but src/simd.rs has 4 non-Scalar variants
(Avx512, Avx2, NeonDotProd, Neon) that would trigger the same lint
under -D warnings on i686. Both fixed in one commit.
Verification:
- `cargo clippy --no-deps` (default x86_64) → clean (the variants ARE
constructed via `is_x86_feature_detected!("avx512f")`)
- `RUSTFLAGS="-D dead_code" cargo check --lib --features ...` → clean
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supersedes #139 — same 2 original cursor-agent commits (doctest fixes + thumbv6m target install) plus 1 fix commit that unblocks two pre-existing latent CI failures #139 surfaced but didn't introduce. Could not push directly to
cursor/fix-doctests-9e6ddue to proxy policy; opening here instead.Commits
7aa39a3dfix(doctests): 3 doctest failures (crystal_encoder::distillunusedRole,nibble::nibble_propagate_bfsunusednibble_unpack,udf_kernels::udf_sigma_classifyarg swap)0594db33fix(ci): installthumbv6m-none-eabitarget for pinned 1.94.1 toolchainea7beeb5Merge origin/master(rebases over #140 series)5f3a43f3fix(ci): simd duplicate scalar mask types + blake3 nostd gatingWhat
5f3a43f3fixes(1) i686-unknown-linux-gnu cross-tests — duplicate scalar mask types
src/simd.rshad explicitpub struct F32Mask8Scalar(pub u8);+pub struct F64Mask4Scalar(pub u8);re-declarations belowimpl_float_type!(F32x8, ..., F32Mask8Scalar, u8)andimpl_float_type!(F64x4, ..., F64Mask4Scalar, u8). The macro at line 562 already emitspub struct $mask(pub $mask_prim);from themask_primparameter — it's been emitting them since PR #49. The explicit re-declarations were dead duplicates.On x86_64 the entire scalar module is
#[cfg(not(target_arch = "x86_64"))]→ invisible. On i686 (32-bit x86) the cfg matches → both definitions compile → 8 errors:E0428 × 2(duplicate type names)E0119 × 6(conflictingCopy/Clone/Debugderive impls)Fix: delete the 4 explicit re-declaration lines.
(2) thumbv6m-none-eabi nostd build — blake3 transitive dep pulls std
cargo rustc --target=thumbv6m-none-eabi --no-default-features --features portable-atomic-critical-sectionfailed with:Root cause:
blake3 = { version = "1" }in Cargo.toml — unconditional, pulled into every build. blake3 itself supports no_std butconstant_time_eq(its transitive dep) doesn't declare#![no_std].Fix: make blake3 optional + add
dep:blake3to thehpc-extrasfeature (which the Cargo.toml comment already documents as the blake3 home: "HPC extras: blake3 hashing, p64 palette/NARS bridge, fractal manifold"). Sincedefault = ["std", "hpc-extras"], all default builds still get blake3 unchanged. Only--no-default-featuresdrops it — exactly what the thumbv6m nostd job runs.The 8 hpc/* files using blake3 (plane/seal/merkle_tree/vsa/spo_bundle/crystal_encoder/compression_curves/deepnsm) need no cfg gating — the existing module-level cfg conditions already exclude them from the no_std build.
Verification (local, 2026-05-13)
cargo clippy --no-deps(default features) → cleancargo check --no-default-features --features portable-atomic-critical-section --lib→ cleancargo check --target=thumbv6m-none-eabi --no-default-features --features portable-atomic-critical-section→ cleancargo check --target=i686-unknown-linux-gnugets past source compilation (the simd duplicates error is gone); blake3's C build script is the next step and needsgcc-multilibwhich CI has but my local doesn'tAction for #139
This PR can be merged as-is and #139 closed (its 2 commits are included verbatim with attribution preserved via the original SHAs in the merge history).
Both fixes are pre-existing latent bugs (simd duplicates since PR #49, blake3 unconditional since the workspace was set up); they're surfaced by the new CI matrix entries (cross-tests + nostd) added recently. PR #139 was the canary, not the cause.
https://claude.ai/code
Generated by Claude Code