Fix assert! and cover! expansions in expression contexts - #4875
feliperodri merged 5 commits into
Conversation
Emit an explicit block expression so strict warnings do not reject the no-message assert macro in constant and other expression positions. Add focused regression coverage for expression forms, single evaluation, and an expected assertion failure. Fixes model-checking#4874
|
I investigated the Intel macOS failure. The same fixture also failed with a timeout on the upstream base commit, before this PR. The assertion regression suite passed, and all other platform regression jobs passed. This appears to be an existing intermittent timeout. Could a maintainer rerun the failed job? My account lacks permission to trigger it. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The targeted fix matches existing macro patterns and is comprehensively covered.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes no-message assert! usage in expression contexts by emitting a unit-valued block.
Changes:
- Wraps the macro expansion in an explicit block.
- Adds regression coverage for expression contexts, evaluation count, and failure behavior.
| File | Description |
|---|---|
library/std/src/lib.rs |
Corrects the no-message assert! expansion. |
tests/kani/Assert/expression_contexts.rs |
Adds focused regression harnesses. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
feliperodri
left a comment
There was a problem hiding this comment.
Approving. Reproduced on main: the new test fails with 7 trailing semicolon in macro used in expression position errors and passes with the fix. kani/expected/ui suites are green, and no assertion message changed.
One point that strengthens the case: this lint is future-incompatible (rust-lang/rust#79813, "will become a hard error in a future release"), so it isn't only a -D warnings problem. Without the fix, a future toolchain would break expression-position asserts for everyone. On this nightly the lint is semicolon_in_expressions_from_non_local_macros, not ..._from_macros.
kani::cover! (library/kani/src/lib.rs, all three arms) has the same shape and fails the same way: true => kani::cover!(), errors under -D warnings even with this PR. Not blocking; it would fit here as the same three-line change, or as a follow-up.
The macOS-Intel failure (cargo_autoharness_fmt_impls) isn't from this PR: the same test fails on main at 3e07bd2de.
Wrap all three cover macro arms in explicit unit-valued blocks and add an expected-output regression for emitted properties and single evaluation. Explain the assertion macro braces requested in review.
|
Merged current upstream I also investigated all four failures from the preceding
The dependency failure also occurs on current upstream main after the s2n-quic update. I posted the source diagnosis, proposed s2n-quic repair, and validation boundary on #4901. The same checksum timing gate also trips on current main (12.14612s → 22.71049s). I added these measurements and the caveat that equal VCC/step counts do not establish formula identity on #4846. A controlled repeat is still needed before attributing or dismissing a change-specific slowdown; the full benchmark runs were not clean. I found no correctness change needed in the macro patch from this investigation. The new Kani CI run and companion workflows are currently |
f43aa6d
Problem and resulting behavior
The no-message
assert!arm and all threekani::cover!arms emit a trailing semicolon without an enclosing Rust block. Valid expression-position calls therefore produce a trailing-semicolon diagnostic; with warnings denied, compilation fails before verification.This is also a future-compatibility defect, tracked in rust-lang/rust#79813. Our original Kani 0.68.0 environment (
nightly-2026-08-21) reportedsemicolon_in_expressions_from_macros; the PR's newernightly-2026-09-23reportssemicolon_in_expressions_from_non_local_macros. The underlying invalid expansion is the same.Resolves #4874. Related: #1572 addressed expression-context expansion for a message-bearing assertion.
Changes
assert!arm emit a unit-valued block expression, matching the existing message-bearing arms.kani::cover!forms: location, condition, and condition with a literal message.!!handling from Kani assert override is more restrictive than standard assert #2108.The inner braces are emitted as a Rust block, so the existing call's semicolon is valid inside it:
No additional branch, allocation, formatting operation, or condition evaluation is introduced. No caller rewrite or lint suppression is needed.
Regression coverage
tests/kani/Assert/expression_contexts.rsdenies warnings and covers constant initializers, inline constants, statement/expression forms, the optional comma, single condition evaluation, and a separate#[kani::should_panic]control to reject a no-op assertion implementation.tests/expected/cover/expression-contexts/uses the existing expected-output suite. Under#![deny(warnings)], all three cover forms appear as unit-typed initializers. The test requires:SATISFIED.falsecondition to beUNSATISFIABLE.Checking the emitted properties and their outcomes prevents a no-op cover implementation from passing.
Validation
Focused follow-up validation (Linux aarch64, nightly-2026-09-23): used the compiler from this PR's successful CI bundle build, rebuilding the verifier/model libraries from the PR source plus these changes in an isolated installation.
--suite expected --mode expected --timeout 120 cover/passed 6 tests, including the new exact-output regression and existing cover cases.--suite kani --mode kani --timeout 120 Assert/passed 13 tests, including the original assertion regression and its expected-failure harness.Independent maintainer validation of the original assertion fix: feliperodri reproduced seven expression-position errors on main and confirmed the fix passes, with the
kani,expected, anduisuites green and assertion messages unchanged.Earlier downstream validation: the assertion repair was also validated with the released Kani 0.68.0 compiler, strict-warning positive and deliberate-failure controls, and an existing downstream proof suite, without changing application assertions or warning policy.
The full upstream suite and other platforms have not been rerun locally for this follow-up; the new commit's CI remains the broader check. The previously reported Intel macOS
cargo_autoharness_fmt_implstimeout also occurred on upstream main, as the maintainer confirmed.Adoption
Distribute the corrected libraries through Kani's normal release process so affected users can return to the unmodified official bundle.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.