Perform MixinFullResolution only for source libraries.

Perform MixinFullResolution only for source libraries.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/0dd81884bf12cd0bb760ab75e41d28e9d1719c7a

[Paul Berry, 2017-05-26 20:13:46]

lgtm assuming Kevin, Peter, or Siggi approves

[Siggi Cherem (dart-lang), 2017-05-26 22:08:33]

lgtm too, but let's still wait on Kevin for API feedback and general direction
of this in kernel.

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
File pkg/kernel/lib/transformations/mixin_full_resolution.dart (right):

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/mixin_full_resolution.dart:47: var program =
libraries.first.enclosingProgram;
consider adding a TODO that this reference will be removed?

Mainly this felt a bit like an unnecessary change (why not do program + filter
like you done elsewhere), but I think it makes sense when you put in context the
other changes you are making to make CoreTypes and ClassHierarchy parameters
instead of creating them here.

[ahe, 2017-05-29 14:16:54]

I think more work is needed for this to work.

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
File pkg/kernel/lib/transformations/mixin_full_resolution.dart (right):

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/mixin_full_resolution.dart:90:
transformClass(processedClasses, transformedClasses, class_.superclass);
This method will also transform superclasses even if they aren't in the list of
libraries passed in.

[scheglov, 2017-05-30 04:20:15]

PTAL

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
File pkg/kernel/lib/transformations/mixin_full_resolution.dart (right):

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/mixin_full_resolution.dart:47: var program =
libraries.first.enclosingProgram;
On 2017/05/26 22:08:32, Siggi Cherem (dart-lang) wrote:
> consider adding a TODO that this reference will be removed?
> 
> Mainly this felt a bit like an unnecessary change (why not do program + filter
> like you done elsewhere), but I think it makes sense when you put in context
the
> other changes you are making to make CoreTypes and ClassHierarchy parameters
> instead of creating them here.

Done.

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/mixin_full_resolution.dart:90:
transformClass(processedClasses, transformedClasses, class_.superclass);
On 2017/05/29 14:16:54, ahe wrote:
> This method will also transform superclasses even if they aren't in the list
of
> libraries passed in.

These libraries are new libraries to transform, with the expectation that all
the other libraries have already been transformed. I've added this expectation
into the comment for [transform].

And when a superclass has already been transformed, we will exit because of the
line #95.

[ahe, 2017-05-30 09:51:16]

lgtm

OK if you can find a way to check that we only transform classes in the set of
libraries we have asked to transform.

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
File pkg/kernel/lib/transformations/mixin_full_resolution.dart (right):

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/mixin_full_resolution.dart:95: if
(!class_.isMixinApplication) return;
Could you add an assertion here that checks if class_ is in the set of libraries
to be transformed?

[scheglov, 2017-05-30 16:33:05]

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
File pkg/kernel/lib/transformations/mixin_full_resolution.dart (right):

https://codereview.chromium.org/2912503003/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/mixin_full_resolution.dart:95: if
(!class_.isMixinApplication) return;
On 2017/05/30 09:51:16, ahe wrote:
> Could you add an assertion here that checks if class_ is in the set of
libraries
> to be transformed?

Done.

[scheglov, 2017-05-30 16:33:35]

Description was changed from

==========
Perform MixinFullResolution only for source libraries.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
Perform MixinFullResolution only for source libraries.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/0dd81884bf12cd0bb760ab75e41d28e9d1719c7a
==========

[scheglov, 2017-05-30 16:33:36]

Committed patchset #3 (id:40001) manually as
0dd81884bf12cd0bb760ab75e41d28e9d1719c7a (presubmit successful).