first stage of Value cleanups

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 489 matching lines @@
     }
 
     // Make sure variables don't shadow any names we might need to access.
     if (!isParameter) {
       int index = 0;
       while (_isDefinedInParent(jsName)) {
         jsName = '$name${index++}';
       }
     }
 
-    var ret = new Value(type, jsName, false, false); // TODO: needsTemp:false);
+    var ret = new Value(type, jsName, location != null ? location.span : null,
+      false, false);
     _vars[name] = ret;
     return ret;
   }
 
   Value declareParameter(Parameter p) {
     return create(p.name, p.type, p.definition, isParameter:true);
   }
 
   /** Declares a variable in the current scope for this identifier. */
   Value declare(DeclaredIdentifier id) {
@@ skipping 51 matching lines @@
     // For named lambdas, add the name to this scope so we can call it
     // recursively.
     if (enclosingMethod != null && method.name != '') {
       MethodMember m = method; // lambdas must be MethodMembers
       _scope.create(m.name, m.functionType, m.definition);
     }
     _usedTemps = new Set();
     _freeTemps = [];
   }
 
+  Library get library() => method.library;
+
   // TODO(jimhug): Where does this really belong?
   MemberSet findMembers(String name) {
-    return method.library._findMembers(name);
+    return library._findMembers(name);
   }
 
   bool get isClosure() => (enclosingMethod != null);
 
   bool get isStatic() => method.isStatic;
 
   Value getTemp(Value value) {
     return value.needsTemp ? forceTemp(value) : value;
   }
 
   Value forceTemp(Value value) {
     String name;
     if (_freeTemps.length > 0) {
       name = _freeTemps.removeLast();
     } else {
       name = '\$' + _usedTemps.length;
     }
     _usedTemps.add(name);
-    return new Value(value.type, name, /*isSuper:*/false, /*needsTemp:*/false);
+    return new Value(value.type, name, value.span,
+      /*isSuper:*/false, /*needsTemp:*/false);
   }
 
   Value assignTemp(Value tmp, Value v) {
     if (tmp == v) {
       return v;
     } else {
       // TODO(jmesserly): we should mark this returned value with the temp
       // somehow, so getTemp will reuse it instead of allocating a new one.
-      return new Value(v.type, '(${tmp.code} = ${v.code})');
+      return new Value(v.type, '(${tmp.code} = ${v.code})', v.span);
     }
   }
 
   void freeTemp(Value value) {
     if (_usedTemps.remove(value.code)) {
       _freeTemps.add(value.code);
     } else {
       world.internalError(
         'tried to free unused value or non-temp "${value.code}"');
     }
@@ skipping 172 matching lines @@
       if (initializers != null && p.isInitializer) {
         var field = method.declaringType.getMember(p.name);
         if (field == null) {
           world.error('bad this parameter - no matching field',
             p.definition.span);
         }
         if (!field.isField) {
           world.error('"this.${p.name}" does not refer to a field',
             p.definition.span);
         }
-        var paramValue = new Value(field.returnType, p.name, false, false);
+        var paramValue = new Value(field.returnType, p.name,
+          p.definition.span, false, false);
         _paramCode.add(paramValue.code);
 
         initializers.add('this.${field.jsname} = ${paramValue.code};');
         initializedFields.add(p.name);
       } else {
         var paramValue = _scope.declareParameter(p);
         _paramCode.add(paramValue.code);
       }
     }
 
