Skip to content

fix(semantic): make multi_index_vec clone panic-safe#19299

Merged
graphite-app[bot] merged 1 commit intomainfrom
boshen/multi-index-vec-followup
Feb 12, 2026
Merged

fix(semantic): make multi_index_vec clone panic-safe#19299
graphite-app[bot] merged 1 commit intomainfrom
boshen/multi-index-vec-followup

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Feb 12, 2026

Summary

Follow-up to #19138 after merge.

This PR fixes panic-safety in multi_index_vec! clone and keeps strict lint clean for oxc_semantic tests.

Changes

  • Make macro-generated Clone panic-safe by cloning row-by-row through push.
    • Avoids setting len before all elements are initialized.
    • Prevents dropping uninitialized elements if a field clone() panics.
  • Add #[expect(clippy::unused_self)] to cfg-disabled integration helper methods to keep cargo clippy -p oxc_semantic --lib --tests -D warnings green.

Validation

  • cargo clippy -p oxc_semantic --lib --tests -- -D warnings
  • just lint

AI disclosure

AI-assisted: I used Codex to draft and apply this patch, then ran lint checks and reviewed the result before opening this PR.

@Boshen Boshen requested a review from Dunqing as a code owner February 12, 2026 07:39
Copilot AI review requested due to automatic review settings February 12, 2026 07:39
@github-actions github-actions bot added A-semantic Area - Semantic C-bug Category - Bug labels Feb 12, 2026
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Feb 12, 2026
Copy link
Member Author

Boshen commented Feb 12, 2026

Merge activity

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is a follow-up to #19138 that addresses panic-safety in the macro-generated Clone implementation for multi_index_vec!, and keeps oxc_semantic integration tests clippy-clean under strict lint settings.

Changes:

  • Rework multi_index_vec!’s Clone implementation to clone row-by-row using push, avoiding setting len before initialization is complete.
  • Add #[expect(clippy::unused_self, reason = ...)] to cfg-disabled integration helper methods to keep strict clippy runs warning-free.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/oxc_semantic/src/multi_index_vec.rs Changes macro-generated Clone to be panic-safe by building the clone incrementally via push.
crates/oxc_semantic/tests/integration/util/mod.rs Suppresses clippy::unused_self for cfg-disabled helper methods while preserving consistent signatures.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 12, 2026

Merging this PR will not alter performance

✅ 47 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing boshen/multi-index-vec-followup (a791f94) with main (5b90d46)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

## Summary
Follow-up to #19138 after merge.

This PR fixes panic-safety in `multi_index_vec!` clone and keeps strict lint clean for `oxc_semantic` tests.

### Changes
- Make macro-generated `Clone` panic-safe by cloning row-by-row through `push`.
  - Avoids setting `len` before all elements are initialized.
  - Prevents dropping uninitialized elements if a field `clone()` panics.
- Add `#[expect(clippy::unused_self)]` to cfg-disabled integration helper methods to keep `cargo clippy -p oxc_semantic --lib --tests -D warnings` green.

## Validation
- `cargo clippy -p oxc_semantic --lib --tests -- -D warnings`
- `just lint`

## AI disclosure
AI-assisted: I used Codex to draft and apply this patch, then ran lint checks and reviewed the result before opening this PR.
@graphite-app graphite-app bot force-pushed the boshen/multi-index-vec-followup branch from a791f94 to bfb15a3 Compare February 12, 2026 08:54
@graphite-app graphite-app bot merged commit bfb15a3 into main Feb 12, 2026
21 checks passed
@graphite-app graphite-app bot deleted the boshen/multi-index-vec-followup branch February 12, 2026 09:00
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Feb 12, 2026
Limerio pushed a commit to Limerio/oxc that referenced this pull request Feb 12, 2026
## Summary
Follow-up to oxc-project#19138 after merge.

This PR fixes panic-safety in `multi_index_vec!` clone and keeps strict lint clean for `oxc_semantic` tests.

### Changes
- Make macro-generated `Clone` panic-safe by cloning row-by-row through `push`.
  - Avoids setting `len` before all elements are initialized.
  - Prevents dropping uninitialized elements if a field `clone()` panics.
- Add `#[expect(clippy::unused_self)]` to cfg-disabled integration helper methods to keep `cargo clippy -p oxc_semantic --lib --tests -D warnings` green.

## Validation
- `cargo clippy -p oxc_semantic --lib --tests -- -D warnings`
- `just lint`

## AI disclosure
AI-assisted: I used Codex to draft and apply this patch, then ran lint checks and reviewed the result before opening this PR.
camc314 pushed a commit that referenced this pull request Feb 16, 2026
### 🚀 Features

- 429d876 semantic: Assign ast node ids during semantic build (#19263)
(Boshen)
- ebb80b3 ast: Add `node_id` field to all AST struct nodes (#18138)
(Boshen)

### 🐛 Bug Fixes

- bfb15a3 semantic: Make multi_index_vec clone panic-safe (#19299)
(Boshen)
- 41c50a5 transformer: Ignore invalid JSX pragma identifiers (#19296)
(Boshen)
- deed3d8 transformer: Remove unnecessary trailing expression in object
rest spread assignment (#19259) (Boshen)
- 5bdaacc transformer: Propagate source spans for sourcemap correctness
(#19258) (Boshen)
- 3e0e5ba isolated-declarations: Align readonly class array initializer
diagnostics with tsc (#19218) (camc314)

### ⚡ Performance

- c169c77 syntax: Optimize `is_identifier_name_patched` (#19386)
(sapphi-red)
- aa1e1a8 allocator: Inline BitSet accessors (#19331) (Boshen)
- 5b90d46 semantic: Improve SoA with multi index vec (#19138) (Boshen)
- 99ce2a6 isolated_declarations: Mark all diagnostic functions as
`#[cold]` (#19279) (camc314)
- dd0220f transformer: Remove TS-only nodes earlier in
`enter_statements` (#19166) (Dunqing)
- e5baf60 isolated-declarations: Replace hash collections with
index-based Vec (#19221) (Dunqing)

### 📚 Documentation

- 569aa61 rust: Add missing rustdocs and remove missing_docs lint attrs
(#19306) (Boshen)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-semantic Area - Semantic C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants