can't resolve against exports when loading from .dill

[sigmundch]

Example: b.dart:

  export 'dart:io' show File;

a.dart:

  import 'b.dart';
  main() => 'x' is File;

when compiling a.dart using the batch compilation, there are no issues. If we compile it with IKG, there is a warning that File cannot be resolved in a.dart.

Note: if you provide compileSdk to the IKG options, then it works fine.

[sigmundch]

I started looking into this, turns out this is not related to IKG specifically, but this is an issue in general about exports and modular compilation. Here is a small repro using the modular APIs:

import 'package:front_end/src/testing/compiler_common.dart';

main() async {
  var sources = <String, dynamic>{
    'a.dart': 'a() => print("hi"); class A{} ',
        'b.dart': 'export "a.dart";',

        'c.dart': 'import "b.dart"; class C extends A {}',
  };
  sources['ab.dill'] = await summarize(['a.dart', 'b.dart'], sources);
  await compileUnit(['c.dart'], sources, inputSummaries: ['ab.dill']);
}

Run it as: DART_CONFIGURATION=ReleaseX64 dart repro.dart

This produces the following error message:

Warning: Type 'A' not found.
Error: The type 'A' can't be used as supertype.
Error: Compilation aborted due to fatal error.

What is our current strategy for representing exports in dill files?

/cc @peter-ahe-google

[peter-ahe-google]

Although we can represent exports in kernel, there's currently no way to represent an export scope. In dartk, I implemented a hack to add a member named "_exports#" to a library that would contain a list of exported elements.

[sigmundch]

Interesting - that makes sense - I recall @scheglov needed some of the export scopes for summaries as well, but I don't know if what he needs is more limited than what we need here.

I'm guessing that at this time we probably want to extend kernel with special nodes for the export scope, correct? Then the VM can simply ignore those nodes when deserializing kernel?

[scheglov]

Yes, KernelDriver currently uses raw export information stored in UnlinkedUnit of FileState for each library to recompute full export namespaces, both after compiling and loading library kernel from a file. Unfortunately this breaks when we don't load library by library and don't have FileState, like happens when we load full SDK (or package bundles).

The idea was that we add this information into Kernel, I remember someone saying that they planning to do this. OTOH, it might be the person who left Kernel team. Anyway, I think that we should add export namespace directly into Kernel and if a client does not need export namespace, it can be ignored.

[sigmundch]

sounds good - @peter-ahe-google I assigned you on this issue assuming that you have the full context to move this forward and can iterate quickly with Kevin on the changes in kernel, but let us know if we should have someone else help on this.

[peter-ahe-google]

I've discussed this with @kmillikin today, he will look for a solution in kernel, and then I'll implement it in fasta.

[sigmundch]

Thanks both for taking a look!

I should note that this issue is in the front burner for both flutter hot-reload (package:flutter uses tons of exports, so trival flutter apps run into this), and bazel integration (many packages used by our internal customers use exports too, the modular compilation in bazel breaks).

[scheglov]

BTW, the hack does not work nice with outline shaker. We still need real presentation of exported entities in Kernel. And for now probably another hack into shaker.

[peter-ahe-google]

@scheglov is the problem with the outline shaker that the field is private?

[scheglov]

Ah, sorry. I should have provided more information. The problem happens when you export a library that exports a library.

    addLibrarySource('/a.dart', 'export "b.dart";');
    addLibrarySource('/b.dart', 'export "c.dart";');
    addLibrarySource('/c.dart', 'class C {} enum E { v } typedef F();');
    var library = await checkLibrary('import "a.dart"; C c; E e; F f;');

When we shake a.dart we lost the knowledge that outlines of nodes from c.dart should be included.

Well, actually I think currently this happens even for b.dart, but I was trying to fix this by (conservatively ignoring combinators) including outlines of all nodes from c.dart.

[peter-ahe-google]

Are we doing incremental tree-shaking? I've always thought of tree-shaking as something that's a whole-program transformation. Am I conflating concepts here? You say "outline shaker" and I think "tree shaker", not the same thing?

[scheglov]

The way Bazel is supposed to work with kernels is that you take kernels of direct dependencies, and compile the target sources. Then you shake it to keep only required outlines for direct dependencies. Next target. So, yes, I think we will do incremental tree-shaking.

[peter-ahe-google]

So basically, what Bazel needs is outlines without private members?

[scheglov]

Yes, but that's not what causes the problem here.

Because of the way exports are represented now (additional exports), we don't know that {uri, name} represents are reference to a node in c.dart, so we don't keep it in the output outline.

[peter-ahe-google]

@scheglov thank you, I think I'm getting it now.

What should be in the outline of a.dart? References to c.dart or copies of the elements?

[scheglov]

Each kernel file is a set of libraries. Some of the libraries are full, end some are outlines (external).

So, the kernel file for a.dart should include libraries a.dart, b.dart and c.dart in it.

The library a.dart should include all its nodes (zero in the example above), plus all LibraryDependency nodes. The export LibraryDependency in a.dart should have references to nodes that were exported from c.dart through the export from b.dart.

The library b.dart should be empty.

The library c.dart should include outlines (no bodies, no initializers) of nodes of c.dart that are exported (so referenced) by a.dart.

[scheglov]

Actually, I will try to implement it myself. Adding NamedNodes LibraryDependency and serializing their canonical names should not be so hard.

[scheglov]

https://codereview.chromium.org/3008763002

[sigmundch]

From chatting with Peter this morning I believe we are pretty close on this. Konstantin's fixes the general valid input. We just need to add some additional representation in kernel to handle erroneous cases. Correct?

[sigmundch]

@kmillikin - any updates on adding the kernel representation needed for this?

[kmillikin]

I think we need to understand what the feature you need is. I can gather from reading this issue that something still doesn't work because something is missing from Kernel.

Is there an easy way that I can reproduce the thing that doesn't work so I could try to understand the problem? Or can someone who understands the problem summarize it and speculate about the missing feature (better yet, propose a Dart-like syntax for it)?

[peter-ahe-google]

The remaining issue boils down to representing duplicated exports that lead to a compile-time error. See #29840.

[jensjoha]

If this issue boils down to another issue, can we close this one?