Skip to content

Box the generator wrapper as a heap-allocated ObjectType instead of a value tuple - #245

Merged
ASDAlexander77 merged 1 commit into
mainfrom
generator-param-by-ref
Jul 18, 2026
Merged

Box the generator wrapper as a heap-allocated ObjectType instead of a value tuple#245
ASDAlexander77 merged 1 commit into
mainfrom
generator-param-by-ref

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

Summary

  • Object literals in this compiler compile to value-typed tuples by default. The generator wrapper ({step, next()}, built by buildGeneratorWrapperDeclaration) has mutable identity (step advanced by .next()) but was still a value tuple — so passing a generator to a function, capturing it in a closure, or reassigning it (const b = a) all silently copied its state instead of aliasing it.
  • Root-cause fix: mark the synthetic wrapper literal with a new InternalFlags::BoxAsObject flag and heap-box it into mlir_ts::ObjectType (NewOp + StoreOp + CastOp — the same recipe castTupleToInterface already uses) instead of leaving it as a tuple. Property access on ObjectType already emits a PropertyRefOp directly on the pointer, so .next() now mutates shared storage for every alias.
  • This fixes the parameter-aliasing bug that PR Give const bindings with bound-method fields (generators) real identity storage #244 (const-storage fix) explicitly left open, plus closure-capture and plain-assignment aliasing, all in one change with no changes needed to function-type equality/assignability/generic-instantiation machinery (the wrapper is one consistent type everywhere, not a RefType-wrapped special case).
  • for...of/yield*/destructuring needed no changes — they discover next through generic property access, which already handles ObjectType.
  • Design docs included: docs/generator-object-wrapper-design.md (chosen approach, implemented) and docs/generator-param-by-ref-design.md (a RefType-parameter alternative that was analyzed and rejected for its function-type-equality blast radius — kept for the record).

Test plan

  • Debug build clean
  • 00generator_manual_next.ts, 00generator7.ts pass via JIT
  • 00generator_manual_next2.ts extended with permanent regression coverage for parameter aliasing (caller observes callee's .next() calls), closure-capture aliasing, and plain-assignment aliasing — all pass
  • Full suite: 350/350 JIT + 354/354 compile, 0 failures, first attempt
  • The chronic "losing this reference" warning on generator tests is gone (confirms the value/this type mismatch this fix addresses)

🤖 Generated with Claude Code

… value tuple

Object literals in this compiler compile to value-typed tuples by default,
even though the generator wrapper ({step, next()}) has mutable identity that
must be shared across aliases. This meant passing a generator to a function,
capturing it in a closure, or reassigning it (const b = a) all silently
copied its state instead of aliasing it.

Add InternalFlags::BoxAsObject to mark the synthetic wrapper literal built by
buildGeneratorWrapperDeclaration, and heap-box it (NewOp + StoreOp + CastOp,
the same recipe castTupleToInterface already uses) into ObjectType instead of
leaving it as a tuple. Property access on ObjectType already emits a
PropertyRefOp directly on the pointer, so .next() now mutates shared storage
for every alias.

Supersedes the const-storage-only fix in #244 for generators specifically,
and fixes the parameter-aliasing bug that fix explicitly left open.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 merged commit 7dc8ff8 into main Jul 18, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the generator-param-by-ref branch July 18, 2026 13:17
ASDAlexander77 added a commit that referenced this pull request Jul 18, 2026
… tuple cast (#248)

Generalizes the generator-wrapper's BoxAsObject boxing (PR #245) toward
supporting any object literal with methods: getFields now looks through
ObjectType to its storage type, object spread ({...obj}) handles a boxed
ObjectType source, and castTupleLikeVariants gained an ObjectType->tuple
unboxing cast. No behavior change for existing code paths; verified via
generator objects (the only literals boxed today) in new test
00object_boxed_infra.ts. Full suite: 352/352 JIT + 356/356 AOT.

See docs/object-literal-boxing-design.md for the full staged plan (this is
PR A of three).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
ASDAlexander77 added a commit that referenced this pull request Jul 18, 2026
…ype (#249)

Generalizes the generator-wrapper's BoxAsObject boxing (PR #245, PR #248)
to any object literal with a method or accessor: mlirGen(ObjectLiteralExpression)
now boxes whenever oli.methodInfos/methodInfosWithCaptures is non-empty, not
just synthetic wrappers carrying the flag. Every alias of such a literal
(second binding, parameter, closure capture, array/object element) now
shares mutable state, matching JS semantics; pure-data literals keep value
(struct) semantics.

The flip itself is two lines, but exercising ObjectType through every path
that previously only saw tuples for method-bearing literals surfaced five
more missing ObjectType cases, each fixed:
- boxing seed value routed through mlirGenCreateTuple instead of the
  generic cast pipeline, avoiding a spurious "losing this reference" warning
- TupleGetSetAccessor's get/set now handles a pointer (ObjectType/RefType)
  operand via PropertyRefOp+LoadOp, not just ExtractPropertyOp
- castObjectToInterface gained the same field-type-coercion fallback
  castTupleToInterface already had (e.g. si32 vs number)
- element access (obj[Symbol.x]) dispatches ObjectType like Class/Interface
- generic intersection-type merge (D & M) and parameter inference look
  through ObjectType's storage type, same as MLIRTypeHelper::getFields

New test 00object_ref_semantics.ts covers alias mutation through bindings,
parameters, closures, array/object elements, accessors, globals, and
conditional-expression merges. Full suite green: 353/353 JIT + 357/357 AOT.

See docs/object-literal-boxing-design.md (PR B of three staged PRs).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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