Skip to content

Commit

Permalink
[dartdevc] Add implicit casts for SpreadElement nodes
Browse files Browse the repository at this point in the history
Issue: flutter#36006
Change-Id: Ie7252f70bd62d9b2b2382335a7853f5eb7f09480
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97684
Reviewed-by: Jenny Messerly <jmesserly@google.com>
  • Loading branch information
nshahan committed Mar 25, 2019
1 parent f09097b commit 093c290
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 28 deletions.
102 changes: 75 additions & 27 deletions pkg/dev_compiler/lib/src/analyzer/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6530,49 +6530,97 @@ class CodeGenerator extends Object

@override
JS.Statement visitSpreadElement(SpreadElement node) {
/// Returns `true` if [node] is or is a sub element of a Map literal.
isMap(AstNode node) {
/// Returns `true` if [node] is or is a child element of a map literal.
bool isMap(AstNode node) {
if (node is SetOrMapLiteral) return node.isMap;
if (node is ListLiteral) return false;
return isMap(node.parent);
}

/// Appends all elements in [spreadItems] to the collection being built.
/// Returns [expression] wrapped in an implict cast to [castType] or
/// [expression] as provided if [castType] is `null` signifying that
/// no cast is needed.
JS.Expression wrapInImplicitCast(JS.Expression expression, castType) =>
castType == null ? expression : _emitCast(castType, expression);

/// Returns a statement spreading the elements of [expression] into
/// [_currentCollectionVariable].
///
/// Walks the parent nodes starting at [parent] to determine if this is
/// spread is in a Map literal.
emitSpread(AstNode parent, JS.Expression spreadExpression) {
if (isMap(parent)) {
return js.statement('#.forEach((k, v) => {#.push(k); #.push(v)})', [
spreadExpression,
_currentCollectionVariable,
_currentCollectionVariable
]);
/// Expects the collection literal containing [expression] to be a list or
/// set literal. Inserts implicit casts to [elementCastType] for each
/// element if needed.
JS.Statement emitListOrSetSpread(
JS.Expression expression, DartType elementCastType) {
var forEachTemp =
_emitSimpleIdentifier(_createTemporary('i', types.dynamicType));
return js.statement('#.forEach((#) => #.push(#))', [
expression,
forEachTemp,
_currentCollectionVariable,
wrapInImplicitCast(forEachTemp, elementCastType)
]);
}

/// Returns a statement spreading the key/value pairs of [expression]
/// into [_currentCollectionVariable].
///
/// Expects the collection literal containing [expression] to be a map
/// literal. Inserts implicit casts to [keyCastType] for keys and
/// [valueCastType] for values if needed.
JS.Statement emitMapSpread(JS.Expression expression, DartType keyCastType,
DartType valueCastType) {
var keyTemp =
_emitSimpleIdentifier(_createTemporary('k', types.dynamicType));
var valueTemp =
_emitSimpleIdentifier(_createTemporary('v', types.dynamicType));
return js.statement('#.forEach((#, #) => {#.push(#); #.push(#)})', [
expression,
keyTemp,
valueTemp,
_currentCollectionVariable,
wrapInImplicitCast(keyTemp, keyCastType),
_currentCollectionVariable,
wrapInImplicitCast(valueTemp, valueCastType)
]);
}

/// Appends all elements in [expression] to the collection being built.
///
/// Uses implict cast information from [node] to insert the correct casts
/// for the collection elements when spreading. Inspects parents of [node]
/// to determine the type of the enclosing collection literal.
JS.Statement emitSpread(JS.Expression expression, Expression node) {
expression = wrapInImplicitCast(expression, getImplicitCast(node));

// Start searching for a map literal at the parent of the SpreadElement.
if (isMap(node.parent.parent)) {
return emitMapSpread(expression, getImplicitSpreadKeyCast(node),
getImplicitSpreadValueCast(node));
}
return js.statement('#.forEach((i) => #.push(i))',
[spreadExpression, _currentCollectionVariable]);
return emitListOrSetSpread(expression, getImplicitSpreadCast(node));
}

/// Emits a null check on [spreadExpression] then appends all elements to
/// the collection being built.
/// Emits a null check on [expression] then appends all elements to the
/// collection being built.
///
/// Walks the parent nodes starting at [parent] to determine if this is
/// spread is in a Map literal.
emitNullSafeSpread(AstNode parent, JS.Expression spreadExpression) {
/// Uses implict cast information from [node] to insert the correct casts
/// for the collection elements when spreading. Inspects parents of [node]
/// to determine the type of the enclosing collection literal.
JS.Statement emitNullSafeSpread(JS.Expression expression, Expression node) {
// TODO(nshahan) Could optimize out if we know the value is null.
var spreadItems = _emitSimpleIdentifier(
_createTemporary('items', getStaticType(node.expression)));
var spreadItems =
_emitSimpleIdentifier(_createTemporary('items', getStaticType(node)));
return JS.Block([
js.statement('let # = #', [spreadItems, spreadExpression]),
js.statement('if (# != null) #',
[spreadItems, emitSpread(node.parent, spreadItems)])
js.statement('let # = #', [spreadItems, expression]),
js.statement(
'if (# != null) #', [spreadItems, emitSpread(spreadItems, node)])
]);
}

var spreadExpression = _visitExpression(node.expression);
var expression = _visitExpression(node.expression);
return node.beginToken.lexeme == '...?'
? emitNullSafeSpread(node.parent, spreadExpression)
: emitSpread(node.parent, spreadExpression);
? emitNullSafeSpread(expression, node.expression)
: emitSpread(expression, node.expression);
}

@override
Expand Down
14 changes: 13 additions & 1 deletion pkg/dev_compiler/lib/src/analyzer/reify_coercions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:analyzer/dart/element/type.dart' show DartType;
import 'package:analyzer/src/dart/ast/ast.dart' show FunctionBodyImpl;
import 'package:analyzer/src/dart/ast/utilities.dart'
show AstCloner, NodeReplacer;
import 'package:analyzer/src/dart/element/type.dart' show DynamicTypeImpl;
import 'package:analyzer/src/generated/parser.dart' show ResolutionCopier;
import 'package:analyzer/src/task/strong/ast_properties.dart' as ast_properties;

Expand Down Expand Up @@ -66,6 +65,13 @@ class CoercionReifier extends GeneralizingAstVisitor<void> {
}
}

@override
visitSpreadElement(SpreadElement node) {
// Skip visiting the expression so we can handle all casts during code
// generation.
node.expression.visitChildren(this);
}

@override
visitMethodInvocation(MethodInvocation node) {
if (isInlineJS(node.methodName.staticElement)) {
Expand Down Expand Up @@ -128,6 +134,12 @@ class _TreeCloner extends AstCloner {
clone, ast_properties.getImplicitCast(node));
ast_properties.setImplicitOperationCast(
clone, ast_properties.getImplicitOperationCast(node));
ast_properties.setImplicitSpreadCast(
clone, ast_properties.getImplicitSpreadCast(node));
ast_properties.setImplicitSpreadKeyCast(
clone, ast_properties.getImplicitSpreadKeyCast(node));
ast_properties.setImplicitSpreadValueCast(
clone, ast_properties.getImplicitSpreadValueCast(node));
ast_properties.setIsDynamicInvoke(
clone, ast_properties.isDynamicInvoke(node));
}
Expand Down

0 comments on commit 093c290

Please sign in to comment.