@@ skipping 427 matching lines @@
       writer.enterBlock('for (var ${tmpi.code} = 0;' +
           '${tmpi.code} < ${listVar.code}.length; ${tmpi.code}++) {');
       var value = listVar.invoke(this, '\$index', node.list,
           new Arguments(null, [tmpi]));
       writer.writeln('var ${item.code} = ${value.code};');
     } else {
       _pushBlock();
       // Needed to tell the runtime that we're doing this behind its back.
       var c = world.coreimpl.types['ListIterator'].getConstructor('');
       c.invoke(this, node, null,
-        new Arguments(null, [new Value(null, 'l')]));
+        new Arguments(null, [new Value(null, 'l', node.list.span)]));
 
       var iterator = list.invoke(this, 'iterator', node.list,
         Arguments.EMPTY);
       var tmpi = _scope.create('\$i', iterator.type, null);
 
       var hasNext = tmpi.invoke(this, 'hasNext', node.list, Arguments.EMPTY);
       var next = tmpi.invoke(this, 'next', node.list, Arguments.EMPTY);
 
       writer.enterBlock(
         'for (var ${tmpi.code} = ${iterator.code}; ${hasNext.code}; ) {');
       writer.writeln('var ${item.code} = ${next.code};');
     }
 
     visitStatementsInBlock(node.body);
     writer.exitBlock('}');
     _popBlock();
     return false;
   }
 
   void _genToDartException(String ex, Node node) {
     var types = const [
       'NullPointerException', 'ObjectNotClosureException',
       'NoSuchMethodException', 'StackOverflowException'];
-    var target = new Value(null, 'this');
+    var target = new Value(null, 'this', node.span);
     for (var name in types) {
       world.corelib.types[name].markUsed();
       world.corelib.types[name].members['toString'].invoke(
           this, node, target, Arguments.EMPTY);
     }
     writer.writeln('$ex = \$toDartException($ex);');
     world.gen.corejs.useToDartException = true;
   }
 
   bool visitTryStatement(TryStatement node) {
@@ skipping 197 matching lines @@
       world.warning('not allowed in static method', node.span);
     }
   }
 
   _makeSuperValue(Node node) {
     var parentType = method.declaringType.parent;
     _checkNonStatic(node);
     if (parentType == null) {
       world.error('no super class', node.span);
     }
-    return new Value(parentType, 'this',
+    return new Value(parentType, 'this', node.span,
       /*isSuper:*/true, /*needsTemp:*/false);
   }
 
   _getOutermostMethod() {
     var result = this;
     while (result.enclosingMethod != null) {
       result = result.enclosingMethod;
     }
     return result;
   }
 
+
+  // TODO(jimhug): Share code better with _makeThisValue.
+  String _makeThisCode() {
+    if (enclosingMethod != null) {
+      _getOutermostMethod().needsThis = true;
+      return '\$this';
+    } else {
+      return 'this';
+    }
+  }
+
   /**
    * Creates a reference to the enclosing type ('this') that can be used within
    * closures.
    */
-  _makeThisValue(Node node) {
+  Value _makeThisValue(Node node) {
     if (enclosingMethod != null) {
       var outermostMethod = _getOutermostMethod();
       outermostMethod._checkNonStatic(node);
       outermostMethod.needsThis = true;
       return new Value(outermostMethod.method.declaringType, '\$this',
-        /*isSuper:*/false, /*needsTemp:*/false);
+        node != null ? node.span : null, /*isSuper:*/false, /*needsTemp:*/false);
     } else {
       _checkNonStatic(node);
-      return new Value(method.declaringType, 'this',
+      return new Value(method.declaringType, 'this', node != null ? node.span : null,

[Jennifer Messerly, 2011/11/10 22:50:39]

this code is screaming for a ?. operator :)

Maybe something like a top level "spanOf(Node n) => n != null ? n.span : n;"
function if we're going to be doing it a lot?

         /*isSuper:*/false, /*needsTemp:*/false);
     }
   }
 
   // ******************* Expressions *******************
   visitLambdaExpression(LambdaExpression node) {
     var name = '';
     if (node.func.name != null) {
       name = world.toJsIdentifier(node.func.name.name);
     }
 
     var meth = _makeLambdaMethod(name, node.func);
 
     var w = new CodeWriter();
     meth.generator.writeDefinition(w, node);
 
-    return new Value(meth.functionType, w.text);
+    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;
       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;
-      var meth = method.declaringType.resolveMember(name);
-      if (meth != null) {
-        target = _makeThisOrType();
-        return meth.invoke(this, varExpr, target,
-            _makeArgs(node.arguments));
-      }
-      // Look for members of the top-level type (or imported libs).
-      meth = method.declaringType.library.lookup(name, varExpr.span);
-      if (meth != null) {
-        return meth.invoke(this, varExpr, null, _makeArgs(node.arguments));
+     // First check in block scopes.
+      target = _scope.lookup(name);
+      if (target != null) {
+        return target.invoke(this, '\$call', node, _makeArgs(node.arguments));
       }
 
-      name = '\$call';
-      target = varExpr.visit(this);
+      target = _makeThisOrType(varExpr.span);
+      return target.invoke(this, name, node, _makeArgs(node.arguments));
     } else {
       target = node.target.visit(this);
     }
 
     return target.invoke(this, name, position, _makeArgs(node.arguments));
   }
 
   visitIndexExpression(IndexExpression node) {
     var target = visitValue(node.target);
     var index = visitValue(node.index);
     return target.invoke(this, '\$index', node, new Arguments(null, [index]));
   }
 
   visitBinaryExpression(BinaryExpression node) {
     final kind = node.op.kind;
     // TODO(jimhug): Ensure these have same semantics as JS!
     if (kind == TokenKind.AND || kind == TokenKind.OR) {
       var x = visitValue(node.x);
       var y = visitValue(node.y);
       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 new EvaluatedValue(x.type, value, '$value', node.span);
       }
-      return new Value(null, code);
+      return new Value(null, code, node.span);
     } 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 value = kind == TokenKind.EQ_STRICT
             // 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.
             ? x.actualValue == y.actualValue : x.actualValue != y.actualValue;
         return new EvaluatedValue(world.boolType, value, "$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(null, '${x.code} $op ${y.code}');
+        return new Value(null, '${x.code} $op ${y.code}', node.span);
       } else {
         // TODO(jimhug): Resolve issue with undefined and null here.
-        return new Value(null, '${x.code} ${node.op} ${y.code}');
+        return new Value(null, '${x.code} ${node.op} ${y.code}', node.span);
       }
     }
 
     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';
