Kernel debugging; service tests

pkg/kernel/lib/analyzer/ast_from_analyzer.dart

 // Copyright (c) 2016, 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.
 library kernel.analyzer.ast_from_analyzer;
 
 import '../ast.dart' as ast;
 import '../frontend/accessors.dart';
 import '../frontend/super_initializers.dart';
 import '../log.dart';
 import '../type_algebra.dart';
@@ skipping 506 matching lines @@
   }
 
   ast.Statement buildMandatoryFunctionBody(FunctionBody body) {
     try {
       if (body is BlockFunctionBody) {
         return buildStatement(body.block);
       } else if (body is ExpressionFunctionBody) {
         if (bodyHasVoidReturn(body)) {
           return new ast.ExpressionStatement(buildExpression(body.expression));
         } else {
-          return new ast.ReturnStatement(buildExpression(body.expression));
+          return new ast.ReturnStatement(buildExpression(body.expression))
+            ..fileOffset = body.expression.offset;
         }
       } else {
         return internalError('Missing function body');
       }
     } on _CompilationError catch (e) {
       return new ast.ExpressionStatement(buildThrowCompileTimeError(e.message));
     }
   }
 
   ast.AsyncMarker getAsyncMarker({bool isAsync: false, bool isStar: false}) {
@@ skipping 119 matching lines @@
       return buildType(element.type);
     } else if (element is VariableElement) {
       return buildType(element.type);
     } else {
       log.severe('Unexpected variable element: $element');
       return const ast.DynamicType();
     }
   }
 
   ast.VariableDeclaration makeVariableDeclaration(LocalElement element,
-      {ast.DartType type, ast.Expression initializer}) {
+      {ast.DartType type, ast.Expression initializer, int equalsOffset}) {
     var declaration = getVariableReference(element);
+    if (equalsOffset != null) declaration.fileEqualsOffset = equalsOffset;
     declaration.type = type ?? getInferredVariableType(element);
     if (initializer != null) {
       declaration.initializer = initializer..parent = declaration;
     }
     return declaration;
   }
 
   /// Returns true if [arguments] can be accepted by [target]
   /// (not taking type checks into account).
   bool areArgumentsCompatible(
@@ skipping 275 matching lines @@
       LabeledStatement labeled = node;
       node = labeled.statement;
     }
     if (node is VariableDeclarationStatement) {
       VariableDeclarationList list = node.variables;
       ast.DartType type = scope.buildOptionalTypeAnnotation(list.type);
       for (VariableDeclaration decl in list.variables) {
         LocalElement local = decl.element as dynamic; // Cross cast.
         output.add(scope.makeVariableDeclaration(local,
             type: type,
-            initializer: scope.buildOptionalExpression(decl.initializer)));
+            initializer: scope.buildOptionalExpression(decl.initializer),
+            equalsOffset: decl.equals?.offset));
       }
     } else {
       output.add(build(node));
     }
   }
 
   ast.Statement makeBreakTarget(ast.Statement node, LabelStack stackNode) {
     if (stackNode.jumps.isEmpty) return node;
     var labeled = new ast.LabeledStatement(node);
     for (var jump in stackNode.jumps) {
@@ skipping 193 matching lines @@
   ast.Statement visitForStatement(ForStatement node) {
     List<ast.VariableDeclaration> variables = <ast.VariableDeclaration>[];
     ast.Expression initialExpression;
     if (node.variables != null) {
       VariableDeclarationList list = node.variables;
       var type = scope.buildOptionalTypeAnnotation(list.type);
       for (var variable in list.variables) {
         LocalElement local = variable.element as dynamic; // Cross cast.
         variables.add(scope.makeVariableDeclaration(local,
             initializer: scope.buildOptionalExpression(variable.initializer),
-            type: type));
+            type: type,
+            equalsOffset: variable.equals?.offset));
       }
     } else if (node.initialization != null) {
       initialExpression = scope.buildExpression(node.initialization);
     }
     var breakNode = new LabelStack.unlabeled(breakStack);
     var continueNode = new LabelStack.unlabeled(continueStack);
     addLoopLabels(node, continueNode);
     var body = buildInScope(node.body, breakNode, continueNode);
     var loop = new ast.ForStatement(
         variables,
@@ skipping 168 matching lines @@
     extends GeneralizingAstVisitor /* <ast.Expression | Accessor> */ {
   final ExpressionScope scope;
   ast.VariableDeclaration cascadeReceiver;
   ExpressionBuilder(this.scope);
 
   ast.Expression build(Expression node) {
     var result = node.accept(this);
     if (result is Accessor) {
       result = result.buildSimpleRead();
     }
-    return result..fileOffset = _getOffset(node);
+    return result..fileOffset = _getOffset(node, result);
   }
 
-  int _getOffset(AstNode node) {
+  int _getOffset(AstNode node, ast.Expression expression) {

[Kevin Millikin (Google), 2017/02/08 15:37:51]

This is a warty API.  It takes an analyzer AstNode (isn't it always an
Expression?) and a Kernel Expression.  It just ignores the second argument
except in case the first argument is a MethodInvocation and the second argument
has a non-negative file offset.

As far as I can tell, the left-hand side of an assignment cannot be a method
invocation, so we definitely don't need to pass the second argument to the
recursive call.  And for the non-recursive call (am I missing other call sites?)
we could avoid passing the second argument with:

// For some method invocations we have already set a file offset to
// override the default behavior of _getOffset.
if (node is! MethodInvocation || result.fileOffset < 0) {
  result.fileOffset = _getOffset(node);
}
return result;

[jensj, 2017/02/13 14:04:15]

Done.

     if (node is MethodInvocation) {
+      if (expression.fileOffset >= 0) return expression.fileOffset;
       return node.methodName.offset;
     } else if (node is InstanceCreationExpression) {
       return node.constructorName.offset;
     } else if (node is BinaryExpression) {
       return node.operator.offset;
     } else if (node is PrefixedIdentifier) {
       return node.identifier.offset;
     } else if (node is AssignmentExpression) {
-      return _getOffset(node.leftHandSide);
+      return _getOffset(node.leftHandSide, expression);
     } else if (node is PropertyAccess) {
       return node.propertyName.offset;
     } else if (node is IsExpression) {
       return node.isOperator.offset;
+    } else if (node is AsExpression) {
+      return node.asOperator.offset;
     } else if (node is StringLiteral) {
       // Use a catch-all for StringInterpolation and AdjacentStrings:
       // the debugger stops at the end.
       return node.end;
+    } else if (node is IndexExpression) {
+      return node.leftBracket.offset;
     }
     return node.offset;
   }
 
   Accessor buildLeftHandValue(Expression node) {
     var result = node.accept(this);
     if (result is Accessor) {
       return result;
     } else {
       return new ReadOnlyAccessor(result);
@@ skipping 461 matching lines @@
       return new ast.ConstructorInvocation(constructor, arguments,
           isConst: node.isConst);
     } else {
       return noSuchMethodError();
     }
   }
 
   ast.Expression visitIsExpression(IsExpression node) {
     if (node.notOperator != null) {
       return new ast.Not(new ast.IsExpression(
-          build(node.expression), scope.buildTypeAnnotation(node.type)));
+          build(node.expression), scope.buildTypeAnnotation(node.type))
+        ..fileOffset = node.isOperator.offset);

[Kevin Millikin (Google), 2017/02/08 15:37:51]

It seems weird that the IsExpression that comes from 'is!' has a file offset,
but the IsExpression that comes from 'is' does not.  I can't figure out why, so
it probably needs a comment.

It also seems better to call _getOffset(node) instead of writing
node.isOperator.offset explicitly.  The only documentation we currently have of
which offsets we record for which kinds of expressions is by reading _getOffset.
 It would be easy to miss the case for is!.

[jensj, 2017/02/13 14:04:15]

Done.

     } else {
       return new ast.IsExpression(
           build(node.expression), scope.buildTypeAnnotation(node.type));
     }
   }
 
   /// Emit a method invocation, either as a direct call `o.f(x)` or decomposed
   /// into a getter and function invocation `o.f.call(x)`.
   ast.Expression buildDecomposableMethodInvocation(ast.Expression receiver,
       ast.Name name, ast.Arguments arguments, Element targetElement) {
@@ skipping 32 matching lines @@
       scope.addTransformerFlag(TransformerFlag.superCalls);
       return new ast.SuperMethodInvocation(
           scope.buildName(node.methodName),
           buildArgumentsForInvocation(node),
           scope.resolveConcreteMethod(element));
     } else if (isLocal(element)) {
       return new ast.MethodInvocation(
           new ast.VariableGet(scope.getVariableReference(element)),
           callName,
           buildArgumentsForInvocation(node),
-          scope.resolveInterfaceFunctionCall(element));
+          scope.resolveInterfaceFunctionCall(element))
+        ..fileOffset = node.methodName.end;

[Kevin Millikin (Google), 2017/02/08 15:37:51]

This definitely needs a comment.  We are explicitly setting the file offset
here, for the case of invoking a method through a local, to override the default
behavior of the call in 'build' that would use the methodName's (start)
position, right?

Add a comment to that effect here.

[jensj, 2017/02/13 14:04:15]

Done.

     } else if (isStaticMethod(element)) {
       var method = scope.resolveConcreteMethod(element);
       var arguments = buildArgumentsForInvocation(node);
       if (method == null || !scope.areArgumentsCompatible(element, arguments)) {
         return scope.buildThrowNoSuchMethodError(
             new ast.NullLiteral(), node.methodName.name, arguments,
             candidateTarget: element);
       }
       return new ast.StaticInvocation(method, arguments);
     } else if (isStaticVariableOrGetter(element)) {
@@ skipping 59 matching lines @@
   }
 
   ast.Expression visitPostfixExpression(PostfixExpression node) {
     String operator = node.operator.value();
     switch (operator) {
       case '++':
       case '--':
         var leftHand = buildLeftHandValue(node.operand);
         var binaryOperator = new ast.Name(operator[0]);
         return leftHand.buildPostfixIncrement(binaryOperator,
-            offset: node.offset,
+            offset: node.operator.offset,
             voidContext: isInVoidContext(node),
             interfaceTarget: scope.resolveInterfaceMethod(node.staticElement));
 
       default:
         return scope.internalError('Invalid postfix operator $operator');
     }
   }
 
   ast.Expression visitPrefixExpression(PrefixExpression node) {
     String operator = node.operator.value();
@@ skipping 1025 matching lines @@
     if (list[i - 1].compareTo(item) == 0) {
       ++deleted;
     } else if (deleted > 0) {
       list[i - deleted] = item;
     }
   }
   if (deleted > 0) {
     list.length -= deleted;
   }
 }