Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 1. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the spec hierarchy log generation to create log files per verification target rather than processing all specs at once. The change improves the accuracy of spec hierarchy logs by ensuring each target (function or module) gets its own filtered context during verification.
Changes:
- Moved spec hierarchy log generation from the initialization phase to the verification phase
- Refactored
spec_hierarchy::display_spec_hierarchyto support per-spec log writing via newwrite_spec_hierarchy_logfunction - Removed premature spec hierarchy generation from
sui-prover/prove.rsthat used unfiltered targets
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/sui-prover/src/prove.rs | Removed early spec hierarchy generation code and unnecessary imports (create_and_process_bytecode, FunctionHolderTarget, spec_hierarchy) |
| crates/move-stackless-bytecode/src/spec_hierarchy.rs | Added write_spec_hierarchy_log function for per-spec logging and refactored display_spec_hierarchy to use it |
| crates/move-prover-boogie-backend/src/generator.rs | Added write_spec_hierarchy_logs function that generates hierarchy logs during verification with properly filtered targets (only during AssertsMode::Check to avoid duplicates) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@claude internal tests failed in CI, is it related my changes? I checked, tests pass locally. |
|
Claude finished @danilych's task in 7s —— View job CI Failure Analysis
SummaryYES, the CI failure is related to your changes. The test Failure DetailsFailed Test: Test Code: fun foo(m: &mut vec_map::VecMap<u64, u8>) {
m.remove(&10);
}
#[spec(prove)]
fun foo_spec(m: &mut vec_map::VecMap<u64, u8>) {
requires(m.contains(&10));
foo(m);
ensures(!m.contains(&10)); // ← Verification fails here
}Root CauseThe PR removes the early call to Key Behavioral Change: Before (main branch): // In prove.rs - processes ALL targets together upfront
let (targets, _) = create_and_process_bytecode(
&options, &model, &package_targets,
FunctionHolderTarget::All // ← Processes all specs together
);
spec_hierarchy::display_spec_hierarchy(&model, &targets, output_dir);After (this PR): // In generator.rs - processes per function during verification
let (mut targets, _) = create_and_process_bytecode(
options, env, package_targets,
FunctionHolderTarget::Function(*qid) // ← Only processes this function
);Why This Breaks VerificationWhen using
VecMap is in the Sui framework (address Recommendations
Next StepsCould you clarify the intended behavior? The PR description mentions fixing "lack of ghosts" bugs, but this change seems to introduce a new issue with VecMap verification. The simplest fix might be to restore the early |
#512
The bug was that we run
create_and_process_bytecodefor get target for create log files, and we not filter specs per visibility so it was cause of a lot of bugs like lack of ghosts even if some specs not included.