@@ skipping 30 matching lines @@
       return _visitDotAssign(kind, xn, yn, position, captureOriginal);
     } else {
       world.error('illegal lhs', position.span);
     }
   }
 
   // TODO(jmesserly): it'd be nice if we didn't have to deal directly with
   // MemberSets here and in visitVarExpression.
   _visitVarAssign(int kind, VarExpression xn, Expression yn, Node position,
       Value captureOriginal(Value right)) {
+    final name = xn.name.name;
 
     // First check in block scopes.
-    var x = _scope.lookup(xn.name.name);
+    var x = _scope.lookup(name);
     var y = visitValue(yn);
 
     if (x == null) {
       // Look for a setter in the class
-      var members = method.declaringType.resolveMember(xn.name.name);
+      var members = method.declaringType.resolveMember(name);
       if (members != null) {
-        x = _makeThisOrType();
-      } else {
-        // Look for a top-level setter
-        final member = method.declaringType.library.lookup(
-            xn.name.name, xn.name.span);
-        if (member == null) {
-          world.warning('can not resolve ${xn.name.name}', xn.span);
-          return _makeMissingValue(xn.name.name);
-        }
-        members = new MemberSet(member);
-      }
-
-      // If we can't treat it as a field, generate a setter call.
-      // Also make sure we dont't try to set a method.
-      if (!members.treatAsField || members.containsMethods) {
-        if (kind != 0) {
-          var right = members.get_(this, position, x);
+        x = _makeThisOrType(position.span);
+        if (kind == 0) {
+          return x.set_(this, name, position, y);
+        } else if (!members.treatAsField || members.containsMethods) {
+          var right = x.get_(this, name, position);
+          //var right = members._get(this, position, x);

[Jennifer Messerly, 2011/11/10 22:50:39]

remove? Or make a TODO?

           right = captureOriginal(right);
           y = right.invoke(this, TokenKind.binaryMethodName(kind),
               position, new Arguments(null, [y]));
+          return x.set_(this, name, position, y);
+        } else {
+          x = x.get_(this, name, position);
         }
-        return members.set_(this, position, x, y);
+      } else {
+        // Look for a top-level setter
+        final member = library.lookup(name, xn.name.span);
+        if (member == null) {
+          world.warning('can not resolve ${name}', xn.span);
+          return _makeMissingValue(name);
+        }
+        members = new MemberSet(member);
+        // If we can't treat it as a field, generate a setter call.
+        // Also make sure we dont't try to set a method.

[Jennifer Messerly, 2011/11/10 22:50:39]

guessing we're on a path to removing this duplication?
Might be worth a TODO though. Unless this is just your next checkin ;)

+        if (!members.treatAsField || members.containsMethods) {
+          if (kind != 0) {
+            var right = members._get(this, position, x);
+            right = captureOriginal(right);
+            y = right.invoke(this, TokenKind.binaryMethodName(kind),
+                position, new Arguments(null, [y]));
+          }
+          return members._set(this, position, x, y);
+        } else {
+          x = members._get(this, position, x);
+        }
       }
 
       // Otherwise treat it as a field.
       // This makes for nicer code in the $op= case
-      x = members.get_(this, position, x);
     }
 
     // TODO(jimhug): Needs checks for final and other rules to enforce.
     y = y.convertTo(this, x.type, yn);
 
     if (kind == 0) {
       x = captureOriginal(x);
-      return new Value(y.type, '${x.code} = ${y.code}');
+      return new Value(y.type, '${x.code} = ${y.code}', position.span);
     } else if (x.type.isNum && y.type.isNum && (kind != TokenKind.TRUNCDIV)) {
       // Process everything but ~/ , which has no equivalent JS operator
       x = captureOriginal(x);
       // Very localized optimization for numbers!
       final op = TokenKind.kindToString(kind);
-      return new Value(y.type, '${x.code} $op= ${y.code}');
+      return new Value(y.type, '${x.code} $op= ${y.code}', position.span);
     } else {
       var right = x;
       right = captureOriginal(right);
       y = right.invoke(this, TokenKind.binaryMethodName(kind),
         position, new Arguments(null, [y]));
-      return new Value(y.type, '${x.code} = ${y.code}');
+      return new Value(y.type, '${x.code} = ${y.code}', position.span);
     }
   }
 
   _visitIndexAssign(int kind, IndexExpression xn, Expression yn, Node position,
       Value captureOriginal(Value right)) {
     var target = visitValue(xn.target);
     var index = visitValue(xn.index);
     var y = visitValue(yn);
 
     var tmptarget = target;
@@ skipping 35 matching lines @@
     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:
         if (value.type.isNum) {
-          return new Value(value.type, '${node.op}${value.code}');
+          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);
           var operand = new LiteralExpression(1,
             new TypeReference(node.span, world.numType), '1', node.span);
 
           return _visitAssign(kind, node.self, operand, node, null);
         }
       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);
         } else {
           var newVal = value.convertToNonNullBool(this, node);
-          return new Value(world.boolType, '!${newVal.code}');
+          return new Value(world.boolType, '!${newVal.code}', node.span);
         }
 
       case TokenKind.ADD:
         // TODO(jimhug): Issue #359 seeks to clarify this behavior.
         return value.convertTo(this, world.numType, node);
 
       case TokenKind.SUB:
       case TokenKind.BIT_NOT:
         if (node.op.kind == TokenKind.BIT_NOT) {
           return value.invoke(this, '\$bit_not', node, Arguments.EMPTY);
         } else if (node.op.kind == TokenKind.SUB) {
           return value.invoke(this, '\$negate', node, Arguments.EMPTY);
         } else {
           world.internalError('unimplemented: unary ${node.op}',
             node.span);
         }
       default:
         world.internalError('unimplemented: ${node.op}', node.span);
     }
   }
 
   visitPostfixExpression(PostfixExpression node, [bool isVoid = false]) {
     var value = visitValue(node.body);
     if (value.type.isNum) {
-      return new Value(value.type, '${value.code}${node.op}');
+      return new Value(value.type, '${value.code}${node.op}', node.span);
     }
 
     // x++ is equivalent to (t = x, x = t + 1, t), where we capture all temps
     // needed to evaluate x so we're not evaluating multiple times. Likewise,
     // x-- is equivalent to (t = x, x = t - 1, t).
     var kind = (TokenKind.INCR == node.op.kind) ? TokenKind.ADD : TokenKind.SUB;
     var operand = new LiteralExpression(1,
       new TypeReference(node.span, world.numType), '1', node.span);
 
     // Use _visitAssign to do most of the work, but save the right side in a
     // temporary variable if needed.
     // TODO(jmesserly): I don't like passing function args like this, but the
     // alternative is duplicating most of the _visitAssign logic. Needs cleanup.
     var tmpleft = null, left = null;
     var ret = _visitAssign(kind, node.body, operand, node, (l) {
       if (isVoid) {
         // No need for a temp if we're throwing away the result.
         return l;
       } else {
         // We always need a temp to capture the old value
         left = l;
         tmpleft = forceTemp(l);
         return assignTemp(tmpleft, left);
       }
     });
 
     if (tmpleft != null) {
-      ret = new Value(ret.type, "(${ret.code}, ${tmpleft.code})");
+      ret = new Value(ret.type, "(${ret.code}, ${tmpleft.code})", node.span);
     }
     if (tmpleft != left) {
       freeTemp(tmpleft);
     }
     return ret;
   }
 
   visitNewExpression(NewExpression node) {
     var typeRef = node.type;
 
@@ skipping 60 matching lines @@
           argsCode.add(arg.canonicalCode);
         }
       } else {
         argsCode.add(arg.code);
       }
     }
 
     world.coreimpl.types['ListFactory'].markUsed();
 
     final code = '[${Strings.join(argsCode, ", ")}]';
