[VM] Move runtime/lib/{developer,profiler,timeline}.dart -> ..._patch.dart, fix owner of constructors

This makes sure we can distinguish e.g. 2 different timeline.dart files
(from runtime/lib/timeline.dart and sdk/lib/developer/timeline.dart)

This CL also fixes the owner of constructors to be patch classes, if
the constructors come from a patch.

This CL also adds a service/valid_source_locations_test, which loops
over libraries/classes/fields/functions and obtains source locations for
them, including line numbers.  This ensures that if there is a source
location attached to a member, we can use it's token position to get to
the line number.

This CL also changes package:kernel's [Cloner] to clone fileOffsets (and
not just fileEndOffsets). This is important for mixin resolution, where
we copy members into mixin application classes.

Issue https://github.com/dart-lang/sdk/issues/32489

Change-Id: I4fea5cd646d81f47e1c4ede1e86d477ba6de3e82
Reviewed-on: https://dart-review.googlesource.com/46141
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>

pkg/kernel/lib/clone.dart

@@ -391,6 +391,7 @@ class CloneVisitor implements TreeVisitor {
         isSynthetic: node.isSynthetic,
         initializers: node.initializers.map(clone).toList(),
         transformerFlags: node.transformerFlags)
+      ..fileOffset = node.fileOffset
       ..fileEndOffset = node.fileEndOffset;
   }
 
@@ -406,6 +407,7 @@ class CloneVisitor implements TreeVisitor {
         fileUri: node.fileUri,
         forwardingStubSuperTarget: node.forwardingStubSuperTarget,
         forwardingStubInterfaceTarget: node.forwardingStubInterfaceTarget)
+      ..fileOffset = node.fileOffset
       ..fileEndOffset = node.fileEndOffset
       ..isGenericContravariant = node.isGenericContravariant;
   }
@@ -422,6 +424,7 @@ class CloneVisitor implements TreeVisitor {
         hasImplicitSetter: node.hasImplicitSetter,
         transformerFlags: node.transformerFlags,
         fileUri: node.fileUri)
+      ..fileOffset = node.fileOffset
       ..fileEndOffset = node.fileEndOffset
       ..flags = node.flags
       ..flags2 = node.flags2;
@@ -461,6 +464,7 @@ class CloneVisitor implements TreeVisitor {
         returnType: visitType(node.returnType),
         asyncMarker: node.asyncMarker,
         dartAsyncMarker: node.dartAsyncMarker)
+      ..fileOffset = node.fileOffset
       ..fileEndOffset = node.fileEndOffset;
   }
 

runtime/lib/developer_patch.dart

@@ -14,8 +14,8 @@ import "dart:async" show Future, Zone;
 import "dart:isolate" show SendPort;
 
 /// These are the additional parts of this patch library:
-// part "profiler.dart"
-// part "timeline.dart"
+// part "profiler_patch.dart"
+// part "timeline_patch.dart"
 
 @patch
 bool debugger({bool when: true, String message}) native "Developer_debugger";

runtime/lib/developer_sources.gni

@@ -6,11 +6,11 @@
 developer_runtime_sources = [
   "developer.cc",
 
-  # developer.dart needs to be the first dart file because it contains
+  # developer_patch.dart needs to be the first dart file because it contains
   # imports.
-  "developer.dart",
+  "developer_patch.dart",
   "profiler.cc",
-  "profiler.dart",
+  "profiler_patch.dart",
   "timeline.cc",
-  "timeline.dart",
+  "timeline_patch.dart",
 ]

