Add import dependencies to Kernel libraries and use them to resynthesize ImportElement(s) in Analyz…

pkg/kernel/lib/binary/ast_from_binary.dart

 // Copyright (c) 2016, 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.
 library kernel.ast_from_binary;
 
 import 'dart:convert';
 import 'dart:typed_data';
 
 import '../ast.dart';
 import '../transformations/flags.dart';
@@ skipping 381 matching lines @@
     _mergeNamedNodeList(library.fields, readField, library);
     _mergeNamedNodeList(library.procedures, readProcedure, library);
 
     debugPath.removeLast();
     _currentLibrary = null;
     return library;
   }
 
   void _readLibraryDependencies(Library library) {
     int length = readUInt();
-    if (library.isExternal) {
-      assert(length == 0);

[Siggi Cherem (dart-lang), 2017/07/20 18:29:00]

do we need the library dependencies for external libraries? If not, I feel the
fix should be elsewhere:

 - we could we fix it in the serialization, or
 - in how we produce external libraries in fasta

I have a preference for the latter until we figure out what is the right
approach. In particular, right now fasta doesn't seem to have a clear way to
generate external libraries, and we do some adhoc things for them (e.g. in the
public API we load .dill into summaries, and then mark them as external, but we
don't mutate them in any other way - if they happen to have method bodies or
library dependencies, we keep them around).

[scheglov, 2017/07/20 19:33:53]

Analyzer's LibraryElement has "imports" that should return all imports of the
library, regardless whether this LibraryElement comes from a "local" or
"external" summary / kernel. So, yes, we need it. And it is not clear to me
which problem this check was solving. It could be to check the status quo, but
my change effectively changes this expectation.

[Siggi Cherem (dart-lang), 2017/07/20 22:50:36]

I see - I understand now. Thanks for the explanation.

My understanding is that an "external library" is a library where some
information may be missing - so far I don't believe there is a requirement that
some information must be missing (like lib dependencies). So, I now agree that
your change seems good here.

Regarding analyzer and input summaries, just want to make sure we are on the
same page:
  - I believe summaries are not external when they are serialized or when we
read them.
  - we will use external libraries when we read a summary, compile code, and
emit a program with some nodes that were derived from the summary - these
derived nodes will be marked as external (and if we do tree-shaking, they might
only be a subset of what was initially read from the summary)

[scheglov, 2017/07/21 16:21:33]

I agree about external libraries.
And we don't need dependencies for them.
I made the change to kernel_generator_impl.dart and restored the check here.

-      return;
-    }
     library.dependencies.length = length;
     for (int i = 0; i < length; ++i) {
       var flags = readByte();
       var annotations = readExpressionList();
       var targetLibrary = readLibraryReference();
       var prefixName = readStringOrNullIfEmpty();
       var names = readCombinatorList();
       library.dependencies[i] = new LibraryDependency.byReference(
           flags, annotations, targetLibrary, prefixName, names)
         ..parent = library;
@@ skipping 825 matching lines @@
       ..fileOffset = offset
       ..fileEqualsOffset = fileEqualsOffset;
   }
 
   int readOffset() {
     // Offset is saved as unsigned,
     // but actually ranges from -1 and up (thus the -1)
     return readUInt() - 1;
   }
 }