-    var value = new Value(world.listType, code);
+    var value = new Value(world.listType, code, node.span);
     if (node.isConst) {
       final immutableList = world.coreimpl.types['ImmutableList'];
       final immutableListCtor = immutableList.getConstructor('from');
       final result = immutableListCtor.invoke(
           this, node, null, new Arguments(null, [value]));
       value = world.gen.globalForConst(
           new ConstListValue(immutableList, argValues, 'const $code',
               result.code, node.span),
           argValues);
     }
@@ skipping 29 matching lines @@
       } else {
         argsCode.add(key.code);
         argsCode.add(value.code);
       }
     }
     var argList = '[${Strings.join(argsCode, ", ")}]';
     final code = '\$map($argList)';
     if (node.isConst) {
       final immutableMap = world.coreimpl.types['ImmutableMap'];
       final immutableMapCtor = immutableMap.getConstructor('');
-      final argsValue = new Value(world.listType, argList);
+      final argsValue = new Value(world.listType, argList, node.span);
       final result = immutableMapCtor.invoke(
           this, node, null, new Arguments(null, [argsValue]));
       final value = new ConstMapValue(
           immutableMap, argValues, code, result.code, node.span);
       return world.gen.globalForConst(value, argValues);
     }
-    return new Value(mapImplType, code);
+    return new Value(mapImplType, code, node.span);
   }
 
   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);
