Skip to content

Commit

Permalink
Adds spread syntax in new expressions in ES5
Browse files Browse the repository at this point in the history
  • Loading branch information
tinganho committed May 12, 2015
1 parent 5320faa commit 8ca1a24
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 32 deletions.
8 changes: 4 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6585,7 +6585,7 @@ module ts {
result.splice(spliceIndex, 0, signature);
}
}

function getSpreadArgumentIndex(args: Expression[]): number {
for (let i = 0; i < args.length; i++) {
if (args[i].kind === SyntaxKind.SpreadElementExpression) {
Expand Down Expand Up @@ -7107,10 +7107,10 @@ module ts {
}

function resolveNewExpression(node: NewExpression, candidatesOutArray: Signature[]): Signature {
if (node.arguments && languageVersion < ScriptTarget.ES6) {
if (node.arguments && languageVersion < ScriptTarget.ES5) {
let spreadIndex = getSpreadArgumentIndex(node.arguments);
if (spreadIndex >= 0) {
error(node.arguments[spreadIndex], Diagnostics.Spread_operator_in_new_expressions_is_only_available_when_targeting_ECMAScript_6_and_higher);
error(node.arguments[spreadIndex], Diagnostics.Spread_operator_in_new_expressions_is_only_available_when_targeting_ECMAScript_5_and_higher);
}
}

Expand All @@ -7128,7 +7128,7 @@ module ts {
// If ConstructExpr's apparent type(section 3.8.1) is an object type with one or
// more construct signatures, the expression is processed in the same manner as a
// function call, but using the construct signatures as the initial set of candidate
// signatures for overload resolution.The result type of the function call becomes
// signatures for overload resolution. The result type of the function call becomes
// the result type of the operation.
expressionType = getApparentType(expressionType);
if (expressionType === unknownType) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ module ts {
The_0_operator_cannot_be_applied_to_type_symbol: { code: 2469, category: DiagnosticCategory.Error, key: "The '{0}' operator cannot be applied to type 'symbol'." },
Symbol_reference_does_not_refer_to_the_global_Symbol_constructor_object: { code: 2470, category: DiagnosticCategory.Error, key: "'Symbol' reference does not refer to the global Symbol constructor object." },
A_computed_property_name_of_the_form_0_must_be_of_type_symbol: { code: 2471, category: DiagnosticCategory.Error, key: "A computed property name of the form '{0}' must be of type 'symbol'." },
Spread_operator_in_new_expressions_is_only_available_when_targeting_ECMAScript_6_and_higher: { code: 2472, category: DiagnosticCategory.Error, key: "Spread operator in 'new' expressions is only available when targeting ECMAScript 6 and higher." },
Spread_operator_in_new_expressions_is_only_available_when_targeting_ECMAScript_5_and_higher: { code: 2472, category: DiagnosticCategory.Error, key: "Spread operator in 'new' expressions is only available when targeting ECMAScript 5 and higher." },
Enum_declarations_must_all_be_const_or_non_const: { code: 2473, category: DiagnosticCategory.Error, key: "Enum declarations must all be const or non-const." },
In_const_enum_declarations_member_initializer_must_be_constant_expression: { code: 2474, category: DiagnosticCategory.Error, key: "In 'const' enum declarations member initializer must be constant expression." },
const_enums_can_only_be_used_in_property_or_index_access_expressions_or_the_right_hand_side_of_an_import_declaration_or_export_assignment: { code: 2475, category: DiagnosticCategory.Error, key: "'const' enums can only be used in property or index access expressions or the right hand side of an import declaration or export assignment." },
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@
"category": "Error",
"code": 2471
},
"Spread operator in 'new' expressions is only available when targeting ECMAScript 6 and higher.": {
"Spread operator in 'new' expressions is only available when targeting ECMAScript 5 and higher.": {
"category": "Error",
"code": 2472
},
Expand Down
127 changes: 101 additions & 26 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1809,28 +1809,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
}

function emitCallWithSpread(node: CallExpression) {
let target: Expression;
let expr = skipParentheses(node.expression);
if (expr.kind === SyntaxKind.PropertyAccessExpression) {
// Target will be emitted as "this" argument
target = emitCallTarget((<PropertyAccessExpression>expr).expression);
write(".");
emit((<PropertyAccessExpression>expr).name);
}
else if (expr.kind === SyntaxKind.ElementAccessExpression) {
// Target will be emitted as "this" argument
target = emitCallTarget((<PropertyAccessExpression>expr).expression);
write("[");
emit((<ElementAccessExpression>expr).argumentExpression);
write("]");
}
else if (expr.kind === SyntaxKind.SuperKeyword) {
target = expr;
write("_super");
}
else {
emit(node.expression);
}
let { target } = emitCallTargetAndAccessor(node.expression);
write(".apply(");
if (target) {
if (target.kind === SyntaxKind.SuperKeyword) {
Expand Down Expand Up @@ -1881,13 +1860,109 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
}
}

/** Emit call target and accessor from an expression. The call target represents
* the default call context of "this". If it can be decided — this function will
* return the node representhing the call target — otherwise it returns undefined.
* The accessor is a property accessor of the target. If an expression has an

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 12, 2015

Since the word "accessor" means something specific in typescript, perhaps we should call this memberName or something like that

* accessor this function will both emit it and return it. To support spread syntax
* in TypeScript, we sometimes need to emit the target twice in an expression that
* only expresses it once. The target could be a call expression and therefore calling
* it twice would alter its' state differently as oppose to calling it once. This

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 12, 2015

remove apostrophe from its'

* function is very handy for those situations because it is emitting and returning
* the call target and the accessor. */
function emitCallTargetAndAccessor(node: Expression): CallTargetAndAccessor {
node = skipParentheses(node);
let target: Expression;
let accessor: Expression;
if (node.kind === SyntaxKind.PropertyAccessExpression) {
// Target will be emitted as "this" argument
target = emitCallTarget((<PropertyAccessExpression>node).expression);
accessor = (<PropertyAccessExpression>node).name;
write(".");
emit(accessor);
}
else if (node.kind === SyntaxKind.ElementAccessExpression) {
// Target will be emitted as "this" argument
target = emitCallTarget((<PropertyAccessExpression>node).expression);
accessor = (<ElementAccessExpression>node).argumentExpression;
write("[");
emit(accessor);
write("]");
}
else if (node.kind === SyntaxKind.SuperKeyword) {
target = node;
write("_super");
}
else {
emit(node);
}
return { target, accessor };
}

function emitNewExpression(node: NewExpression) {
write("new ");
emit(node.expression);
if (node.arguments) {

// Spread operator logic can be supported in new expressions in ES5 using a combination
// of Function.prototype.bind() and Function.prototype.apply().
//
// Example 1, with a non property accessor constructor:
//
// var arguments = [1, 2, 3, 4, 5];
// new Array(...arguments);
//
// Could be transpiled into ES5:
//
// var arguments = [1, 2, 3, 4, 5];
// new (Array.bind.apply(Array, [void 0].concat(arguments)));
//
// Example 2, with a property accessor constructor:
//
// var arguments = [1, 2, 3, 4, 5];
// new object.Array(...arguments);
//
// Could be transpiled into ES5:
//
// var arguments = [1, 2, 3, 4, 5];
// new ((_a = object.Array).bind.apply(_a, [void 0].concat(arguments)));
// var _a;
//
// `[void 0]` is the first argument which represents `thisArg` to the bind method above.
// And `thisArg` will be set to the return value of the constructor when instantiated
// with the new operator — regardless of any value we set `thisArg` to. Thus, we set it
// to an empty object, `void 0`.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 12, 2015

Not an empty object, more like undefined I think

if (languageVersion === ScriptTarget.ES5 &&
node.arguments &&
hasSpreadElement(node.arguments)) {

write("(");
emitCommaList(node.arguments);
write(")");
let { target, accessor } = emitCallTargetAndAccessor(node.expression);
write(".bind.apply(");
if (target) {
emit(target);
if (accessor.kind === SyntaxKind.Identifier) {
write(".");

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 12, 2015

Sorry to be dense, but I am confused about something. It seems like whenever you have a property access or an element access, you assign to a temp, and then emit the temp as the first argument to apply. So when would you ever emit the dot with the member name here, or the brackets with the member name below?

I wonder if it makes more sense to just emit the whole expression when you emit the target the first time, and not do the special casing for emitCallTarget. Seems like you do not care about the target for a new expression.

This comment has been minimized.

Copy link
@tinganho

tinganho May 12, 2015

Author Owner

So when would you ever emit the dot with the member name here, or the brackets with the member name below?

I would emit it every time the accessor is defined. So all these cases:

new object.accessor(..args)// accessor is an identifier
new object[1](..args)// accessor is a numeric literal
new object[foo.bar](..args)// accessor is a property access expression

And I will emit the original node expression if the accessor is undefined:

new object(...arg);

I wonder if it makes more sense to just emit the whole expression when you emit the target the first time, and not do the special casing for emitCallTarget. Seems like you do not care about the target for a new expression.

I'm not sure I understand. Can you explain in code?

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 12, 2015

You do emit those constructs, yes. But you only emit them inside emitCallTargetAndAccessor. By the time you get out here to emitNewExpression, you have an identifier instead (namely the temp that you created for the target and accessor). This is because you only evaluate such expressions once. So the dot and bracket code would be necessary in emitCallTargetAndAccessor, but not in emitNewExpression.

What I am trying to say is that you don't have to call emitCallTargetAndAccessor. Instead, you can just call emitCallTarget on node.expression (the same place you call emitCallTargetAndAccessor now), and then get the temp that results from that. Then when you are ready to emit the thisArg, you can just emit the temp that you got from emitCallTarget.

This also means that emitCallTargetAndAccessor is only used in emitCallWithSpread, and it doesn't have to return the accessor anymore.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 13, 2015

Another way to think about is that the distinction between target and accessor is only significant for call expressions, not new expressions.

This comment has been minimized.

Copy link
@tinganho

tinganho May 13, 2015

Author Owner

What I am trying to say is that you don't have to call emitCallTargetAndAccessor. Instead, you can just call emitCallTarget on node.expression (the same place you call emitCallTargetAndAccessor now), and then get the temp that results from that. Then when you are ready to emit the thisArg, you can just emit the temp that you got from emitCallTarget.
This also means that emitCallTargetAndAccessor is only used in emitCallWithSpread, and it doesn't have to return the accessor anymore.

You still need to emit the accessor(for thisArg)? just emitting the temp variable won't work. And you would need the additional node checks to emit the accessor.

I could write as you said just emitting the with emitCallTarget() where the emitCallTargetAndAccessor() is.
and then emit the accessor(if target is defined):

if (node.kind === SyntaxKind.PropertyAccessExpression) {
   write(".");
   emit((<PropertyAccessExpression>node).name);
}
else if (node.kind === SyntaxKind.ElementAccessExpression) {
   write("[");
   emit((<ElementAccessExpression>node).argumentExpression);
   write("]");
}

hmm. I just figured out that my example was wrong in the comments above should be:

new ((_a = object).Array.bind.apply(_a.Array, [void 0].concat(arguments)));

This comment has been minimized.

Copy link
@tinganho

tinganho May 13, 2015

Author Owner

I could write as you said just emitting the with emitCallTarget() where the emitCallTargetAndAccessor() is.
and then emit the accessor(if target is defined):
if (node.kind === SyntaxKind.PropertyAccessExpression) {
write(".");
emit((node).name);
}
else if (node.kind === SyntaxKind.ElementAccessExpression) {
write("[");
emit((node).argumentExpression);
write("]");
}

Just figured out that this doesn't work either. emitCallTarget doesn't emit the accessor. I think we need emitCallTargetAndAccessor.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 13, 2015

Oh I thought your example was correct. That's what my argument was based on. In that case, why is it not

new ((_a = object.Array).bind.apply(_a, [void 0].concat(arguments)));

?

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman May 13, 2015

Or for that matter, couldn't you alternatively emit the following?

new (Function.bind.apply(object.Array, [void 0].concat(arguments)));

This would reduce the complexity, as it would eliminate the reliance on a temp.

This comment has been minimized.

Copy link
@tinganho

tinganho May 13, 2015

Author Owner

Oh I thought your example was correct. That's what my argument was based on. In that case, why is it not

I think I could write it like that. It was just that I thought that we don't share much code with emitCallWithSpread then. Then I need a new emitCallTarget that assign a temp on the target+accessor and not just the target.

I can investigate if it works with Function, because that would be a great solution.

This comment has been minimized.

Copy link
@tinganho

tinganho May 13, 2015

Author Owner

@JsonFreeman I can confirm that the Function idea works 😄 ! I think I can rewrite this to a much simpler commit. I will restore emitCallTargetAndAccessor where it belonged then.

emit(accessor);
}
else {
write("[");
emit(accessor);
write("]");
}
}
else {
emit(node.expression);
}
write(", [void 0].concat(");
emitListWithSpread(node.arguments, /*multiline*/false, /*trailingComma*/false);
write(")))");
}
else {
emit(node.expression);
if (node.arguments) {
write("(");
emitCommaList(node.arguments);
write(")");
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,15 @@ module ts {
originalKeywordKind?: SyntaxKind; // Original syntaxKind which get set so that we can report an error later
}

/** Target and accessor from a function like call. Target is the object that represent the default call context
* "this". The accessor is a property member of the target used in a call expression. This interface is only
* used in the emitter for emitting expressions with spread syntax.*/
/* @internal */
export interface CallTargetAndAccessor {
target?: Expression;
accessor?: Expression;
}

export interface QualifiedName extends Node {
// Must have same layout as PropertyAccess
left: EntityName;
Expand Down

0 comments on commit 8ca1a24

Please sign in to comment.