Skip to content

Commit

Permalink
Allow iteration over the 'arguments' object even if it escapes and ca…
Browse files Browse the repository at this point in the history
…nnot be detected. Note that this causes some incorrect code to "work" when transpiled.

let obj = {
  0: 'x',
  1: 'y',
  length: 2,
};
for (let x of obj) {}

should throw a TypeError, but will not when transpiled. However, if typechecking is enabled there will be a compiler error.

Fixes #1303
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=116058485
  • Loading branch information
tbreisacher authored and blickly committed Mar 2, 2016
1 parent 0affbf5 commit 6a40212
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 112 deletions.
2 changes: 1 addition & 1 deletion externs/es6.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Generator.prototype.return = function(value) {};
Generator.prototype.throw = function(exception) {};


// TODO(johnlenz): Array should be Iterable.
// TODO(johnlenz): Array and Arguments should be Iterable.



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ private void visitArrayPattern(NodeTraversal t, Node arrayPattern, Node parent)
String tempVarName = DESTRUCTURING_TEMP_VAR + (destructuringVarCounter++);
Node tempDecl = IR.var(
IR.name(tempVarName),
makeIterator(t, compiler, rhs.detachFromParent()));
makeIterator(compiler, rhs.detachFromParent()));
tempDecl.useSourceInfoIfMissingFromForTree(arrayPattern);
nodeToDetach.getParent().addChildBefore(tempDecl, nodeToDetach);

Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/Es6RewriteGenerators.java
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ private void visitYieldFor(NodeTraversal t, Node n, Node parent) {
Node enclosingStatement = NodeUtil.getEnclosingStatement(n);
Node generator = IR.var(
IR.name(GENERATOR_YIELD_ALL_NAME),
makeIterator(t, compiler, n.removeFirstChild()));
makeIterator(compiler, n.removeFirstChild()));
Node entryDecl = IR.var(IR.name(GENERATOR_YIELD_ALL_ENTRY));
Node assignIterResult =
IR.assign(
Expand Down
33 changes: 7 additions & 26 deletions src/com/google/javascript/jscomp/Es6ToEs3Converter.java
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ private void visitForOf(NodeTraversal t, Node node, Node parent) {
Node iterResult = IR.name(ITER_RESULT + variableName);
iterResult.makeNonIndexable();

Node init = IR.var(iterName.cloneTree(), makeIterator(t, compiler, iterable));
Node init = IR.var(iterName.cloneTree(), makeIterator(compiler, iterable));
Node initIterResult = iterResult.cloneTree();
initIterResult.addChildToFront(getNext.cloneTree());
init.addChildToBack(initIterResult);
Expand Down Expand Up @@ -397,7 +397,7 @@ private void visitArrayLitOrCallWithSpread(NodeTraversal t, Node node, Node pare
currGroup = null;
}
compiler.needsEs6Runtime = true;
groups.add(arrayFromIterable(t, compiler, currElement.removeFirstChild()));
groups.add(arrayFromIterable(compiler, currElement.removeFirstChild()));
} else {
if (currGroup == null) {
currGroup = IR.arraylit();
Expand Down Expand Up @@ -937,36 +937,17 @@ private void cannotConvertYet(Node n, String feature) {
}

/**
* Returns a call to {@code $jscomp.makeIterator} with {@code iterable} as its argument, unless
* {@code iterable} is the special {@code arguments} variable, in which case the returned Node is:
* {@code $jscomp.makeIterator($jscomp.arrayFromArguments(iterable))}.
* Returns a call to {@code $jscomp.makeIterator} with {@code iterable} as its argument.
*/
static Node makeIterator(NodeTraversal t, AbstractCompiler compiler, Node iterable) {
if (iterable.isName()) {
Var var = t.getScope().getVar(iterable.getString());
if (var != null && var.isArguments()) {
iterable = IR.call(
NodeUtil.newQName(compiler, "$jscomp.arrayFromArguments"),
iterable);
}
}
static Node makeIterator(AbstractCompiler compiler, Node iterable) {
return callEs6RuntimeFunction(compiler, iterable, "makeIterator");
}

/**
* Returns a call to $jscomp.arrayFromIterable with {@code iterable} as its argument, unless
* {@code iterable} is the special {@code arguments} variable, in which case
* {@code $jscomp.arrayFromArguments} is called instead.
* Returns a call to $jscomp.arrayFromIterable with {@code iterable} as its argument.
*/
private static Node arrayFromIterable(NodeTraversal t, AbstractCompiler compiler, Node iterable) {
String fnName = "arrayFromIterable";
if (iterable.isName()) {
Var var = t.getScope().getVar(iterable.getString());
if (var != null && var.isArguments()) {
fnName = "arrayFromArguments";
}
}
return callEs6RuntimeFunction(compiler, iterable, fnName);
private static Node arrayFromIterable(AbstractCompiler compiler, Node iterable) {
return callEs6RuntimeFunction(compiler, iterable, "arrayFromIterable");
}

private static Node callEs6RuntimeFunction(
Expand Down
23 changes: 3 additions & 20 deletions src/com/google/javascript/jscomp/js/es6/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ $jscomp.initSymbolIterator = function() {
/**
* Creates an iterator for the given iterable.
*
* @param {string|!Array<T>|!Iterable<T>|!Iterator<T>} iterable
* @param {string|!Array<T>|!Iterable<T>|!Iterator<T>|!Arguments<T>} iterable
* @return {!Iterator<T>}
* @template T
* @suppress {reportUnknownTypes}
Expand All @@ -101,10 +101,7 @@ $jscomp.makeIterator = function(iterable) {
if (iterable[$jscomp.global.Symbol.iterator]) {
return iterable[$jscomp.global.Symbol.iterator]();
}
if (!(iterable instanceof Array) && typeof iterable != 'string' &&
!(iterable instanceof String)) {
throw new TypeError(iterable + ' is not iterable');
}

let index = 0;
return /** @type {!Iterator} */ ({
next() {
Expand Down Expand Up @@ -142,7 +139,7 @@ $jscomp.arrayFromIterator = function(iterator) {

/**
* Copies the values from an Iterable into an Array.
* @param {string|!Array<T>|!Iterable<T>} iterable
* @param {string|!Array<T>|!Iterable<T>|!Arguments<T>} iterable
* @return {!Array<T>}
* @template T
*/
Expand All @@ -155,20 +152,6 @@ $jscomp.arrayFromIterable = function(iterable) {
};


/**
* Copies the values from an Arguments object into an Array.
* @param {!Arguments} args
* @return {!Array}
*/
$jscomp.arrayFromArguments = function(args) {
const result = [];
for (let i = 0; i < args.length; i++) {
result.push(args[i]);
}
return result;
};


/**
* Inherit the prototype methods and static methods from one constructor
* into another.
Expand Down
16 changes: 2 additions & 14 deletions src/com/google/javascript/jscomp/js/es6_runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,12 @@
/**@suppress {reportUnknownTypes}
@template T
@param {(string|!Array<T>|!Iterable<T>|!Iterator<T>)} iterable
@param {(string|!Array<T>|!Iterable<T>|!Iterator<T>|!Arguments<T>)} iterable
@return {!Iterator<T>} */$jscomp.makeIterator = function(iterable) {
$jscomp.initSymbolIterator();
if (iterable[$jscomp.global.Symbol.iterator]) {
return iterable[$jscomp.global.Symbol.iterator]();
}
if (!(iterable instanceof Array) && typeof iterable != "string" && !(iterable instanceof String)) {
throw new TypeError(iterable + " is not iterable");
}
var index = 0;
return /**@type {!Iterator} */({next:function() {
if (index == iterable.length) {
Expand All @@ -81,7 +78,7 @@
};
/**@template T
@param {(string|!Array<T>|!Iterable<T>)} iterable
@param {(string|!Array<T>|!Iterable<T>|!Arguments<T>)} iterable
@return {!Array<T>} */$jscomp.arrayFromIterable = function(iterable) {
if (iterable instanceof Array) {
return iterable;
Expand All @@ -90,15 +87,6 @@
}
};
/**
@param {!Arguments} args
@return {!Array} */$jscomp.arrayFromArguments = function(args) {
/**@const */var result = [];
for (var i = 0;i < args.length;i++) {
result.push(args[i]);
}
return result;
};
/**
@param {!Function} childCtor
@param {!Function} parentCtor */$jscomp.inherits = function(childCtor, parentCtor) {
/**@constructor */function tempCtor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,7 @@ public void testArrayDestructuringArguments() {
"function f() { var [x, y] = arguments; }",
LINE_JOINER.join(
"function f() {",
" var $jscomp$destructuring$var0 = $jscomp.makeIterator(",
" $jscomp.arrayFromArguments(arguments));",
" var $jscomp$destructuring$var0 = $jscomp.makeIterator(arguments);",
" var x = $jscomp$destructuring$var0.next().value;",
" var y = $jscomp$destructuring$var0.next().value;",
"}"));
Expand Down
Loading

0 comments on commit 6a40212

Please sign in to comment.