+    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);
     }
-    return new Value(body.type, '(${body.code})');
+    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) {
+    final name = node.name.name;
+
     // First check in block scopes.
-    var ret = _scope.lookup(node.name.name);
+    var ret = _scope.lookup(name);
     if (ret != null) return ret;
 
-    // Then check for members on my type - including supertypes.
-    ret = method.declaringType.resolveMember(node.name.name);
-    if (ret != null) {
-      return ret.get_(this, node, _makeThisOrType());
-    }
-
-    // Then look for members of the top-level type.
-    // This will also match types in the library and any libraries imported
-    // without a prefix.
-    ret = method.declaringType.library.lookup(node.name.name, node.span);
-    if (ret != null) {
-      return ret.get_(this, node, null);
-    }
-
-    world.warning('can not resolve ${node.name.name}', node.span);
-    return _makeMissingValue(node.name.name);
+    return _makeThisOrType(node.span).get_(this, name, node);

[Jennifer Messerly, 2011/11/10 22:50:39]

awesome cleanup!

   }
 
   _makeMissingValue(String name) {
-    // TODO(jimhug): Needs major revision to support doesNotUnderstand.
-    return new Value(null, '$name()/*NotFound*/');
+    // TODO(jimhug): Probably goes away to be fully replaced by doesNotUnder

[Jennifer Messerly, 2011/11/10 22:50:39]

noSuchMethod?

+    return new Value(null, '$name()/*NotFound*/', null);
   }
 
