Make EString::string() join folded ropes, fixing require.resolve() and other readers of the first segment - #38944
Open
robobun wants to merge 4 commits into
Open
Make EString::string() join folded ropes, fixing require.resolve() and other readers of the first segment#38944robobun wants to merge 4 commits into
robobun wants to merge 4 commits into
Claude / Claude Code Review
completed
Aug 15, 2026 in 41m 38s
Code review found 1 potential issue
Found 4 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/ast/e.rs:1919-1922 |
Same-class rope readers via direct .data access are not covered by the .string() audit |
Annotations
Check warning on line 1922 in src/ast/e.rs
claude / Claude Code Review
Same-class rope readers via direct .data access are not covered by the .string() audit
The new `debug_assert` guards `.string()`, and the PR description says the three fixed sites "were the only ones that read a rope" — but the audit only covered `.string()` callers. Several post-visit readers reach the rope's first segment via a direct `.data` field access, which neither the audit nor the assert catches: `Import::import_record_loader` (this file, ~L2464 — `Loader::from_string(&str_.data)`; reachable with no minify flag because the `EImport` arm forces folding), `rewrite_import_me
Loading