Skip to content

feat(allocator): add allocator#2

Merged
Boshen merged 1 commit into
mainfrom
allocator
Feb 11, 2023
Merged

feat(allocator): add allocator#2
Boshen merged 1 commit into
mainfrom
allocator

Conversation

@Boshen

@Boshen Boshen commented Feb 11, 2023

Copy link
Copy Markdown
Member

No description provided.

@Boshen Boshen merged commit 664c376 into main Feb 11, 2023
@Boshen Boshen deleted the allocator branch February 11, 2023 09:05
graphite-app Bot pushed a commit that referenced this pull request May 15, 2026
…n for downstream (#22349)

Closes #17480.

Supersedes #21526 (gthb) and #22251 (mo-n) — both target the same bug from different angles. This PR takes a single coordinated approach.

Closes: #21526
Closes: #22251

## What

A `/* @__PURE__ */`-annotated IIFE assigned to an unused `var`/`let`/`const` was not being dropped, and inlining such an IIFE silently destroyed the annotation that rolldown's tree-shaker relies on. The fix lands three coordinated checks in `substitute_iife_call`, one small swap in `handle_variable_declaration`, and an export-ancestor guard.

### 1. Widened `is_expression_result_unused`

`is_expression_result_unused` already short-circuited pure IIFEs to `void 0` for bare expression statements. This PR extends it to also recognize `VariableDeclaratorInit` whose binding has no references and isn't exported. The unused declaration then drops in a single iteration — regardless of body shape (call, new, member access, tagged template, chain, sequence, conditional, etc.).

Guards:
- `can_remove_unused_declarators` — respects script-mode and direct-eval gates.
- `decl.kind().is_using()` — `using` / `await using` run `[Symbol.dispose]` at scope exit, so the init isn't truly unused.
- `var_declaration_is_exported` — exported bindings bypass `handle_variable_declaration`, so dropping the init would silently break the export's runtime value. The check looks only at `ctx.ancestors().nth(2)` (the slot directly above `VariableDeclaration`); walking the full chain would over-broaden the guard to function-local vars inside exported functions.

### 2. `iife_inline_would_lose_pure` (DCE-only gate)

In DCE-only mode (rolldown's per-module preprocess), refuse to inline a pure IIFE whose body has *any* side effects. The original outer IIFE call has zero arguments, so its `pure` flag covers the body's evaluation as a unit; inlining surfaces sub-effects at the outer call's argument level, where rolldown's side-effect detector flags them regardless of any propagated `pure` flag.

Gated on `ctx.state.dce` because in full-minify mode there's no downstream consumer to preserve the annotation for — inline aggressively for smaller output.

### 3. `try_take_iife_body` helper

Unifies the three IIFE inline branches (expression body, expression-statement body, return-statement body). Returns `Some(inlined)` after taking the body and stamping `pure = true` on a `CallExpression`/`NewExpression` replacement; returns `None` to bail when `iife_inline_would_lose_pure` says inlining would weaken the assertion — caller leaves the IIFE intact.

### 4. `remove_unused_expression` swap in `handle_variable_declaration`

Replace `init.may_have_side_effects(ctx)` with `!Self::remove_unused_expression(&mut init, ctx)`. The smart expression cleanup peels pure-call wrappers and keeps only side-effectful args (`var x = /* @__PURE__ */ foo(a)` → `a;`).

## Relation to #21526 and #22251

| Mechanism | This PR | #21526 (gthb) | #22251 (mo-n) |
|---|---|---|---|
| Widened unused-var-init drop | ✓ | ✗ | ✓ (as `is_in_unused_variable_declarator`) |
| Export-ancestor guard | ✓ | ✗ | ✗ (caught here: exports could be wrongly dropped) |
| `remove_unused_expression` swap in `handle_variable_declaration` | ✓ | ✗ | ✓ |
| Propagate `pure` onto inlined call/new | ✓ | ✓ (recursive into containers) | ✗ |
| Preserve IIFE wrapper when body has any side effect (DCE-only) | ✓ | ✗ | ✗ |
| Recursive propagation into sequence/conditional/etc bodies | ✗ | ✓ | ✗ |
| TS wrapper transparency in `may_have_side_effects` | ✗ | ✓ | ✗ |

Two mechanisms from #21526 are intentionally omitted here:

- **Recursive container propagation.** The DCE-only gate preserves the IIFE wrapper for those body shapes, so the outer call's `pure` flag stays visible to downstream tools. Recursive propagation also spreads any developer-lie annotation to inner sub-expressions, which this PR's restraint avoids.
- **TS wrapper transparency in `may_have_side_effects`.** Orthogonal; can land separately.

If this PR is accepted, #21526 and #22251 should be closed as superseded.

## Coverage

| Case | Output |
|---|---|
| `var x = /* @__PURE__ */ (() => stuff())()` (x unused — original bug) | empty |
| `var x = /* @__PURE__ */ (() => g.x)()` (x unused, non-call body) | empty |
| `var x = /* @__PURE__ */ foo(a)` (x unused, side-effectful arg) | `a;` |
| `var a = /* @__PURE__ */ (() => x)(); console.log(a)` (armano2 #1, full mode) | `var a = x; console.log(a);` |
| `(/* @__PURE__ */ (() => !0)() ? () => x() : () => {})()` (armano2 #2) | `x();` |
| `export const x = /* @__PURE__ */ (() => _M.exec)();` (lights0123, DCE mode) | preserves IIFE → rolldown can tree-shake |
| `export const x = /* @__PURE__ */ (() => foo(bar()))();` (DCE mode, sub-effects) | preserves IIFE → rolldown can tree-shake |
| `export function f() { var x = /* @__PURE__ */ (() => foo())(); } f();` | `export function f() {} f();` (function-local var is local, not export) |
| `var x = (async () => {})()` / `var x = (function* () {})()` (x unused) | empty (empty-IIFE branch via widened predicate) |

## End-to-end verification with rolldown

Locally patched rolldown to use this branch. Added a regression fixture (`tree_shaking/pure_iife_unused_export`) covering five body shapes.

**Before this PR** (rolldown bundle for unused-export module):

```js
//#region lib.js
function maybeExpensive() { ... }
function MaybeExpensiveCtor() { ... }
const _M = { exec: 1, tag: () => 1 };
maybeExpensive();
new MaybeExpensiveCtor();
_M.exec;
_M?.exec;
_M.tag`tpl`;
//#endregion
//#region main.js
console.log("hello");
//#endregion
```

**After this PR**:

```js
//#region main.js
console.log("hello");
//#endregion
```

Two pre-existing rolldown tests (`strict_execution_order/issue_5303`, `concatenate_wrapped_modules`) fail with the local patch — they use `/* #__PURE__ */ (() => globalValue)()` where `globalValue` is set by a sibling module's side effect. The annotation is technically incorrect; the tests previously "worked" only because oxc destroyed the annotation and rolldown defensively wrapped the module. These need a follow-up in rolldown to correct the annotations.

## Snapshots

- `minsize.snap` — unchanged.
- `allocs_minifier.snap` — `cal.com.tsx` +1 alloc (negligible); `antd.js` +1921 allocs (~0.6% of the 331k baseline). Comes from the new `may_have_side_effects` call in `iife_inline_would_lose_pure` plus the deeper rewrites in `remove_unused_expression`. Trade is correctness/tree-shaking-fidelity for a small intra-pass alloc bump; output size unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant