Skip to content

fix(multitude): gate over-aligned doctest behind utc_backend cfg - #650

Open
Kateřina Churanová (kate-shine) wants to merge 1 commit into
mainfrom
kchuranov/fix-multitude-over-aligned-tests
Open

fix(multitude): gate over-aligned doctest behind utc_backend cfg#650
Kateřina Churanová (kate-shine) wants to merge 1 commit into
mainfrom
kchuranov/fix-multitude-over-aligned-tests

Conversation

@kate-shine

@kate-shine Kateřina Churanová (kate-shine) commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Problem

cargo test -p multitude --doc fails on codegen backends that support a lower maximum type alignment than the default one.

The doctest on AllocError::is_alignment_too_large has to declare an over-aligned type in order to demonstrate the error it documents:

#[repr(align(32768))]
struct OverAligned;

let arena = multitude::Arena::new();
let Some(error) = arena.try_alloc(OverAligned).err() else {
    panic!("over-aligned values must be rejected");
};
assert!(error.is_alignment_too_large());

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 workspace check-cfg list, but this doctest was missed.

Fix

Wrap the body in the hidden #[cfg(...)] shim documented in AGENTS-feature-gated-doctests.md (Pattern C — no ?, no user main):

/// # fn main() {
/// # #[cfg(not(utc_backend))] {
/// ... existing body, unchanged ...
/// # }
/// # }

The # prefix keeps the shim out of the rendered documentation, so the published example is unchanged.

Note for reviewers: RUSTDOCFLAGS is required

Doctests are compiled by rustdoc, and cargo passes RUSTFLAGS to rustc but not to rustdoc. Enabling the gate for doctests therefore needs both:

RUSTFLAGS=--cfg utc_backend
RUSTDOCFLAGS=--cfg utc_backend

Verified: with RUSTFLAGS alone 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 only RUSTFLAGS) needs RUSTDOCFLAGS as well, otherwise this fix will appear to have no effect.

Verification

  • Doctests with the gate set (both env vars): 301 passed, 0 failed — previously 1 failed.
  • Doctests without the gate, on a normal toolchain: 301 passed, 0 failed — the example still genuinely compiles and asserts, it is not silently skipped.
  • 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 multitude library builds cleanly on such a backend and its 46 unit tests pass. The library declares no over-aligned types; alignment is handled at runtime via align_of::<T>(). Only test and doc code that must construct an over-aligned type is affected.

AB#7707893

Copilot AI lite review requested due to automatic review settings August 7, 2026 09:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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" (matches RUST_LATEST in constants.env).
  • Ensure clippy and rustfmt are installed for the pinned toolchain (so rustup users on the minimal profile can still run just clippy / just format).
  • Restore the previously-pinned additional compilation targets.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings August 7, 2026 09:49
@kate-shine
Kateřina Churanová (kate-shine) force-pushed the kchuranov/fix-multitude-over-aligned-tests branch from 9f43233 to 90599ca Compare August 7, 2026 09:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (d799037) to head (d25a7f5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #650   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         473      473           
  Lines       45493    45493           
=======================================
  Hits        45493    45493           
Flag Coverage Δ
linux 80.1% <ø> (-19.9%) ⬇️
linux-arm 80.1% <ø> (-19.9%) ⬇️
scheduled ?
windows 80.1% <ø> (-19.9%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread rust-toolchain.toml Outdated

[toolchain]
# This needs to match RUST_LATEST in constants.env.
channel = "1.96.1"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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>
@kate-shine
Kateřina Churanová (kate-shine) force-pushed the kchuranov/fix-multitude-over-aligned-tests branch from 90599ca to d25a7f5 Compare August 7, 2026 11:59
Copilot AI review requested due to automatic review settings August 7, 2026 11:59
@kate-shine Kateřina Churanová (kate-shine) changed the title fix(build): restore accidentally deleted rust-toolchain.toml fix(multitude): gate over-aligned doctest behind utc_backend cfg Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Approved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants