Skip to content

refactor(rust): remove resolved borrowck-reshape comments (bucket A, 256 sites) - #35360

Open
robobun wants to merge 8 commits into
mainfrom
farm/4a4d67fa/borrowck-cleanup-a
Open

refactor(rust): remove resolved borrowck-reshape comments (bucket A, 256 sites)#35360
robobun wants to merge 8 commits into
mainfrom
farm/4a4d67fa/borrowck-cleanup-a

refactor(rust): remove resolved borrowck-reshape comments (bucket A, …

f49e6b3
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 24, 2026 in 22m 58s

Code review found 1 important issue

Found 5 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/bundler/ParseTask.rs:2728-2733 Several A1 sites deleted safety invariants that borrowck cannot enforce (should be A2)
🟡 Nit src/js_parser_jsc/Macro.rs:222-224 New A2 SAFETY comment contradicts the adjacent SAFETY comment on the same unsafe block
🟡 Nit src/http/lib.rs:4371 Orphaned comment fragments left after partial deletion

Annotations

Check failure on line 2733 in src/bundler/ParseTask.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Several A1 sites deleted safety invariants that borrowck cannot enforce (should be A2)

Four sites bucketed A1 actually document memory-safety invariants that borrowck cannot enforce, so deleting them violates REVIEW.md's "Don't delete existing why-comments in cleanup passes": this `ParseTask.rs` site (the take-replace keeps a lifetime-erased `StoreStr` borrow into `entry.contents` alive — the callee comment at ~2365 still says "see ParseTask::run", which now points at bare code), plus `IOReader.rs` `on_read_chunk_cb` and `IOWriter.rs` `get_buffer_impl`/`bump` (the deleted text exp

Check warning on line 224 in src/js_parser_jsc/Macro.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

New A2 SAFETY comment contradicts the adjacent SAFETY comment on the same unsafe block

The new A2 SAFETY comment here asserts reentry "may … mutate self.macros/self.bump", but the pre-existing SAFETY comment 8 lines below (guarding the same unsafe derefs) asserts "`self.macros` … is not mutated for the duration of this closure; `bump` … is not otherwise borrowed" — the two now directly contradict each other. The `self.bump` claim is also wrong (after `get_or_insert_with` the Option is `Some`, so reentry is a no-op and the `*const Arena` address is stable), and this site is a `*con

Check warning on line 4371 in src/http/lib.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Orphaned comment fragments left after partial deletion

Three of the multi-line `reshaped for borrowck` comments were only partially deleted, leaving orphaned tail sentences with no context: this line's cross-reference now points at a function whose own pattern explanation was also deleted in this PR; `src/parsers/yaml.rs:4097` ("Take it out before scanning.") lost the antecedent for "it"; and `src/runtime/bake/production.rs:992` ("Fetch the output file fresh at each use site…") floats before a blank line as bare narration. These fragments should be