Fixes 2 tests and a crash

frog/gen.dart

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Top level generator object for writing code and keeping track of
  * dependencies.
  *
  * Should have two compilation models, but only one implemented so far.
  *
@@ skipping 113 matching lines @@
         var lib = toCheck.removeFirst();
         if (seen.contains(lib)) continue;
         seen.add(lib);
         lib.imports.forEach((i) => toCheck.addLast(lib));
         lib.types.getValues().forEach((t) => types.add(t));
       }
     }
     return types;
   }
 
-  GlobalValue globalForStaticField(FieldMember field, Value fieldValue,
+  GlobalValue globalForStaticField(FieldMember field, Value exp,
       List<Value> dependencies) {
     hasStatics = true;
-    var fullname = "${field.declaringType.jsname}.${field.jsname}";
-    if (!globals.containsKey(fullname)) {
-      globals[fullname] = new GlobalValue.fromStatic(
-          field, fieldValue, dependencies);
+    var key = "${field.declaringType.jsname}.${field.jsname}";
+    var ret = globals[key];
+    if (ret === null) {
+      ret = new GlobalValue(exp.type, exp.code, field.isFinal, field, null,
+        exp, exp.span, dependencies);
+      globals[key] = ret;
     }
-    return globals[fullname];
+    return ret;
   }
 
   GlobalValue globalForConst(EvaluatedValue exp, List<Value> dependencies) {
     // Include type name to ensure unique constants - this matches
     // the code above that includes the type name for static fields.
-    var key = exp.type.jsname + ':' + exp.canonicalCode;
-    if (!globals.containsKey(key)) {
-      globals[key] =
-        new GlobalValue.fromConst(globals.length, exp, dependencies);
+    var key = exp.type.jsname + ':' + exp.code;
+    var ret = globals[key];
+    if (ret === null) {
+      var name = "const\$${globals.length}";
+      ret = new GlobalValue(exp.type, name, true, null, name, exp,
+          exp.span, dependencies);
+      globals[key] = ret;
     }
-    assert(globals[key].type == exp.type);
-    return globals[key];
+    assert(ret.type == exp.type);
+    return ret;
   }
 
   writeTypes(Library lib) {
     if (lib.isWritten) return;
 
     // Do this first to be safe in the face of circular refs.
     lib.isWritten = true;
 
     // Ensure all imports have been written.
     for (var import in lib.imports) {
@@ skipping 1763 matching lines @@
     meth.generator.writeDefinition(w, node);
     return new Value(meth.functionType, w.text, node.span);
   }
 
   visitCallExpression(CallExpression node) {
     var target;
     var position = node.target;
     var name = ':call';
     if (node.target is DotExpression) {
       DotExpression dot = node.target;
-      // ????
-      if (dot.self is LiteralExpression) {
-        target = (new ParenExpression(dot.self, dot.self.span)).visit(this);
-      } else {
-        target = dot.self.visit(this);
-      }
+      target = dot.self.visit(this);
       name = dot.name.name;
       position = dot.name;
     } else if (node.target is VarExpression) {
       VarExpression varExpr = node.target;
       name = varExpr.name.name;
      // First check in block scopes.
       target = _scope.lookup(name);
       if (target != null) {
         return target.invoke(this, ':call', node, _makeArgs(node.arguments));
       }
@@ skipping 17 matching lines @@
     return (e is BinaryExpression || e is ConditionalExpression
             || e is PostfixExpression || _isUnaryIncrement(e));
   }
 
   visitBinaryExpression(BinaryExpression node, [bool isVoid = false]) {
     final kind = node.op.kind;
     // TODO(jimhug): Ensure these have same semantics as JS!
     if (kind == TokenKind.AND || kind == TokenKind.OR) {
       var x = visitTypedValue(node.x, world.nonNullBool);
       var y = visitTypedValue(node.y, world.nonNullBool);
-      final code = '${x.code} ${node.op} ${y.code}';
-      if (x.isConst && y.isConst) {
-        var value = (kind == TokenKind.AND)
-            ? x.actualValue && y.actualValue : x.actualValue || y.actualValue;
-        return Value.fromBool(value, node.span);
-      }
-      return new Value(world.nonNullBool, code, node.span);
+      return x.binop(kind, y, this, node);

[Jennifer Messerly, 2012/01/09 19:06:54]

so nice!

     } else if (kind == TokenKind.EQ_STRICT || kind == TokenKind.NE_STRICT) {
       var x = visitValue(node.x);
       var y = visitValue(node.y);
-      if (x.isConst && y.isConst) {
-        var xVal = x.actualValue;
-        var yVal = y.actualValue;
-
-        // Note: it is ok to use == and not === here since all of these
-        // constant comparisons are applied to doubles, bool, or strings.
-        // We need it for the compile-time evaluator because
-        // (9).toDouble() === 9.0 is false in dartvm.
-        var value = kind == TokenKind.EQ_STRICT ? xVal == yVal : xVal != yVal;
-        return Value.fromBool(value, node.span);
-      }
-      if (x.code == 'null' || y.code == 'null') {
-        // Switching to == ensures that null and undefined are interchangable.
-        final op = node.op.toString().substring(0,2);
-        return new Value(world.nonNullBool, '${x.code} $op ${y.code}',
-            node.span);
-      } else {
-        // TODO(jimhug): Resolve issue with undefined and null here.
-        return new Value(world.nonNullBool, '${x.code} ${node.op} ${y.code}',
-          node.span);
-      }
+      return x.binop(kind, y, this, node);
     }
 
     final assignKind = TokenKind.kindFromAssign(node.op.kind);
     if (assignKind == -1) {
       final x = visitValue(node.x);
       final y = visitValue(node.y);
-      var name = TokenKind.binaryMethodName(node.op.kind);
-      if (node.op.kind == TokenKind.NE) {
-        name = ':ne';
-      }
-      if (name == null) {
-        world.internalError('unimplemented binary op ${node.op}', node.span);
-        return;
-      }
-      return x.invoke(this, name, node, new Arguments(null, [y]));
+      return x.binop(kind, y, this, node);
     } else if ((assignKind != 0) && _expressionNeedsParens(node.y)) {
       return _visitAssign(assignKind, node.x,
           new ParenExpression(node.y, node.y.span), node, null, isVoid);
     } else {
       return _visitAssign(assignKind, node.x, node.y, node, null, isVoid);
     }
   }
 
   /**
    * Visits an assignment expression.
@@ skipping 158 matching lines @@
         this, xn.name.name, xn.name, y);
     if (tmptarget != target) freeTemp(tmptarget);
     return ret;
   }
 
   visitUnaryExpression(UnaryExpression node) {
     var value = visitValue(node.self);
     switch (node.op.kind) {
       case TokenKind.INCR:
       case TokenKind.DECR:
+        // TODO(jimhug): Requires non-null num to be correct.
         if (value.type.isNum) {
           return new Value(value.type, '${node.op}${value.code}', node.span);
         } else {
           // ++x becomes x += 1
           // --x becomes x -= 1
           // TODO(jimhug): Confirm that --x becomes x -= 1 as it is in VM.
           var kind = (TokenKind.INCR == node.op.kind ?
               TokenKind.ADD : TokenKind.SUB);
           // TODO(jimhug): Shouldn't need a full-expression here.
           var operand = new LiteralExpression(Value.fromInt(1, node.span),
             node.span);
 
           var assignValue = _visitAssign(kind, node.self, operand, node, null);
           return new Value(assignValue.type, '(${assignValue.code})',
               node.span);
         }
       case TokenKind.NOT:
         // TODO(jimhug): Issue #359 seeks to clarify this behavior.
         if (value.type.isBool && value.isConst) {
           var newVal = !value.actualValue;
-          return new EvaluatedValue(value.type, newVal, '${newVal}', node.span);
+          return Value.fromBool(newVal, node.span);
         } else {
           var newVal = value.convertTo(this, world.nonNullBool);
           return new Value(newVal.type, '!${newVal.code}', node.span);
         }
 
       case TokenKind.ADD:
         // TODO(jimhug): Issue #359 seeks to clarify this behavior.
         return value.convertTo(this, world.numType);
 
       case TokenKind.SUB:
@@ skipping 117 matching lines @@
 
     // Call the constructor on the type we want to construct.
     // NOTE: this is important for correct type checking of factories.
     // If the user calls "new Interface()" we want the result type to be the
     // interface, not the class.
     var target = new Value.type(type, typeRef.span);
     return m.invoke(this, node, target, _makeArgs(node.arguments));
   }
 
   visitListExpression(ListExpression node) {
-    // TODO(jimhug): Use node.type or other type inference here.
-    var argsCode = [];
     var argValues = [];
-    var type = null;
-    if (node.type != null) {
-      // The parser makes node.type a list type, we extract its type argument.
-      type = method.resolveType(node.type, true).typeArgsInOrder[0];
+    //var listType = node.isConst ? world.immutableListType : world.listType;

[Jennifer Messerly, 2012/01/09 19:06:54]

remove?

[jimhug, 2012/01/09 21:34:30]

Done.

+    var listType = world.listType;
+    var type = world.varType;
+    if (node.itemType != null) {
+      type = method.resolveType(node.itemType, true);
       if (node.isConst && (type is ParameterType || type.hasTypeParams)) {
         world.error('type parameter cannot be used in const list literals');
       }
+      listType = listType.getOrMakeConcreteType([type]);

[Jennifer Messerly, 2012/01/09 19:06:54]

correct generic List type, awesome :)

     }
     for (var item in node.values) {
       var arg = visitTypedValue(item, type);
       argValues.add(arg);
-      if (node.isConst) {
-        if (!arg.isConst) {
-          world.error('const list can only contain const values', item.span);
-          argsCode.add(arg.code);
-        } else {
-          argsCode.add(arg.canonicalCode);
-        }
-      } else {
-        argsCode.add(arg.code);
+      if (node.isConst && !arg.isConst) {
+        world.error('const list can only contain const values', arg.span);
       }
     }
 
     world.listFactoryType.markUsed();
 
-    final code = '[${Strings.join(argsCode, ", ")}]';
-    var value = new Value(world.listType, code, node.span);
-    if (node.isConst) {
-      final immutableList = world.immutableListType;
-      final immutableListCtor = immutableList.getConstructor('from');
-      final result = immutableListCtor.invoke(this, node,
-          new Value.type(value.type, node.span), new Arguments(null, [value]));
-      value = world.gen.globalForConst(
-          new ConstListValue(immutableList, argValues, 'const $code',
-              result.code, node.span),
-          argValues);
-    }
-    return value;
+    var ret = new ListValue(argValues, node.isConst, listType, node.span);
+    if (ret.isConst) return ret.getGlobalValue();
+    return ret;
   }
 
 
   visitMapExpression(MapExpression node) {
     // Special case the empty non-const map.
     if (node.items.length == 0 && !node.isConst) {
       return world.mapType.getConstructor('').invoke(this, node,
         new Value.type(world.mapType, node.span), Arguments.EMPTY);
     }
 
-    var argValues = [];
-    var argsCode = [];
-    var type = null;
-    if (node.type != null) {
-      // node.type is a map type, extract the type argument for the values.
-      type = method.resolveType(node.type, true).typeArgsInOrder[1];
-      if (node.isConst && (type is ParameterType || type.hasTypeParams)) {
+    var values = new List<Value>();

[Jennifer Messerly, 2012/01/09 19:06:54]

perhaps:
    var values = <Value>[];
?

[jimhug, 2012/01/09 21:34:30]

Done.

+    var valueType = world.varType, keyType = world.stringType;
+    var mapType = world.mapType; // TODO(jimhug): immutable type?
+    if (node.valueType !== null) {
+      if (node.keyType !== null) {
+        keyType = method.resolveType(node.keyType, true);
+        // TODO(jimhug): Would be nice to allow arbitrary keys here (this is
+        // currently not allowed by the spec).
+        if (!keyType.isString) {
+          world.error('the key type of a map literal must be "String"',
+              keyType.span);
+        }
+        if (node.isConst && (keyType is ParameterType || keyType.hasTypeParams)) {

[Jennifer Messerly, 2012/01/09 19:06:54]

long line

[Jennifer Messerly, 2012/01/09 19:06:54]

keyType.isUnboundType, or something like that? Where it's defined like:
class Type { ...; bool get isUnboundType() => hasTypeParameters; ... }
class ParameterType { ...; bool get isUnboundType() => true; ... }

or maybe hasTypeParameters should return "true" from ParameterType

[jimhug, 2012/01/09 21:34:30]

Done.

[jimhug, 2012/01/09 21:34:30]

Good idea - for longer-term cleanup.
On 2012/01/09 19:06:54, John Messerly wrote:

+          world.error('type parameter cannot be used in const map literals');
+        }
+      }
+
+      valueType = method.resolveType(node.valueType, true);
+      if (node.isConst && (valueType is ParameterType || valueType.hasTypeParams)) {
         world.error('type parameter cannot be used in const map literals');
       }
-    }
-    for (int i = 0; i < node.items.length; i += 2) {
-      // TODO(jimhug): Use node.type or other type inference here.
-      // TODO(jimhug): Would be nice to allow arbitrary keys here (this is
-      // currently not allowed by the spec).
-      var key = visitTypedValue(node.items[i], world.stringType);
-      final valueItem = node.items[i+1];
-      var value = visitTypedValue(valueItem, type);
-      argValues.add(key);
-      argValues.add(value);
 
-      if (node.isConst) {
-        if (!key.isConst || !value.isConst) {
-          world.error('const map can only contain const values',
-              valueItem.span);
-          argsCode.add(key.code);
-          argsCode.add(value.code);
-        } else {
-          argsCode.add(key.canonicalCode);
-          argsCode.add(value.canonicalCode);
-        }
-      } else {
-        argsCode.add(key.code);
-        argsCode.add(value.code);
-      }
+      mapType = mapType.getOrMakeConcreteType([keyType, valueType]);
     }
 
-    var argList = '[${Strings.join(argsCode, ", ")}]';
-    var items = new Value(world.listType, argList, node.span);
-    var tp = world.corelib.topType;
-    Member f = node.isConst ? tp.getMember('_constMap') : tp.getMember('_map');
-    var value = f.invoke(this, node, new Value.type(tp, null),
-      new Arguments(null, [items]));
+    for (int i = 0; i < node.items.length; i += 2) {
+      var key = visitTypedValue(node.items[i], keyType);
+      if (node.isConst && !key.isConst) {
+        world.error('const map can only contain const keys', key.span);
+      }
+      values.add(key);
 
-    if (node.isConst) {
-      value = new ConstMapValue(value.type, argValues, value.code,
-        value.code, value.span);
-      return world.gen.globalForConst(value, argValues);
-    } else {
-      return value;
+      var value = visitTypedValue(node.items[i+1], valueType);

[Jennifer Messerly, 2012/01/09 19:06:54]

nit: spacing on i + 1

[jimhug, 2012/01/09 21:34:30]

Done.

+      if (node.isConst && !value.isConst) {
+        world.error('const map can only contain const values', value.span);
+      }
+      values.add(value);
     }
+
+    var ret = new MapValue(values, node.isConst, mapType, node.span);
+    if (ret.isConst) return ret.getGlobalValue();
+    return ret;
   }
 
   visitConditionalExpression(ConditionalExpression node) {
     var test = visitBool(node.test);
     var trueBranch = visitValue(node.trueBranch);
     var falseBranch = visitValue(node.falseBranch);
 
     var code = '${test.code} ? ${trueBranch.code} : ${falseBranch.code}';
     return new Value(Type.union(trueBranch.type, falseBranch.type), code,
       node.span);
   }
 
   visitIsExpression(IsExpression node) {
     var value = visitValue(node.x);
     var type = method.resolveType(node.type, false);
     return value.instanceOf(this, type, node.span, node.isTrue);
   }
 
   visitParenExpression(ParenExpression node) {
     var body = visitValue(node.body);
-    if (body.isConst) {
-      return new EvaluatedValue(body.type, body.actualValue,
-          '(${body.canonicalCode})', node.span);
-    }
+    // Assumption implicit here that const values never need parens...
+    if (body.isConst) return body;
     return new Value(body.type, '(${body.code})', node.span);
   }
 
   visitDotExpression(DotExpression node) {
     // Types are legal targets of .
     var target = node.self.visit(this);
     return target.get_(this, node.name.name, node.name);
   }
 
   visitVarExpression(VarExpression node) {
@@ skipping 171 matching lines @@
       result.add(new Value(world.varType, '\$$i', null, /*needsTemp:*/false));
     }
     for (int i = bareCount; i < length; i++) {
       var name = getName(i);
       if (name == null) name = '\$$i';
       result.add(new Value(world.varType, name, null, /*needsTemp:*/false));
     }
     return new Arguments(nodes, result);
   }
 }