Store parts in Kernel Library, resynthesize parts in Analyzer.

Store parts in Kernel Library, resynthesize parts in Analyzer.

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

Committed: https://github.com/dart-lang/sdk/commit/60a576108c903c8c34a2ff8cd64dfe6cd911f4d6

[Siggi Cherem (dart-lang), 2017-08-02 23:06:23]

kernel/fe changes lgtm after minor changes below

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

https://codereview.chromium.org/2988373002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:779: //
TODO(scheglov): https://github.com/dart-lang/sdk/issues/30284
Nit, format as: 

// TODO(scheglov): Add support for annotations, see
// https://github.com/dart-lang/sdk/issues/30284.

https://codereview.chromium.org/2988373002/diff/1/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2988373002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:272: final List<LibraryPart> parts;
should we add @informational to this field?

[scheglov, 2017-08-03 00:16:35]

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

https://codereview.chromium.org/2988373002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:779: //
TODO(scheglov): https://github.com/dart-lang/sdk/issues/30284
On 2017/08/02 23:06:23, Siggi Cherem (dart-lang) wrote:
> Nit, format as: 
> 
> // TODO(scheglov): Add support for annotations, see
> // https://github.com/dart-lang/sdk/issues/30284.

Done.

https://codereview.chromium.org/2988373002/diff/1/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2988373002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:272: final List<LibraryPart> parts;
On 2017/08/02 23:06:23, Siggi Cherem (dart-lang) wrote:
> should we add @informational to this field?

Acknowledged.

[ahe, 2017-08-03 10:43:47]

lgtm

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
File pkg/analyzer/lib/src/kernel/resynthesize.dart (left):

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:619: ElementImpl
_getElement(kernel.CanonicalName name) {
It would have been quicker for me to review this if you hadn't moved this code,
or if you had at least mentioned that you had moved it.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
File pkg/analyzer/lib/src/kernel/resynthesize.dart (right):

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:83: // If the name is private, it
is prefixed with a library URI.
I don't understand this comment and how it relates to the code below.

Do you mean "parent" when you say "prefix"?

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:145: throw new
UnimplementedError('Should not be reached.');
Consider using this message instead:

"Internal error: ${parentElement.runtimeType} unexpected."

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:491: librarySource =
resynthesizer._getSource(uriStr);
Move this line outside the constructor body.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:526: .where((n) => n.fileUri ==
context.fileUri);
Stuff like this is potentially expensive and can turn naive algorithms
quadratic. Consider using toList and a cache.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:619: List<kernel.Field>
kernelFields = kernelUnit.fields.toList(growable: false);
It's non-obvious that this API requires you to call toList. Consider converting
the getters to methods named something like "listFields()".

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

https://codereview.chromium.org/2988373002/diff/40001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:781:
library.addPart(new LibraryPart([], fileUri));
Add type on list literal.

[Brian Wilkerson, 2017-08-03 13:41:35]

lgtm

[scheglov, 2017-08-03 17:25:56]

Thank you for the comments.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
File pkg/analyzer/lib/src/kernel/resynthesize.dart (right):

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:83: // If the name is private, it
is prefixed with a library URI.
On 2017/08/03 10:43:46, ahe wrote:
> I don't understand this comment and how it relates to the code below.
> 
> Do you mean "parent" when you say "prefix"?

See the comment to CanonicalName.

///      Qualified name:
///         if private: URI of library
///         Name text
///
/// The "qualified name" allows a member to have a name that is private to
/// a library other than the one containing that member.
class CanonicalName {

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:145: throw new
UnimplementedError('Should not be reached.');
On 2017/08/03 10:43:47, ahe wrote:
> Consider using this message instead:
> 
> "Internal error: ${parentElement.runtimeType} unexpected."

Done.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:491: librarySource =
resynthesizer._getSource(uriStr);
On 2017/08/03 10:43:46, ahe wrote:
> Move this line outside the constructor body.

Done.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:526: .where((n) => n.fileUri ==
context.fileUri);
On 2017/08/03 10:43:47, ahe wrote:
> Stuff like this is potentially expensive and can turn naive algorithms
> quadratic. Consider using toList and a cache.

Done.

https://codereview.chromium.org/2988373002/diff/40001/pkg/analyzer/lib/src/ke...
pkg/analyzer/lib/src/kernel/resynthesize.dart:619: List<kernel.Field>
kernelFields = kernelUnit.fields.toList(growable: false);
On 2017/08/03 10:43:46, ahe wrote:
> It's non-obvious that this API requires you to call toList. Consider
converting
> the getters to methods named something like "listFields()".

Done.

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

https://codereview.chromium.org/2988373002/diff/40001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:781:
library.addPart(new LibraryPart([], fileUri));
On 2017/08/03 10:43:47, ahe wrote:
> Add type on list literal.

Done.

[scheglov, 2017-08-03 17:36:05]

Description was changed from

==========
Store parts in Kernel Library, resynthesize parts in Analyzer.

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

to

==========
Store parts in Kernel Library, resynthesize parts in Analyzer.

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

Committed:
https://github.com/dart-lang/sdk/commit/60a576108c903c8c34a2ff8cd64dfe6cd911f4d6
==========

[scheglov, 2017-08-03 17:36:06]

Committed patchset #4 (id:60001) manually as
60a576108c903c8c34a2ff8cd64dfe6cd911f4d6 (presubmit successful).