Improve code coverage generation.

runtime/vm/coverage.cc

Index: runtime/vm/coverage.cc
diff --git a/runtime/vm/coverage.cc b/runtime/vm/coverage.cc
index 5d284d21083f2c2d8da0d3e01b65a14ba390a0a7..be80a9c378644dd01ed82877324687485f5f97c8 100644
--- a/runtime/vm/coverage.cc
+++ b/runtime/vm/coverage.cc
@@ -16,28 +16,34 @@ namespace dart {
 DEFINE_FLAG(charp, coverage_dir, NULL,
             "Enable writing coverage data into specified directory.");
 
-void CodeCoverage::PrintClass(const Class& cls, const JSONArray& jsarr) {
-  const Array& functions = Array::Handle(cls.functions());
-  ASSERT(!functions.IsNull());
-  Function& function = Function::Handle();
+
+int CodeCoverage::PrintFunctionsInSource(const String& source,
+                                         const Array& functions,
+                                         int index,
+                                         const JSONArray& hits_arr) {
+  Isolate* isolate = Isolate::Current();
   Code& code = Code::Handle();
+  Function& function = Function::Handle();
   Script& script = Script::Handle();
   String& url = String::Handle();
-  String& name = String::Handle();
   PcDescriptors& descriptors = PcDescriptors::Handle();
   Array& ic_array = Array::Handle();
   ICData& ic_data = ICData::Handle();
-  for (int i = 0; i < functions.Length(); i++) {
-    function ^= functions.At(i);
+  // 2^64-1: 20 digits. String format: "<line>-<line>\0".
+  const intptr_t kMaxLineRangeLen = 42;
+  char line_str[kMaxLineRangeLen];
+
+  while (index < functions.Length()) {
+    HANDLESCOPE(isolate);
+    function ^= functions.At(index);
 
-    JSONObject jsobj(&jsarr);
     script = function.script();
     url = script.url();
-    name = function.QualifiedUserVisibleName();
-    jsobj.AddProperty("source", url.ToCString());
-    jsobj.AddProperty("function", name.ToCString());
-
-    JSONArray jsarr(&jsobj, "hits");
+    if (!url.Equals(source)) {
+      // Abort adding hitcounts to this particular script file entry as soon as
+      // we find a function residing in a different script.
+      return index;
+    }
 
     if (function.HasCode()) {

[Ivan Posva, 2013/09/27 20:19:54]

As we discussed to get more complete coverage of functions it might be necessary
to compile all of the functions instead of just assigning them a zero hit count.

[Michael Lippautz (Google), 2013/09/27 21:46:50]

Done.

       // Print the hit counts for all IC datas.
@@ -46,6 +52,7 @@ void CodeCoverage::PrintClass(const Class& cls, const JSONArray& jsarr) {
       descriptors = code.pc_descriptors();
 
       for (int j = 0; j < descriptors.Length(); j++) {
+        HANDLESCOPE(isolate);
         PcDescriptors::Kind kind = descriptors.DescriptorKind(j);
         // Only IC based calls have counting.
         if ((kind == PcDescriptors::kIcCall) ||
@@ -55,26 +62,54 @@ void CodeCoverage::PrintClass(const Class& cls, const JSONArray& jsarr) {
           if (!ic_data.IsNull()) {
             intptr_t token_pos = descriptors.TokenPos(j);
             intptr_t line = -1;
-            intptr_t col = -1;
-            script.GetTokenLocation(token_pos, &line, &col);
-            JSONObject ic_info(&jsarr);
-            ic_info.AddProperty("line", line);
-            ic_info.AddProperty("col", col);
-            ic_info.AddProperty("count", ic_data.AggregateCount());
+            script.GetTokenLocation(token_pos, &line, NULL);
+            hits_arr.AddValue(line);
+            hits_arr.AddValue(ic_data.AggregateCount());
           }
         }
       }
     } else {
-      // The function has no code so it was never executed and thus we add one
-      // zero count hit at the first token index.
-      intptr_t line = -1;
-      intptr_t col = -1;
-      script.GetTokenLocation(function.token_pos(), &line, &col);
-      JSONObject func_info(&jsarr);
-      func_info.AddProperty("line", line);
-      func_info.AddProperty("col", col);
-      func_info.AddProperty("count", static_cast<intptr_t>(0));
+      // The function has no code so it was never executed and thus we add a
+      // zero count hit for the line (or range of lines) the function is defined
+      // on.
+      intptr_t start_line = -1;
+      intptr_t end_line = -1;
+      script.GetTokenLocation(function.token_pos(), &start_line, NULL);
+      script.GetTokenLocation(function.end_token_pos(), &end_line, NULL);
+      if (start_line == end_line) {
+        hits_arr.AddValue(start_line);
+        hits_arr.AddValue(static_cast<intptr_t>(0));
+      } else {
+        OS::SNPrint(line_str, kMaxLineRangeLen, "%d-%d", start_line, end_line);

[Ivan Posva, 2013/09/27 20:19:54]

Please remove the : from the description of the change as well.

[Michael Lippautz (Google), 2013/09/27 21:46:50]

Done. There are no ranges anymore.

+        hits_arr.AddValue(line_str);
+        hits_arr.AddValue(static_cast<intptr_t>(0));
+      }
     }
+
+    index++;
+  }
+  return index;
+}
+
+void CodeCoverage::PrintClass(const Class& cls, const JSONArray& jsarr) {
+  Isolate* isolate = Isolate::Current();
+  const Array& functions = Array::Handle(cls.functions());
+  ASSERT(!functions.IsNull());
+  Function& function = Function::Handle();
+  Script& script = Script::Handle();
+  String& url = String::Handle();
+  int i = 0;
+  while (i < functions.Length()) {

[Ivan Posva, 2013/09/27 20:19:54]

As discussed off-line we also need to iterate over the closure functions to
gather their coverage once we have finished traversing all functions.

[Michael Lippautz (Google), 2013/09/27 21:46:50]

Done.

+    HANDLESCOPE(isolate);
+    function ^= functions.At(i);
+
+    JSONObject jsobj(&jsarr);
+    script = function.script();
+    url = script.url();
+    jsobj.AddProperty("source", url.ToCString());
+
+    JSONArray hits_arr(&jsobj, "hits");
+    i = PrintFunctionsInSource(url, functions, i, hits_arr);
   }
 }