commit 1b41780198604920cb3bcab9e4f68439248dae77
author Jens Johansen <jensj@google.com> Wed Mar 04 07:21:03 2020 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Wed Mar 04 07:21:03 2020 +0000
tree 97ca1ca1d5c8ea9b9c10c342eedef65b4b2def2f
parent 57462f9ca52000932d2ac82613808131650b440c

[kernel] Investigative inspection into BinaryPrinter

This CL investigates some properties around the BinaryPrinter in the form
of non-optional-assert-like-things.
Said another way, if the assumptions added here are wrong, we'll crash,
but if the bots doesn't explode the assumptions are probably true and
we can procede from there.

Change-Id: Ia366be0e81164a4004f6d6764002cbb0f7383008
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137978
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>

pkg/kernel/lib/binary/ast_to_binary.dart

diff --git a/pkg/kernel/lib/binary/ast_to_binary.dart b/pkg/kernel/lib/binary/ast_to_binary.dart
index 3df06ad..190429c 100644
--- a/pkg/kernel/lib/binary/ast_to_binary.dart
+++ b/pkg/kernel/lib/binary/ast_to_binary.dart
@@ -862,7 +862,30 @@
   void checkCanonicalName(CanonicalName node) {
     if (_knownCanonicalNameNonRootTops.contains(node.nonRootTop)) return;
     if (node == null || node.isRoot) return;
-    if (_reindexedCanonicalNames.contains(node)) return;
+    bool indexCheckContains;
+    {
+      if (node.index < 0) {
+        indexCheckContains = false;
+      } else if (node.index >= _canonicalNameList.length) {
+        indexCheckContains = false;
+      } else {
+        CanonicalName claim = _canonicalNameList[node.index];
+        if (node != claim) {
+          indexCheckContains = false;
+        } else {
+          indexCheckContains = true;
+        }
+      }
+    }
+
+    bool setContains = _reindexedCanonicalNames.contains(node);
+    if (setContains != indexCheckContains) {
+      throw "Unexpected difference between set and index";
+    }
+
+    if (_reindexedCanonicalNames.contains(node)) {
+      return;
+    }
 
     checkCanonicalName(node.parent);
     node.index = _canonicalNameList.length;

pkg/kernel/lib/binary/limited_ast_to_binary.dart

diff --git a/pkg/kernel/lib/binary/limited_ast_to_binary.dart b/pkg/kernel/lib/binary/limited_ast_to_binary.dart
index 104cfe6..6f5208e 100644
--- a/pkg/kernel/lib/binary/limited_ast_to_binary.dart
+++ b/pkg/kernel/lib/binary/limited_ast_to_binary.dart
@@ -13,17 +13,8 @@
 class LimitedBinaryPrinter extends BinaryPrinter {
   final LibraryFilter predicate;
 
-  /// Excludes all uriToSource information.
-  ///
-  /// By default the [predicate] above will only exclude canonical names and
-  /// kernel libraries, but it will still emit the sources for all libraries.
-  /// filtered by libraries matching [predicate].
-  // TODO(sigmund): provide a way to filter sources directly based on
-  // [predicate]. That requires special logic to handle sources from part files.
-  final bool excludeUriToSource;
-
   LimitedBinaryPrinter(
-      Sink<List<int>> sink, this.predicate, this.excludeUriToSource,
+      Sink<List<int>> sink, this.predicate, bool excludeUriToSource,
       {bool includeOffsets = true})
       : super(sink,
             includeSources: !excludeUriToSource,
@@ -55,7 +46,9 @@
 
   @override
   void writeNode(Node node) {
-    if (node is Library && !predicate(node)) return;
+    if (node is Library) {
+      throw "Internal error: writeNode should not see a Library.";
+    }
     super.writeNode(node);
   }