Autoharness: support pattern types (RigidTy::Pat) - #4780
Conversation
Teach autoharness to recognize and generate values for pattern types, since nightly-2026-04-01. Without this, any function taking a NonNull argument (or a struct containing one) is skipped with 'Missing Arbitrary'. Resolves model-checking#4758
|
Verified locally — after rebuilding kani-compiler on this branch, both test cases reproduce the PR description: autoharness_niche: 10/10 functions verified (up from 2/10 pre-change) One question on call_kani_any_for_ty: the base value is generated and transmuted into the pattern type before assume_scalar_niche runs, so there's a brief window where the local holds an out-of-niche value (e.g. a null pointer typed as non-null). Does Kani's codegen insert any validity check on the Transmute cast itself that could fire before the assume takes effect? Tests pass here, so this is likely fine by design, just want to confirm it's intentional rather than incidental to these specific base types. |
|
I noticed a related pointer case: does assume_scalar_niche actually enforce the non-null constraint for NonNull pointer patterns? The current cover shows that a non-null value is reachable, but it doesn’t prove that null values can never be generated. Could we add an assertion that the generated pointer is always non-null? |
Nice catch! I looked into scalar_niche, and turns out it does not match pointers, so it returned None for pointer-based pattern types, and assume_scalar_niche never constrained the generated value. I'll make the change to scalar_niche to also handle Primitive::Pointer, and added a kani::assert alongside the existing kani::cover! |
There was a problem hiding this comment.
🟡 Changes recommended
Pattern-kind inspection can panic, validity is assumed too late, and pointer backing storage may expire prematurely.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds autoharness generation for Rust pattern types, restoring support for ranged scalar fields and NonNull<T>.
Changes:
- Recognizes derivable pattern-type bases.
- Generates constrained pattern-type values.
- Adds and updates autoharness regression tests.
File summaries
| File | Description |
|---|---|
kani-compiler/src/kani_middle/mod.rs |
Adds pattern derivability and pointer niches. |
kani-compiler/src/kani_middle/transform/automatic.rs |
Generates pattern-type values. |
tests/script-based-pre/autoharness_niche/niche_probe.rs |
Updates pattern-type documentation. |
tests/script-based-pre/autoharness_niche/expected |
Expects restored niche verification. |
tests/script-based-pre/autoharness_pattern_type/config.yml |
Configures the new test. |
tests/script-based-pre/autoharness_pattern_type/run.sh |
Runs the pattern-type test. |
tests/script-based-pre/autoharness_pattern_type/pattern_type_probe.rs |
Exercises NonNull arguments and fields. |
tests/script-based-pre/autoharness_pattern_type/expected |
Defines expected verification results. |
Review details
Suppressed comments (2)
kani-compiler/src/kani_middle/mod.rs:1128
NonNull'sNotNullpattern cannot be inspected through stableTy::kind():check_values.rs:919-924explicitly documents that this conversion panics and usesrustc_internal::internalinstead. In this loop, the firstty.kind()at line 1108 will therefore panic before this new arm can recognize the field, so the newNonNulltests cannot be processed reliably. Detect/extract pattern types through the internal rustc type (which will require threadingTyCtxtinto this eligibility logic).
} else if let TyKind::RigidTy(RigidTy::Pat(base_ty, _)) = ty.kind() {
fields_impl_arbitrary &=
pat_base_is_derivable(base_ty, kani_any_def, ty_arbitrary_cache);
kani-compiler/src/kani_middle/transform/automatic.rs:1384
- The transmute constructs a pattern-typed value before its validity constraint is assumed. With
-Z valid-value-checks,ValidValuePassinstruments exactly such transmutes (check_values.rs:632-646), so invalid base values fail before the later assumption; this also violates the immediate validity requirement demonstrated bytests/expected/valid-value-checks/custom_niche.rs:57-67. Constrain the base bits using the pattern layout before constructing the pattern value.
// A pattern type (e.g. `pattern_type!(*const T is !null)`) is layout-compatible with its
// base type. Generate an arbitrary value of the base type, transmute it to the pattern
// type, then constrain it to the pattern's validity range via `assume_scalar_niche`.
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@Tianshu-Huang could you address all copilot comments? |
… integer patterns
Thank you for raising this up! Confirmed with -Z valid-value-checks, it did fail. Fixed by assuming the niche on the base value before the transmute; the test now runs with that flag. |
@feliperodri Thanks for the nudge. All three Copilot comments are now addressed. To keep this PR scoped to #4758 & keep the review simple, I'll split NonNull support into a follow-up PR. |
feliperodri
left a comment
There was a problem hiding this comment.
Approving. I built the branch and went looking for cases your tests do not cover; it held up.
Confirmed the headline numbers: autoharness_niche goes from 2/10 to 10/10, and the new pattern-type test passes with all three covers satisfied. Then I tried the shapes I expected to break it, and none did:
- a wrapping bit-pattern range,
pattern_type!(i8 is -1..=1)— start 255 above end 1, so theBitOrbranch — with an in-range assert on the generated value - a pattern type two structs deep, and
Option<pattern_type!(u8 is 1..=100)> NonNull<u32>as an argument, and a struct with aNonNullfield: both skipped as "Missing Arbitrary implementation", no ICE, which is the deferral you documented
Three things I would still like, none of them worth holding the PR for:
Tie derivability to constrainability. pat_base_is_derivable gates on the base implementing Arbitrary, but the constraint comes from scalar_niche(pat_ty), which returns None for non-scalar ABIs, floats and full ranges — and assume_scalar_niche bails again for widths outside {8,16,32,64,128}. If those two ever disagree we silently generate out-of-pattern values, which is a spurious counterexample at best and a spurious failure under -Z valid-value-checks. No gap exists today because rustc's pattern types are integer and pointer only, but that is being extended. Requiring scalar_niche(tcx, ty).is_some() in pat_base_is_derivable closes it in one line.
Pin all three covers in autoharness_pattern_type/expected. The file pins one Status: SATISFIED for three covers, and an unsatisfiable cover does not fail a harness — covers are subtracted out of number_properties and never counted in number_checks_failed (cbmc_property_renderer.rs:340-347). So if the assumption started over-constraining, which is exactly what those covers are there to catch, two of them could flip to UNSATISFIABLE and the test would still pass.
The Primitive::Pointer arm in scalar_niche belongs with the follow-up. Nothing here uses it: pat_base_is_derivable rejects raw-pointer bases. It does change behaviour at the existing call site in automatic.rs, where every generated Box/Rc/Arc/NonNull/fn-pointer value now picks up a transmute to u64 and a range assume. I checked Box<u32> still verifies, so this looks harmless, but it is untested here and the doc comment advertises a NonNull capability the PR then declines to use. Either test it now or move it to the NonNull PR.
Unrelated, found while probing: -Z valid-value-checks ICEs on any function that allocates, because NonNull<[u8]>'s field is a fat-pointer pattern type and check_values.rs asserts pattern types are scalar. Filed as #4829 — it reproduces on main with no pattern types in the source, so it is not yours, but it will likely land in front of the NonNull follow-up.
|
Thanks for the thorough review. I'll take all three suggestions into the NonNull follow-up rather than re-spin this PR. On #4829: I hit the same assertion while running autoharness over verify-rust-std and already carry the conservative Err(...) fix on my baseline branch; I'll open it as a separate PR. Happy for this one to be merged as is. |
Problem
Since
nightly-2026-04-01,NonNull<T>wraps apattern_type!(*const T is !null)instead of a bare*const T. Sincenightly-2026-06-01(#4760),rustc_layout_scalar_valid_range_start/endattributes were replaced by pattern types more broadly. Autoharness does not recognizeRigidTy::Pat— any function whose signature involves a pattern type (directly or through a struct field) is skipped with "Missing Arbitrary implementation".Solution
can_derive_arbitrary(mod.rs) — recognizeRigidTy::Patin both the struct-field iteration loop and the top-level type match, delegating to a new helperpat_base_is_derivable. Raw-pointer bases (NonNull's field) are not supported: the pointee storage would be a local of the synthesized any() and dangle on return. NonNull is a follow-up PR.call_kani_any_for_ty(automatic.rs) — generate a value of the base type, constrain it via assume_scalar_niche, then transmute — assuming first keeps it valid under -Z valid-value-checks.Tests
Updated
autoharness_niche:Month,Schedule,PosI8, andcheck_monthly::<Month>move from skipped to verified (10/10 functions pass), satisfying the acceptance criteria in #4758.New
autoharness_pattern_type: an integer pattern type as a struct field and as a top-level argument, run with -Z valid-value-checks.Resolves #4758
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.