runtime/lib/profiler_patch.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// part of "developer.dart";
+// part of "developer_patch.dart";
 
 @patch
 class UserTag {

runtime/lib/timeline_patch.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// part of "developer.dart";
+// part of "developer_patch.dart";
 
 @patch
 bool _isDartStreamEnabled() native "Timeline_isDartStreamEnabled";

runtime/observatory/tests/service/service.status

@@ -48,6 +48,7 @@ add_breakpoint_rpc_kernel_test: SkipByDesign # kernel specific version of add_br
 [ $mode == debug ]
 debugger_location_second_test: Pass, Slow
 debugger_location_test: Pass, Slow
+valid_source_locations_test: Pass, Slow
 
 # Service protocol is not supported in product mode.
 [ $mode == product ]

runtime/observatory/tests/service/valid_source_locations_test.dart

@@ -0,0 +1,79 @@
+// Copyright (c) 2018, 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.
+// VMOptions=--error_on_bad_type --error_on_bad_override
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:developer';
+
+import 'package:observatory/service_io.dart';
+import 'package:unittest/unittest.dart';
+
+import 'service_test_common.dart';
+import 'test_helper.dart';
+
+void testFunction() {
+  debugger();
+}
+
+Future validateLocation(Location location) async {
+  if (location == null) return;
+  if (location.tokenPos == -1) return;
+
+  // Ensure the script is loaded.
+  final Script script = await location.script.load();
+
+  // Use the more low-level functions.
+  script.getLine(script.tokenToLine(location.tokenPos));
+  script.tokenToCol(location.tokenPos);
+
+  // Use the helper functions.
+  await location.getLine();
+  await location.getColumn();
+}
+
+Future validateFieldLocation(Field field) async {
+  // TODO(http://dartbug.com/32503): We should `field = await field.load()`
+  // here, but it causes all kinds of strong-mode errors.
+  await validateLocation(field.location);
+}
+
+Future validateFunctionLocation(ServiceFunction fun) async {
+  fun = await fun.load();
+  await validateLocation(fun.location);
+}
+
+Future validateClassLocation(Class klass) async {
+  klass = await klass.load();
+  await validateLocation(klass.location);
+
+  for (Field field in klass.fields) {
+    await validateFieldLocation(field);
+  }
+  for (ServiceFunction fun in klass.functions) {
+    await validateFunctionLocation(fun);
+  }
+}
+
+var tests = <IsolateTest>[
+  hasStoppedAtBreakpoint,
+  (Isolate isolate) async {
+    // Loop over all libraries, classes, functions and fields to ensure .
+    for (Library lib in isolate.libraries) {
+      lib = await lib.load();
+
+      for (Field field in lib.variables) {
+        await validateFieldLocation(field);
+      }
+      for (ServiceFunction fun in lib.functions) {
+        await validateFunctionLocation(fun);
+      }
+      for (Class klass in lib.classes) {
+        await validateClassLocation(klass);
+      }
+    }
+  },
+];
+
+main(args) => runIsolateTests(args, tests, testeeConcurrent: testFunction);

runtime/vm/kernel_loader.cc

@@ -1153,6 +1153,16 @@ void KernelLoader::FinishClassLoading(const Class& klass,
 
     const String& name =
         H.DartConstructorName(constructor_helper.canonical_name_);
+
+    // We can have synthetic constructors, which will not have a source uri
+    // attached to them (which means the index into the source uri table is 0,
+    // see `package:kernel/binary/ast_to_binary::writeUriReference`.
+    const Object* owner = &klass;
+    const intptr_t source_uri_index = constructor_helper.source_uri_index_;
+    if (source_uri_index != 0) {
+      owner = &ClassForScriptAt(klass, source_uri_index);
+    }
+
     Function& function = Function::ZoneHandle(
         Z, Function::New(name, RawFunction::kConstructor,
                          false,  // is_static
@@ -1160,7 +1170,7 @@ void KernelLoader::FinishClassLoading(const Class& klass,
                          false,  // is_abstract
                          constructor_helper.IsExternal(),
                          false,  // is_native
-                         klass, constructor_helper.position_));
+                         *owner, constructor_helper.position_));
     function.set_end_token_pos(constructor_helper.end_position_);
     functions_.Add(&function);
     function.set_kernel_offset(constructor_offset);

sdk/lib/libraries.json

@@ -73,9 +73,9 @@
       },
       "developer": {
         "patches": [
-          "../../runtime/lib/developer.dart",
-          "../../runtime/lib/profiler.dart",
-          "../../runtime/lib/timeline.dart"
+          "../../runtime/lib/developer_patch.dart",
+          "../../runtime/lib/profiler_patch.dart",
+          "../../runtime/lib/timeline_patch.dart"
         ],
         "uri": "developer/developer.dart"
       },

sdk/lib/libraries.yaml

@@ -81,9 +81,9 @@ vm:
     developer:
       uri: "developer/developer.dart"
       patches:
-        - "../../runtime/lib/developer.dart"
-        - "../../runtime/lib/profiler.dart"
-        - "../../runtime/lib/timeline.dart"
+        - "../../runtime/lib/developer_patch.dart"
+        - "../../runtime/lib/profiler_patch.dart"
+        - "../../runtime/lib/timeline_patch.dart"
 
     _http:
       uri: "_http/http.dart"