Add export dependencies to Kernel libraries and use them to resynthesize ExportElement(s) in Analyz…

Add export dependencies to Kernel libraries and use them to resynthesize ExportElement(s) in Analyzer.

R=ahe@google.com, brianwilkerson@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/8a08dbd08e85eb6e59f3633a23e95a77bf56190b

[Brian Wilkerson, 2017-07-26 13:39:35]

lgtm

[Siggi Cherem (dart-lang), 2017-07-26 19:22:43]

lgtm with the fix below

https://codereview.chromium.org/2991753002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/source/source_library_builder.dart (right):

https://codereview.chromium.org/2991753002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:161:
loader.read(resolve(uri), charOffset, accessor: this),
rather than rereading it here, store it in a local variable instead.

LibraryBuilder exportedLibrary = loader.read(...);
exportedLibrary.addExporter(this, combinators, charOffset);
exports.add(new Export(this, exportedLibrary, combinators, charset));

[scheglov, 2017-07-26 19:33:05]

https://codereview.chromium.org/2991753002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/source/source_library_builder.dart (right):

https://codereview.chromium.org/2991753002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:161:
loader.read(resolve(uri), charOffset, accessor: this),
On 2017/07/26 19:22:42, Siggi Cherem (dart-lang) wrote:
> rather than rereading it here, store it in a local variable instead.
> 
> LibraryBuilder exportedLibrary = loader.read(...);
> exportedLibrary.addExporter(this, combinators, charOffset);
> exports.add(new Export(this, exportedLibrary, combinators, charset));

Done.

[scheglov, 2017-07-26 19:33:24]

Description was changed from

==========
Add export dependencies to Kernel libraries and use them to resynthesize
ExportElement(s) in Analyzer.

R=ahe@google.com, brianwilkerson@google.com, sigmund@google.com
BUG=
==========

to

==========
Add export dependencies to Kernel libraries and use them to resynthesize
ExportElement(s) in Analyzer.

R=ahe@google.com, brianwilkerson@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/8a08dbd08e85eb6e59f3633a23e95a77bf56190b
==========

[scheglov, 2017-07-26 19:33:25]

Committed patchset #2 (id:20001) manually as
8a08dbd08e85eb6e59f3633a23e95a77bf56190b (presubmit successful).

[ahe, 2017-08-08 09:47:58]

Fasta changes, LGTM!

Thank you for making the names of exportScope and importScope consistent.

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart (right):

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:742: }
I think it's possible to replace lines 735-742 with this:

Library exportedLibrary = import.exported.target;

For future reference: You can always access the corresponding kernel object from
a builder using `target`.

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/source/source_library_builder.dart (right):

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:156: var
exportedLibrary = loader.read(resolve(uri), charOffset, accessor: this);
Missing type.

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:158:
exports.add(new Export(this, exportedLibrary, combinators, charOffset));
Question: you need access to the metadata as well, right?

[scheglov, 2017-08-08 16:18:46]

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart (right):

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:742: }
On 2017/08/08 09:47:58, ahe wrote:
> I think it's possible to replace lines 735-742 with this:
> 
> Library exportedLibrary = import.exported.target;
> 
> For future reference: You can always access the corresponding kernel object
from
> a builder using `target`.

OK, understood.
See https://codereview.chromium.org/2993283002 for the fix.

[scheglov, 2017-08-08 16:31:17]

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/source/source_library_builder.dart (right):

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:156: var
exportedLibrary = loader.read(resolve(uri), charOffset, accessor: this);
On 2017/08/08 09:47:58, ahe wrote:
> Missing type.

It's not like it is worse typed now than it was. There is no semantic or
cognitive difference between extracting a local variable with "var" or keeping
the expression inlined.

Also, the type is quite cumbersome "LibraryBuilder<TypeBuilder, dynamic>".

https://codereview.chromium.org/2991753002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:158:
exports.add(new Export(this, exportedLibrary, combinators, charOffset));
On 2017/08/08 09:47:58, ahe wrote:
> Question: you need access to the metadata as well, right?

Yes, we need metadata for everything about what the specification says that it
has metadata, including all directives.