Skip to content

refactor(lint): read the HIR through gcx - #16621

Draft
DaniPopes wants to merge 16 commits into
dani/symbolic-cli-testsfrom
refactor/lint-gcx-hir
Draft

refactor(lint): read the HIR through gcx#16621
DaniPopes wants to merge 16 commits into
dani/symbolic-cli-testsfrom
refactor/lint-gcx-hir

Conversation

@DaniPopes

@DaniPopes DaniPopes commented Sep 4, 2026

Copy link
Copy Markdown
Member

Follow-up to #16615. Gcx<'hir> already carries the HIR as gcx.hir, yet most lints kept passing a separate hir: &Hir alongside gcx to their helpers and storing both in their analyzer and visitor structs. This drops that redundancy everywhere: helpers and analyzers read &gcx.hir (or bind it once locally), hir::Visit impls return &self.gcx.hir, and no function or struct in the crate takes both any more.

The LateLintPass API itself is fixed on the Solar side in paradigmxyz/solar#1376, whose hooks receive only the global context; this PR bumps the Solar pin to its merge commit and migrates every pass. The bump also picks up Solar's new for-loop lowering, which keeps the update statement on LoopSource::For { update } instead of splicing it into the body, so the loop-walking analyses now run the update after each completed iteration through the shared loop_update/loop_stmts helpers, and the lints that used to reverse-engineer the old lowering (ecrecover, function-selector-collision, cache-array-length, enumerable-loop-removal) read it directly.

No lint behavior changes; the UI fixtures are untouched and pass unchanged. Written with AI assistance (Claude Code).

🤖 Generated with Claude Code

DaniPopes and others added 5 commits September 4, 2026 13:26
`Gcx` already exposes the HIR as `gcx.hir`, so helpers and visitors
that carried both a `gcx` and a `hir` were redundant. Drop the `hir`
parameter and field from `costly-loop`, `cache-array-length` and
`incorrect-using-for` and read the HIR through `gcx` instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Drop redundant hir parameters and fields from helpers, analyzers and
visitors that already have a Gcx, reading the HIR through gcx.hir.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`Gcx` already exposes the HIR as `gcx.hir`, so the low-severity lints no
longer pass or store a separate `hir` next to `gcx`. Analyzer and walker
structs keep only `gcx` and implement `Visit::hir` as `&self.gcx.hir`;
helpers that were hir-only but only ever called with a `gcx` in scope
(`resolved_super_function_ids`, `is_deprecated_oz`,
`is_unchecked_token_op`, `event_mentions_state_var`) now take `gcx` so
call sites are uniform.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Analyzer and visitor structs in the high-severity lints stored both the
global context and a separate HIR reference, and helpers took both as
parameters. Drop the redundant `hir` fields and parameters and read the
HIR through `gcx.hir` instead; `hir::Visit` impls return `&self.gcx.hir`.
Helpers that only need the HIR and have no `gcx` in scope are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

✅ Changelog found

The deterministic check will validate the changed entry.

Bump Solar to paradigmxyz/solar#1376, whose LateLintPass hooks receive
only the global context, and update every pass accordingly. The bump
also brings the new for-loop lowering that keeps the update statement on
LoopSource::For instead of inside the body, so the loop-walking analyses
now run the update after each completed iteration through the shared
loop_update and loop_stmts helpers, and the lints that reverse-engineered
the old lowering (ecrecover, function-selector-collision,
cache-array-length, enumerable-loop-removal) read it directly.
The lifetime is the global context's, of which the HIR is one field;
name it accordingly, matching the Solar side.
DaniPopes added a commit to paradigmxyz/solar that referenced this pull request Sep 4, 2026
`Gcx<'hir>` already exposes the HIR as `gcx.hir`, so every
`LateLintPass` hook now receives only the global context: the hooks that
took both `gcx` and `hir` drop the `hir` parameter, and the hooks that
took only `hir` take `gcx` instead, giving the whole trait one uniform
shape. `LateLintVisitor` no longer stores a separate HIR handle and its
constructor loses that argument.

This is a breaking change for downstream lint passes (Foundry's lints
are being updated alongside in foundry-rs/foundry#16621). Written with
AI assistance (Claude Code).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
DaniPopes and others added 8 commits September 4, 2026 15:26
Pin the merged paradigmxyz/solar#1376 instead of its branch head.
Drop the local hir bindings that only aliased gcx.hir.
* chore(ci): enforce Cargo cooldown

* fix(ci): annotate Cargo action pins

* fix(ci): harden cargo cooldown

Invoke the checksum-pinned verifier directly with isolated policy state and all-target coverage. Run baseline checks before candidate code and pin Docker build tools outside the workspace lockfile.

* refactor(ci): use shared cargo cooldown

Pin the merged tempoxyz action with strict project policy for both the primary dependency gate and benchmark baseline. Remove the duplicated installer, verifier wrapper, and local behavioral tests now owned upstream.

* fix(ci): allow pre-policy benchmark baseline

Enable strict cooldown configuration only when the checked-out master baseline contains cooldown.toml. Older baselines remain valid while every baseline that carries policy is still validated strictly.
* refactor(symbolic): consolidate call state machines

* style nits

---------

Co-authored-by: Mablr <59505383+mablr@users.noreply.github.com>
@DaniPopes
DaniPopes changed the base branch from master to dani/symbolic-cli-tests September 4, 2026 17:19
Drop a stale duplicate doc comment and import Gcx instead of spelling
its path inline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants