cleanup to Value - fix for StringEscapesTest

frog/member.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.
 
 /** A formal parameter to a [Method]. */
 class Parameter {
   FormalNode definition;
   Member method;
 
   String name;
@@ skipping 10 matching lines @@
       name = name.substring(5);
       isInitializer = true;
     }
 
     type = method.resolveType(definition.type, false);
 
     if (definition.value != null) {
       // To match VM, detect cases where value was not actually specified in
       // code and don't signal errors.
       // TODO(jimhug): Clean up after issue #352 is resolved.
-      if (!hasDefaultValue) return;
+      if (definition.value.span.start == definition.span.start) {

[Jennifer Messerly, 2012/01/09 20:16:26]

I think I originally suggested the property to make it a bit more clear why
we're checking a span. Maybe we could update the comment?

IIRC this check is for the "someMethod([namedArgWithImplicitNullDefault])" case.
But checking the span is rather subtle.

[jimhug, 2012/01/09 21:19:05]

I believe that Bob is going to undo this change, so I'll leave this one
untouched.

+        return;
+      }
 
       if (method.name == ':call') {
         // TODO(jimhug): Need simpler way to detect "true" function types vs.
         //   regular methods being used as function types for closures.
         // TODO(sigmund): Disallow non-null default values for native calls?
         if (method.definition.body == null && !method.isNative) {
           world.error('default value not allowed on function type',
               definition.span);
         }
       } else if (method.isAbstract) {
@@ skipping 21 matching lines @@
 
   Parameter copyWithNewType(Member newMethod, Type newType) {
     var ret = new Parameter(definition, newMethod);
     ret.type = newType;
     ret.name = name;
     ret.isInitializer = isInitializer;
     return ret;
   }
 
   bool get isOptional() => definition != null && definition.value != null;
-
-  /**
-   * Gets whether this named parameter has an explicit default value or relies
-   * on the implicit `null`.
-   */
-  bool get hasDefaultValue() => definition.value is! NullExpression ||
-      (definition.value.span.start != definition.span.start);
 }
 
 
 class Member extends Element {
   final Type declaringType;
 
   bool isGenerated;
   MethodGenerator generator;
 
   Member(String name, Type declaringType)
@@ skipping 1162 matching lines @@
         return new EvaluatedValue(inferredResult, value, "$value", node.span);
       }
     } else if (declaringType.isString) {
       if (name == ':index') {
         // Note: this could technically propagate constness, but that's not
         // specified explicitly and the VM doesn't do that.
         return new Value(declaringType, '${target.code}[${argsCode[0]}]',
           node.span);
       } else if (name == ':add') {
         if (allConst) {
-          final value = _normConcat(target, args.values[0]);
-          return new EvaluatedValue(world.stringType, value, value, node.span);
+          final value = target.dynamic.actualValue +
+            args.values[0].dynamic.actualValue;
+          return Value.fromString(value, node.span);
         }
 
-        // Ensure we generate toString on the right side
         return new Value(declaringType, '${target.code} + ${argsCode[0]}',
           node.span);
       }
     } else if (declaringType.isNative) {
       if (name == ':index') {
         // Note: this could technically propagate constness, but that's not
         // specified explicitly and the VM doesn't do that.
         return
             new Value(returnType, '${target.code}[${argsCode[0]}]', node.span);
       } else if (name == ':setindex') {
@@ skipping 42 matching lines @@
     } else if (name == ':setindex') {
       world.gen.corejs.useSetIndex = true;
     }
 
     // Fall back to normal method invocation.
     var argsString = Strings.join(argsCode, ', ');
     return new Value(inferredResult, '${target.code}.$jsname($argsString)',
         node.span);
   }
 
-  /**
-   * Return the string concatenation of two values, which is normalized to use
-   * double-quotes if any of the input strings used double-quotes.
-   */
-  String _normConcat(Value a, Value b) {

[Jennifer Messerly, 2012/01/09 20:16:26]

yay

-    assert(b.type.isString);
-    var val0 = a.dynamic.actualValue;
-    var quote0 = val0[0];
-    val0 = val0.substring(1, val0.length - 1);
-    var val1 = b.dynamic.actualValue;
-    var quote1 = null;
-    if (b.type.isString) {
-      quote1 = val1[0];
-      val1 = val1.substring(1, val1.length - 1);
-    }
-    var value;
-    if (quote0 == quote1 || quote1 == null) {
-      // If both strings use the same quote, then keep using it.
-      value = '$quote0${val0}${val1}$quote0';
-    } else if (quote0 == '"') {
-      // If they are different, escape the single-quote to be double-quote
-      // the choice of single vs double is arbitrary, but choosing one
-      // ensures that we only do this escaping once on a string portion.
-      assert(quote1 == "'");
-      value = '$quote0${val0}${toDoubleQuote(val1)}$quote0';
-    } else {
-      assert(quote1 == '"');
-      value = '$quote1${toDoubleQuote(val0)}${val1}$quote1';
-    }
-    return value;
-  }
-
   resolve() {
     // TODO(jimhug): cut-and-paste-and-edit from Field.resolve
     isStatic = declaringType.isTop;
     isConst = false;
     isFactory = false;
     isAbstract = !declaringType.isClass;
     if (definition.modifiers != null) {
       for (var mod in definition.modifiers) {
         if (mod.kind == TokenKind.STATIC) {
           if (isStatic) {
@@ skipping 406 matching lines @@
   }
 
   void forEach(void f(Member member)) {
     factories.forEach((_, Map constructors) {
       constructors.forEach((_, Member member) {
         f(member);
       });
     });
   }
 }