Add types referenced in is-checks to the class hierarchy

Add types referenced in is-checks to the class hierarchy

R=johnniwinther@google.com, sigmund@google.com
BUG=

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

[Harry Terkelsen, 2017-08-31 22:43:38]

het@google.com changed reviewers:
+ johnniwinther@google.com, sigmund@google.com

[Siggi Cherem (dart-lang), 2017-08-31 23:33:54]

lgtm

[Johnni Winther, 2017-09-01 08:21:35]

lgtm

https://codereview.chromium.org/3011793002/diff/1/pkg/compiler/lib/src/univer...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/1/pkg/compiler/lib/src/univer...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:947: void
_registerType(DartType type) {
We probably need to visit function types and typedefs as well. Implement this as
a DartTypeVisitor instead of a recusive function.

[Harry Terkelsen, 2017-09-01 18:48:24]

https://codereview.chromium.org/3011793002/diff/1/pkg/compiler/lib/src/univer...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/1/pkg/compiler/lib/src/univer...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:947: void
_registerType(DartType type) {
On 2017/09/01 08:21:34, Johnni Winther wrote:
> We probably need to visit function types and typedefs as well. Implement this
as
> a DartTypeVisitor instead of a recusive function.

Done.

[Siggi Cherem (dart-lang), 2017-09-01 19:05:13]

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:961:
isChecks.forEach((t) {
nit: isChecks.forEach(_classEnsurer.ensureClassesInType);

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:964:
closurizedMembers.forEach((function) {
I'm surprised we were missing so many here. This code path was also used in the
old compiler pipeline, how come it is not missing types for all of these as
well?

[Johnni Winther, 2017-09-01 19:59:56]

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:964:
closurizedMembers.forEach((function) {
On 2017/09/01 19:05:12, Siggi Cherem (dart-lang) wrote:
> I'm surprised we were missing so many here. This code path was also used in
the
> old compiler pipeline, how come it is not missing types for all of these as
> well?

Because ClassElement.ensureResolved registers the class with the class
hierarchy.

[Harry Terkelsen, 2017-09-01 21:25:33]

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:964:
closurizedMembers.forEach((function) {
On 2017/09/01 19:05:12, Siggi Cherem (dart-lang) wrote:
> I'm surprised we were missing so many here. This code path was also used in
the
> old compiler pipeline, how come it is not missing types for all of these as
> well?

The old pipeline adds classes to the hierarchy as it resolves them. This way
adds them based on the impacts after resolution.

[Harry Terkelsen, 2017-09-01 21:48:57]

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/20001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:961:
isChecks.forEach((t) {
On 2017/09/01 19:05:12, Siggi Cherem (dart-lang) wrote:
> nit: isChecks.forEach(_classEnsurer.ensureClassesInType);

Done.

[Siggi Cherem (dart-lang), 2017-09-01 21:49:38]

lgtm, thanks for the explanations

[Harry Terkelsen, 2017-09-01 22:03:04]

Description was changed from

==========
Add types referenced in is-checks to the class hierarchy
==========

to

==========
Add types referenced in is-checks to the class hierarchy

R=johnniwinther@google.com, sigmund@google.com

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

[Harry Terkelsen, 2017-09-01 22:03:05]

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

[Harry Terkelsen, 2017-09-05 18:09:11]

PTAL, I had to revert to fix some tests

[Siggi Cherem (dart-lang), 2017-09-05 18:20:33]

lgtm

https://codereview.chromium.org/3011793002/diff/80001/pkg/compiler/lib/src/un...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/80001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:1055: class
_ClassEnsurer extends BaseResolutionDartTypeVisitor<dynamic, Null> {
consider adding a second TODO: to explain why you need "Resolution" here (to
deal with MalformedTypes) and that we would like to switch to just use the
regular visitor instead.

One way to do that might be to explicitly model malformed types in the entity
model as well in the future

https://codereview.chromium.org/3011793002/diff/80001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:1082: // 
visitGenericType(GenericType type, _) {
delete?

[Harry Terkelsen, 2017-09-05 18:31:11]

https://codereview.chromium.org/3011793002/diff/80001/pkg/compiler/lib/src/un...
File pkg/compiler/lib/src/universe/resolution_world_builder.dart (right):

https://codereview.chromium.org/3011793002/diff/80001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:1055: class
_ClassEnsurer extends BaseResolutionDartTypeVisitor<dynamic, Null> {
On 2017/09/05 18:20:33, Siggi Cherem (dart-lang) wrote:
> consider adding a second TODO: to explain why you need "Resolution" here (to
> deal with MalformedTypes) and that we would like to switch to just use the
> regular visitor instead.
> 
> One way to do that might be to explicitly model malformed types in the entity
> model as well in the future

Done.

https://codereview.chromium.org/3011793002/diff/80001/pkg/compiler/lib/src/un...
pkg/compiler/lib/src/universe/resolution_world_builder.dart:1082: // 
visitGenericType(GenericType type, _) {
On 2017/09/05 18:20:33, Siggi Cherem (dart-lang) wrote:
> delete?

Done.

[Harry Terkelsen, 2017-09-05 19:53:16]

Description was changed from

==========
Add types referenced in is-checks to the class hierarchy

R=johnniwinther@google.com, sigmund@google.com

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

to

==========
Add types referenced in is-checks to the class hierarchy

R=johnniwinther@google.com, sigmund@google.com
BUG=

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