-  _makeThisOrType() {
-    var outermost = _getOutermostMethod();
-    if (outermost.method.isStatic) {
-      return _makeTypeValue(outermost.method.declaringType);
-    } else {
-      return _makeThisValue(null);
-    }
-  }
-
-  _makeTypeValue(Type type) {
-    // TODO(jimhug): Named args!
-    return new Value(type, type.jsname, false, false, true);
+  _makeThisOrType(SourceSpan span) {
+    return new BareValue(this, _getOutermostMethod(), span);
   }
 
   visitThisExpression(ThisExpression node) {
     return _makeThisValue(node);
   }
 
   visitSuperExpression(SuperExpression node) {
     return _makeSuperValue(node);
   }
 
@@ skipping 12 matching lines @@
         var val = visitValue(item);
         val.invoke(this, 'toString', item, Arguments.EMPTY);
 
         // TODO(jimhug): Ensure this solves all precedence problems.
         var code = val.code;
         if (item is BinaryExpression || item is ConditionalExpression) {
           code = '(${code})';
         }
         items.add(code);
       }
-      return new Value(type, '(${Strings.join(items, " + ")})');
+      return new Value(type, '(${Strings.join(items, " + ")})', node.span);
     }
 
     var text = node.text;
     // TODO(jimhug): Confirm that only strings need possible translation
     if (type.isString) {
       if (text.startsWith('@')) {
         text = _escapeString(parseStringLiteral(text));
         text = '"$text"';
       } else if (isMultilineString(text)) {
         // convert multi-line strings into single-line
         text = parseStringLiteral(text);
         // TODO(jimhug): What about \r?
         text = text.replaceAll('\n', '\\n');
         text = text.replaceAll('"', '\\"');
         text = '"$text"';
       }
       if (text !== node.text) {
         node.value = text;
         node.text = text;
       }
     }
 
+    // TODO(jimhug): Should pass node.span - but that breaks something...
     return new EvaluatedValue(type, node.value, node.text, null);
   }
 }
 
 
 // TODO(jmesserly): move this into its own file?
 class Arguments {
   static Arguments _empty;
   static Arguments get EMPTY() {
     if (_empty == null) {
       _empty = new Arguments(null, []);
     }
     return _empty;
   }
 
   List<Value> values;
   List<ArgumentNode> nodes;
   int _bareCount;
 
   Arguments(this.nodes, this.values);
 
   /** Constructs a bare list of arguments. */
   factory Arguments.bare(int arity) {
     var values = [];
     for (int i = 0; i < arity; i++) {
-      values.add(new Value(world.varType, '\$$i', false, /*needsTemp:*/false));
+      // TODO(jimhug): Need source locations.
+      values.add(new Value(world.varType, '\$$i', null, false, /*needsTemp:*/false));
     }
     return new Arguments(null, values);
   }
 
   int get nameCount() => length - bareCount;
   bool get hasNames() => bareCount < length;
 
   int get length() => values.length;
 
   String getName(int i) => nodes[i].label.name;
@@ skipping 67 matching lines @@
     for (int i = bareCount; i < length; i++) {
       names.add(getName(i));
     }
     return names;
   }
 
   /** Gets the argument names used in a call stub; uses $0 $1 for bare args. */
   Arguments toCallStubArgs() {
     var result = [];
     for (int i = 0; i < bareCount; i++) {
-      result.add(new Value(world.varType, '\$$i', false, /*needsTemp:*/false));
+      // TODO(jimhug): Need source locations.

[Jennifer Messerly, 2011/11/10 22:50:39]

synthetic stubs shouldn't need them? The idea is these args are only used to
generate code that we know won't have any errors (due to var type, etc).

+      result.add(new Value(world.varType, '\$$i', null, false, /*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, false, /*needsTemp:*/false));
+      // TODO(jimhug): Need source locations.
+      result.add(new Value(world.varType, name, null, false, /*needsTemp:*/false));
     }
     return new Arguments(nodes, result);
   }
 }