Debugging in kernel shaping up.

runtime/vm/object.cc

 // Copyright (c) 2012, 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.
 
 #include "vm/object.h"
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "vm/assembler.h"
 #include "vm/become.h"
@@ skipping 8808 matching lines @@
   }
   return token_stream.GenerateSource();
 }
 
 
 void Script::set_compile_time_constants(const Array& value) const {
   StorePointer(&raw_ptr()->compile_time_constants_, value.raw());
 }
 
 
 RawGrowableObjectArray* Script::GenerateLineNumberArray() const {

[Kevin Millikin (Google), 2017/01/24 13:44:13]

This function really needs a comment somewhere documenting the format of this
array, which is otherwise unstructured.

It looks like each line is described by a one-based line number and then a
sequence of (debug position, one-based column number), and that each line is
null-prefixed, and there is at least one line.  (I wonder why the lines are
null-prefixed and not null-terminated.)

[jensj, 2017/01/25 12:52:22]

In runtime/include/dart_tools_api.h Dart_ScriptGetTokenInfo is described as

/**
 * Returns an array containing line number and token offset info
 * for the given script.
 *
 * Returns an array of numbers. Null values indicate the beginning of
 * a new line. The first number after null is the line number.
 * The line number is followed by pairs of numbers, with the first value
 * being the "token offset" and the second value being the column number
 * of the token.
 * The "token offset" is a value that is used to indicate a location
 * in code, similarly to a "PC" address.
 * Source lines with no tokens are omitted.
 *
 * Requires there to be a current isolate.
 *
 * \return A handle to an array or an error object.
 */

   Zone* zone = Thread::Current()->zone();
   const GrowableObjectArray& info =
       GrowableObjectArray::Handle(zone, GrowableObjectArray::New());
   const String& source = String::Handle(zone, Source());
   const String& key = Symbols::Empty();
   const Object& line_separator = Object::Handle(zone);
   Smi& value = Smi::Handle(zone);
 
   if (kind() == RawScript::kKernelTag) {
     const Array& line_starts_array = Array::Handle(line_starts());
     if (line_starts_array.IsNull()) {
       // Scripts in the AOT snapshot do not have a line starts array.
       // A well-formed line number array has a leading null.
       info.Add(line_separator);  // New line.
       return info.raw();
     }
     intptr_t line_count = line_starts_array.Length();
     ASSERT(line_count > 0);
+    const Array& tokens_seen_array = Array::Handle(tokens_seen());
+    intptr_t token_count = tokens_seen_array.Length();
+    int token_idx = 0;

[Kevin Millikin (Google), 2017/01/24 13:44:13]

You can also spell out index.

[jensj, 2017/01/25 12:52:22]

Done.

+    int last_line = -1;
 
     for (int i = 0; i < line_count; i++) {

[Kevin Millikin (Google), 2017/01/24 13:44:13]

Instead of i, it's clearer to use the name 'line_index' to keep the indexes
obviously parallel and distinct.

[jensj, 2017/01/25 12:52:22]

Done.

-      info.Add(line_separator);  // New line.
-      value = Smi::New(i + 1);
-      info.Add(value);  // Line number.
       value ^= line_starts_array.At(i);
-      info.Add(value);  // Token position.
-      value = Smi::New(1);
-      info.Add(value);  // Column.
+      intptr_t start = value.Value();
+      // Output the rest of the tokens if we have no next line.
+      intptr_t end = LONG_MAX;

[Kevin Millikin (Google), 2017/01/24 13:44:13]

INTPTR_MAX

[jensj, 2017/01/25 12:52:22]

Done.

+      if (i + 1 < line_count) {
+        value ^= line_starts_array.At(i + 1);
+        end = value.Value();
+      }
+      while (token_idx < token_count) {
+        value ^= tokens_seen_array.At(token_idx);
+        intptr_t seen = value.Value();

[Kevin Millikin (Google), 2017/01/24 13:44:13]

seen ==> debug_position ?

[jensj, 2017/01/25 12:52:22]

Done.

+        if (seen == -1) {

[Kevin Millikin (Google), 2017/01/24 13:44:13]

Can we just eliminate the -1 entries, or do we need them?

[jensj, 2017/01/25 12:52:22]

I'll move the check to the recording of the entries instead.

+          ++token_idx;
+          continue;
+        }
+        if (seen >= end) break;
+
+        if (i != last_line) {

[Kevin Millikin (Google), 2017/01/24 13:44:13]

last_line is just used to detect the first debug position on the line that is
not -1, right?  We don't actually use the value for anything.  I find it clearer
to have something like:

bool first = true;
while (token_index < token_count) {
  ...
  if (first) {
    ...
    first = false;
  }
  ...
}

[jensj, 2017/01/25 12:52:22]

It was done to look like the non-kernel version. I'll rewrite.

+          info.Add(line_separator);  // New line.
+          value = Smi::New(i + 1);   // Line number.
+          info.Add(value);
+          last_line = i;
+        }
+
+        value ^= tokens_seen_array.At(token_idx);
+        info.Add(value);                     // Token position.
+        value = Smi::New(seen - start + 1);  // Column.
+        info.Add(value);
+        ++token_idx;
+      }
     }
     return info.raw();
   }
 
   const TokenStream& tkns = TokenStream::Handle(zone, tokens());
   String& tokenValue = String::Handle(zone);
   ASSERT(!tkns.IsNull());
   TokenStream::Iterator tkit(zone, tkns, TokenPosition::kMinSource,
                              TokenStream::Iterator::kAllTokens);
   int current_line = -1;
@@ skipping 100 matching lines @@
 
 void Script::set_source(const String& value) const {
   StorePointer(&raw_ptr()->source_, value.raw());
 }
 
 void Script::set_line_starts(const Array& value) const {
   StorePointer(&raw_ptr()->line_starts_, value.raw());
 }
 
 
+void Script::set_tokens_seen(const Array& value) const {
+  StorePointer(&raw_ptr()->tokens_seen_, value.raw());
+}
+
+
+void Script::set_yields_seen(const Array& value) const {
+  StorePointer(&raw_ptr()->yields_seen_, value.raw());
+}
+
+
 void Script::set_kind(RawScript::Kind value) const {
   StoreNonPointer(&raw_ptr()->kind_, value);
 }
 
 
 void Script::set_load_timestamp(int64_t value) const {
   StoreNonPointer(&raw_ptr()->load_timestamp_, value);
 }
 
 
@@ skipping 70 matching lines @@
       } else {
         min = midpoint;
       }
     }
     *line = min + 1;
     smi ^= line_starts_array.At(min);
     if (column != NULL) {
       *column = offset - smi.Value() + 1;
     }
     if (token_len != NULL) {
+      // We don't explicitly save this data.
+      // TODO(jensj): Load the source and attempt to find it from there.
       *token_len = 1;
     }
     return;
   }
 
   const TokenStream& tkns = TokenStream::Handle(zone, tokens());
   if (tkns.IsNull()) {
     ASSERT((Dart::snapshot_kind() == Snapshot::kAppAOT));
     *line = -1;
     if (column != NULL) {
@@ skipping 13922 matching lines @@
   return UserTag::null();
 }
 
 
 const char* UserTag::ToCString() const {
   const String& tag_label = String::Handle(label());
   return tag_label.ToCString();
 }
 
 }  // namespace dart