Skip to content

Commit fae67c7

Browse files
committed
WIP fix issues with destructuring
1 parent 93057c9 commit fae67c7

File tree

9 files changed

+514
-100
lines changed

9 files changed

+514
-100
lines changed

rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,15 @@ private void visitExpression(Node node, int contextFlags) {
806806
}
807807
break;
808808

809+
case Token.TO_ITERABLE_ARRAY:
810+
visitExpression(child, 0);
811+
addToken(type);
812+
// Emit element count for iterator finalization
813+
int elemCount = node.getIntProp(Node.DESTRUCTURING_ARRAY_LENGTH, 0);
814+
// Clamp to byte range (0-255), 0 means "unknown"
815+
addUint8(Math.min(elemCount, 255));
816+
break;
817+
809818
case Token.GET_REF:
810819
case Token.DEL_REF:
811820
visitExpression(child, 0);
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
package org.mozilla.javascript;
6+
7+
import java.util.ArrayList;
8+
import java.util.List;
9+
10+
/**
11+
* A wrapper around an iterator for use in array destructuring. This allows destructuring to consume
12+
* iterator values one-by-one on demand, rather than collecting all values upfront. This is
13+
* essential for handling infinite iterators and properly implementing iterator closing semantics
14+
* per ES6 spec.
15+
*
16+
* <p>Key behaviors:
17+
*
18+
* <ul>
19+
* <li>Elements are fetched lazily from the iterator as they are accessed
20+
* <li>Caches fetched values to support multiple accesses to the same index
21+
* <li>Tracks whether the iterator has been exhausted
22+
* <li>Calls iterator.return() when closed early (not exhausted)
23+
* </ul>
24+
*/
25+
public class DestructuringIterator extends IdScriptableObject {
26+
private static final long serialVersionUID = 1L;
27+
28+
private final Context cx;
29+
private final Scriptable scope;
30+
private final Callable nextFunc;
31+
private final Scriptable iteratorThis;
32+
private final Callable returnFunc;
33+
private final List<Object> cachedValues;
34+
private boolean done;
35+
private boolean closed;
36+
private int elementsNeeded;
37+
private boolean finalized;
38+
39+
public DestructuringIterator(Context cx, Scriptable scope, Object iteratorObj) {
40+
this.cx = cx;
41+
this.scope = scope;
42+
this.cachedValues = new ArrayList<>();
43+
this.done = false;
44+
this.closed = false;
45+
this.elementsNeeded = Integer.MAX_VALUE;
46+
this.finalized = false;
47+
48+
// Get the "next" function
49+
ScriptRuntime.LookupResult nextCall =
50+
ScriptRuntime.getPropAndThis(iteratorObj, ES6Iterator.NEXT_METHOD, cx, scope);
51+
this.nextFunc = nextCall.getCallable();
52+
this.iteratorThis = nextCall.getThis();
53+
54+
// Get the "return" function (may be null/undefined)
55+
Object retObj =
56+
ScriptRuntime.getObjectPropNoWarn(
57+
iteratorObj, ES6Iterator.RETURN_PROPERTY, cx, scope);
58+
if ((retObj != null) && !Undefined.isUndefined(retObj)) {
59+
if (!(retObj instanceof Callable)) {
60+
throw ScriptRuntime.notFunctionError(
61+
iteratorObj, retObj, ES6Iterator.RETURN_PROPERTY);
62+
}
63+
this.returnFunc = (Callable) retObj;
64+
} else {
65+
this.returnFunc = null;
66+
}
67+
}
68+
69+
@Override
70+
public String getClassName() {
71+
return "DestructuringIterator";
72+
}
73+
74+
/**
75+
* Set the number of elements needed for destructuring. This allows auto-finalization once
76+
* enough elements have been fetched.
77+
*/
78+
public void setElementsNeeded(int count) {
79+
this.elementsNeeded = count;
80+
}
81+
82+
@Override
83+
public Object get(int index, Scriptable start) {
84+
// Fetch values up to the requested index
85+
while (cachedValues.size() <= index && !done) {
86+
fetchNext();
87+
}
88+
89+
// Auto-finalize if we've fetched all needed elements
90+
if (!finalized && cachedValues.size() >= elementsNeeded) {
91+
finalizeIfNeeded();
92+
}
93+
94+
if (index < cachedValues.size()) {
95+
return cachedValues.get(index);
96+
}
97+
98+
// Index beyond what iterator produced - return undefined
99+
return Undefined.instance;
100+
}
101+
102+
/** Fetch the next value from the iterator. */
103+
private void fetchNext() {
104+
if (done) {
105+
return;
106+
}
107+
108+
try {
109+
Object result = nextFunc.call(cx, scope, iteratorThis, ScriptRuntime.emptyArgs);
110+
// Check the "done" property
111+
Object doneVal =
112+
ScriptableObject.getProperty(
113+
ScriptableObject.ensureScriptable(result), ES6Iterator.DONE_PROPERTY);
114+
if (doneVal == Scriptable.NOT_FOUND) {
115+
doneVal = Undefined.instance;
116+
}
117+
118+
if (ScriptRuntime.toBoolean(doneVal)) {
119+
done = true;
120+
return;
121+
}
122+
123+
// Get the value
124+
Object value =
125+
ScriptRuntime.getObjectPropNoWarn(
126+
result, ES6Iterator.VALUE_PROPERTY, cx, scope);
127+
cachedValues.add(value);
128+
} catch (Exception e) {
129+
// Error during iteration - close the iterator
130+
closeIterator();
131+
throw e;
132+
}
133+
}
134+
135+
/**
136+
* Close the iterator by calling its return() method. Should be called when destructuring
137+
* completes without exhausting the iterator.
138+
*/
139+
public void closeIterator() {
140+
if (closed || returnFunc == null) {
141+
return;
142+
}
143+
144+
closed = true;
145+
try {
146+
returnFunc.call(cx, scope, iteratorThis, ScriptRuntime.emptyArgs);
147+
} catch (Exception e) {
148+
// Errors from return() are typically ignored in destructuring cleanup
149+
}
150+
}
151+
152+
/** Finalize destructuring if not already done. Closes iterator if not exhausted. */
153+
private void finalizeIfNeeded() {
154+
if (finalized) {
155+
return;
156+
}
157+
finalized = true;
158+
159+
// If we're not exhausted after getting what we need, close the iterator
160+
if (!done) {
161+
closeIterator();
162+
}
163+
// Otherwise, iterator was exhausted normally - don't call return()
164+
}
165+
}

