Tweak public APIs and use them in patch_sdk, dart2js, and kernel-service

pkg/kernel/lib/binary/ast_from_binary.dart

Index: pkg/kernel/lib/binary/ast_from_binary.dart
diff --git a/pkg/kernel/lib/binary/ast_from_binary.dart b/pkg/kernel/lib/binary/ast_from_binary.dart
index f0c34cd1dab49c9515f8e8d54d17baa8b22fc2a0..0cb69c6c6e0dc31f80705dde6cf21ebf95612100 100644
--- a/pkg/kernel/lib/binary/ast_from_binary.dart
+++ b/pkg/kernel/lib/binary/ast_from_binary.dart
@@ -157,18 +157,27 @@ class BinaryBuilder {
     return list;
   }
 
-  void _fillTreeNodeList(
-      List<TreeNode> list, TreeNode buildObject(), TreeNode parent) {
-    list.length = readUInt();
-    for (int i = 0; i < list.length; ++i) {
-      list[i] = buildObject()..parent = parent;
+  void _fillTreeNodeList(List<TreeNode> list, TreeNode buildObject(),

[ahe, 2017/07/06 13:12:22]

If it isn't just me that find this method confusing to begin with, I suggest
that you remove the parameter shouldWriteData and add this method instead:

void _skipNodeList(void buildObject()) {
  int length = readUInt();
  for (int i = 0; i < length; i++) {
    buildObject();
  }
}

[Siggi Cherem (dart-lang), 2017/07/06 19:05:28]

I like it. Done.

+      TreeNode parent, bool shouldWriteData) {
+    var length = readUInt();
+    if (shouldWriteData) list.length = length;
+    for (int i = 0; i < length; ++i) {
+      TreeNode object = buildObject();
+      if (shouldWriteData) {
+        list[i] = object..parent = parent;
+      }
     }
   }
 
-  void _fillNonTreeNodeList(List<Node> list, Node buildObject()) {
-    list.length = readUInt();
-    for (int i = 0; i < list.length; ++i) {
-      list[i] = buildObject();
+  void _fillNonTreeNodeList(
+      List<Node> list, Node buildObject(), bool shouldWriteData) {
+    var length = readUInt();
+    if (shouldWriteData) list.length = length;
+    for (int i = 0; i < length; ++i) {
+      Node object = buildObject();
+      if (shouldWriteData) {
+        list[i] = object;
+      }
     }
   }
 
@@ -184,7 +193,7 @@ class BinaryBuilder {
     if (_isReadingLibraryImplementation) {
       // When reading the library implementation, overwrite the whole list
       // with the new one.
-      _fillTreeNodeList(list, readObject, parent);
+      _fillTreeNodeList(list, readObject, parent, true);
     } else {
       // When reading an external library, the results should either be:
       // - merged with the existing external library definition (if any)
@@ -356,7 +365,8 @@ class BinaryBuilder {
 
     debugPath.add(library.name ?? library.importUri?.toString() ?? 'library');
 
-    _fillTreeNodeList(library.annotations, readExpression, library);
+    _fillTreeNodeList(
+        library.annotations, readExpression, library, shouldWriteData);
     _readLibraryDependencies(library);
     _mergeNamedNodeList(library.typedefs, readTypedef, library);
     _mergeNamedNodeList(library.classes, readClass, library);
@@ -445,7 +455,7 @@ class BinaryBuilder {
     readAndPushTypeParameterList(node.typeParameters, node);
     var supertype = readSupertypeOption();
     var mixedInType = readSupertypeOption();
-    _fillNonTreeNodeList(node.implementedTypes, readSupertype);
+    _fillNonTreeNodeList(node.implementedTypes, readSupertype, shouldWriteData);
     _mergeNamedNodeList(node.fields, readField, node);
     _mergeNamedNodeList(node.constructors, readConstructor, node);
     _mergeNamedNodeList(node.procedures, readProcedure, node);
@@ -529,7 +539,8 @@ class BinaryBuilder {
     var function = readFunctionNode();
     pushVariableDeclarations(function.positionalParameters);
     pushVariableDeclarations(function.namedParameters);
-    _fillTreeNodeList(node.initializers, readInitializer, node);
+    _fillTreeNodeList(
+        node.initializers, readInitializer, node, shouldWriteData);
     variableStack.length = 0;
     var transformerFlags = getAndResetTransformerFlags();
     debugPath.removeLast();