blockifier_test_utils: delete dead code (unsure, review carefully) - #14881
Open
asaf-sw wants to merge 1 commit into
Open
blockifier_test_utils: delete dead code (unsure, review carefully)#14881asaf-sw wants to merge 1 commit into
asaf-sw wants to merge 1 commit into
Conversation
Removes two unused `pub fn` helpers and their now-orphaned imports: - `FeatureContract::all_cairo1_casm_compiler_versions` (contracts.rs) and the `use std::collections::HashSet;` it was the sole user of. - `cairo1_compiler_tag` (cairo_compile.rs) and the `use apollo_infra_utils::cairo_compiler_version::CAIRO1_COMPILER_VERSION;` import it was the sole user of (that symbol is still used elsewhere in contracts.rs, so its import there is retained). Evidence of deadness: a workspace-wide grep for each identifier finds only the definition site -- no callers in any crate, in `tests/`/`benches/`, or in the sibling repos `starkware-industries/sequencer-devops` and `starkware-industries/starkware`. These are inherent `pub fn` (not behind a trait, macro, `serde`, or reflection), so grep fully captures their references. Verification (env workarounds: `RUSTC_WRAPPER=`, `CARGO_INCREMENTAL=0`): - `cargo build -p blockifier_test_utils` -- clean, zero warnings. - `cargo build -p blockifier_test_utils --tests` -- clean, zero warnings. - `cargo clippy -p blockifier_test_utils --all-targets` -- clean. - `SEED=0 cargo test -p blockifier_test_utils` -- all tests pass. Filed as low-confidence: `blockifier_test_utils` is a test-utility crate whose `pub fn`s form its public API. They have no in-workspace or sibling-repo callers, but I cannot rule out an external consumer (e.g. another repo's test harness) depending on them, which is the reachability uncertainty a human owner must adjudicate.
|
This change is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Caution
REVIEW WITH CARE! THIS PR REQUIRES CAREFUL HUMAN REVIEW.
If you find this to be a false positive comment in detail why this code should be kept and close the PR.
What
Removes two unused
pub fnhelpers fromblockifier_test_utilsand their now-orphaned imports:FeatureContract::all_cairo1_casm_compiler_versions(contracts.rs) and theuse std::collections::HashSet;it was the sole user of.cairo1_compiler_tag(cairo_compile.rs) and theuse apollo_infra_utils::cairo_compiler_version::CAIRO1_COMPILER_VERSION;import it was the sole user of. (That symbol is still used elsewhere incontracts.rs:310, so the import there is retained.)Why these look dead
A workspace-wide grep for each identifier finds only the definition site — no callers in any crate, in
tests//benches/, or in the sibling reposstarkware-industries/sequencer-devopsandstarkware-industries/starkware. Both are inherentpub fns (not behind a trait, macro,serde, FFI, or reflection), so a textual grep fully captures their references.What a human must verify
blockifier_test_utilsis a test-utility crate whosepub fns form its public API. These two have no in-workspace or sibling-repo callers, but I cannot rule out an external consumer (e.g. another repository's test harness) that depends oncairo1_compiler_tagorall_cairo1_casm_compiler_versions. Please confirm no out-of-tree consumer relies on them before merging.Verification (local)
Env workarounds applied (
RUSTC_WRAPPER=,CARGO_INCREMENTAL=0):cargo build -p blockifier_test_utils— clean, zero warnings.cargo build -p blockifier_test_utils --tests— clean, zero warnings.cargo clippy -p blockifier_test_utils --all-targets— clean.SEED=0 cargo test -p blockifier_test_utils— all tests pass.Caution
REVIEW WITH CARE! THIS PR REQUIRES CAREFUL HUMAN REVIEW.
If you find this to be a false positive comment in detail why this code should be kept and close the PR.
Generated by Claude Code