rhino/src/main/java/org/mozilla/javascript/Interpreter.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,7 @@ private abstract static class InstructionClass {
15811581
instructionObjs[base + Token.REF_NS_MEMBER] = new DoRefNsMember();
15821582
instructionObjs[base + Token.REF_NAME] = new DoRefName();
15831583
instructionObjs[base + Token.REF_NS_NAME] = new DoRefNsName();
1584+
instructionObjs[base + Token.TO_ITERABLE_ARRAY] = new DoToIterableArray();
15841585
instructionObjs[base + Icode_SCOPE_LOAD] = new DoScopeLoad();
15851586
instructionObjs[base + Icode_SCOPE_SAVE] = new DoScopeSave();
15861587
instructionObjs[base + Icode_SPREAD] = new DoSpread();
@@ -4217,6 +4218,26 @@ NewState execute(Context cx, CallFrame frame, InterpreterState state, int op) {
42174218
}
42184219
}
42194220

4221+
private static class DoToIterableArray extends InstructionClass {
4222+
@Override
4223+
NewState execute(Context cx, CallFrame frame, InterpreterState state, int op) {
4224+
final Object obj = frame.stack[state.stackTop];
4225+
// Wrap the iterator for element-by-element access
4226+
DestructuringIterator wrapper =
4227+
(DestructuringIterator)
4228+
ScriptRuntime.wrapDestructuringIterator(obj, cx, frame.scope);
4229+
// Get element count from bytecode if available
4230+
int elementsNeeded = frame.idata.itsICode[frame.pc] & 0xFF;
4231+
if (elementsNeeded == 0) {
4232+
elementsNeeded = Integer.MAX_VALUE; // Unknown, fetch as needed
4233+
}
4234+
wrapper.setElementsNeeded(elementsNeeded);
4235+
frame.stack[state.stackTop] = wrapper;
4236+
frame.pc++; // Skip the element count byte
4237+
return null;
4238+
}
4239+
}
4240+
42204241
private static class DoScopeLoad extends InstructionClass {
42214242
@Override
42224243
NewState execute(Context cx, CallFrame frame, InterpreterState state, int op) {

0 commit comments

Comments
 (0)