fix(multitude): gate over-aligned doctest behind utc_backend cfg - #650
fix(multitude): gate over-aligned doctest behind utc_backend cfg#650Kateřina Churanová (kate-shine) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Restores the repository’s root rust-toolchain.toml so clean checkouts build/test under the intended pinned LLVM-backed Rust toolchain, avoiding failures when a developer’s default rustup toolchain uses the Microsoft UTC backend (with its 8192B alignment cap that breaks multitude’s intentionally over-aligned tests).
Changes:
- Reintroduce the workspace toolchain pin at
channel = "1.96.1"(matchesRUST_LATESTinconstants.env). - Ensure
clippyandrustfmtare installed for the pinned toolchain (sorustupusers on theminimalprofile can still runjust clippy/just format). - Restore the previously-pinned additional compilation targets.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9f43233 to
90599ca
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #650 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 473 473
Lines 45493 45493
=======================================
Hits 45493 45493
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| [toolchain] | ||
| # This needs to match RUST_LATEST in constants.env. | ||
| channel = "1.96.1" |
There was a problem hiding this comment.
🤖 This new pin is required to match RUST_LATEST, but scripts/update_rust_toolchain.ps1 only reads and rewrites constants.env. The next stable update will advance CI while leaving clean local checkouts on the old toolchain. Please update rust-toolchain.toml atomically in that script and adjust its README description.
The doctest on AllocError::is_alignment_too_large declares a type with repr(align(32768)) to trigger the alignment-too-large rejection. Codegen backends that support a lower maximum type alignment cannot compile it, so cargo test --doc failed there. #501 gated the equivalent integration tests behind cfg(utc_backend) but missed this doctest. Wrap the body in the hidden cfg shim documented in AGENTS-feature-gated-doctests.md so it compiles in both configurations. Note that doctests are compiled by rustdoc, so enabling the gate requires RUSTDOCFLAGS in addition to RUSTFLAGS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
90599ca to
d25a7f5
Compare
Ralf Biedert (ralfbiedert)
left a comment
There was a problem hiding this comment.
🤖 Approved.
Ralf Biedert (ralfbiedert)
left a comment
There was a problem hiding this comment.
🤖 Reviewed exact head d25a7f5. The cfg polarity is correct, normal backends continue executing the doctest, and the UTC backend avoids compiling the unsupported over-aligned declaration. I found no blocking correctness, performance, or unsafe-context issues.
Problem
cargo test -p multitude --docfails on codegen backends that support a lower maximum type alignment than the default one.The doctest on
AllocError::is_alignment_too_largehas to declare an over-aligned type in order to demonstrate the error it documents:On a backend that caps alignment below that value, the type cannot be compiled at all, so the doctest fails.
#501 gated the equivalent integration tests behind
cfg(utc_backend)and registered that cfg in the workspacecheck-cfglist, but this doctest was missed.Fix
Wrap the body in the hidden
#[cfg(...)]shim documented inAGENTS-feature-gated-doctests.md(Pattern C — no?, no usermain):The
#prefix keeps the shim out of the rendered documentation, so the published example is unchanged.Note for reviewers:
RUSTDOCFLAGSis requiredDoctests are compiled by
rustdoc, and cargo passesRUSTFLAGStorustcbut not torustdoc. Enabling the gate for doctests therefore needs both:Verified: with
RUSTFLAGSalone the gate compiles in but the doctest still fails, because rustdoc never sees the cfg. Anything driving this build (for example a publish pipeline that currently sets onlyRUSTFLAGS) needsRUSTDOCFLAGSas well, otherwise this fix will appear to have no effect.Verification
cargo clippy -p multitude --profile dev --all-targets --all-features --locked— clean, zero warnings.just package=multitude format-check— clean.just spellcheck— clean.Confirmed separately that the
multitudelibrary builds cleanly on such a backend and its 46 unit tests pass. The library declares no over-aligned types; alignment is handled at runtime viaalign_of::<T>(). Only test and doc code that must construct an over-aligned type is affected.AB#7707893