Make TypeMask an interface and start hiding implementation details of FlatTypeMask.

Make TypeMask an interface and start hiding implementation details of FlatTypeMask.

R=kasperl@google.com

Committed: https://code.google.com/p/dart/source/detail?r=22246

[kasperl, 2013-04-30 14:03:23]

LGTM with a few ideas for future cleanups.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2278:
receiverClass = mask.topClass();
Here we shouldn't compute a receiver class but rather operate on the mask. Could
we redo hasMatchingMember in terms of the mask?

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1702:
world.registerInstantiatedClass(mask.topClass(), work.resolutionTree);
Would exactClass work here?

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/types.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/types.dart:23: if (element ==
compiler.intClass || element == backend.jsIntClass) {
Move these predicates into TypeMask, so you can ask
mask.containsOnlyInts(compiler)?

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/types/type_mask.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:60: ClassElement
topClass();
It would be nice if we could get rid of this in another CL.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:80: /**
I'd move this to another file in the next CL.

[ngeoffray, 2013-05-01 12:56:16]

Thanks Kasper for the comments. PTAL.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2278:
receiverClass = mask.topClass();
On 2013/04/30 14:03:24, kasperl wrote:
> Here we shouldn't compute a receiver class but rather operate on the mask.
Could
> we redo hasMatchingMember in terms of the mask?

Done.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1702:
world.registerInstantiatedClass(mask.topClass(), work.resolutionTree);
On 2013/04/30 14:03:24, kasperl wrote:
> Would exactClass work here?

No, so I use containedClasses instead.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/types.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/types.dart:23: if (element ==
compiler.intClass || element == backend.jsIntClass) {
On 2013/04/30 14:03:24, kasperl wrote:
> Move these predicates into TypeMask, so you can ask
> mask.containsOnlyInts(compiler)?

Done.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/types/type_mask.dart (right):

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:60: ClassElement
topClass();
On 2013/04/30 14:03:24, kasperl wrote:
> It would be nice if we could get rid of this in another CL.

I just got rid of this. As well as exactType.

https://codereview.chromium.org/14636002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:80: /**
On 2013/04/30 14:03:24, kasperl wrote:
> I'd move this to another file in the next CL.

Will do.

[kasperl, 2013-05-01 13:01:04]

LGTM! This is great.

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2307: 
Very nice. I'd remove this newline though.

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1696: //
TODO(kasperl): This looks shaky. It makes sense if the type is
Should we remove the TODO?

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/types/type_mask.dart (right):

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:64: TypeMask
union(TypeMask other, Compiler compiler);
Add documentation for union and intersection?

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:72: 
Remove one newline here.

[ngeoffray, 2013-05-01 13:16:40]

Thanks Kasper.

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2307: 
On 2013/05/01 13:01:04, kasperl wrote:
> Very nice. I'd remove this newline though.

Done.

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1696: //
TODO(kasperl): This looks shaky. It makes sense if the type is
On 2013/05/01 13:01:04, kasperl wrote:
> Should we remove the TODO?

Done.

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/types/type_mask.dart (right):

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:64: TypeMask
union(TypeMask other, Compiler compiler);
On 2013/05/01 13:01:04, kasperl wrote:
> Add documentation for union and intersection?

Done.

https://codereview.chromium.org/14636002/diff/6001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/types/type_mask.dart:72: 
On 2013/05/01 13:01:04, kasperl wrote:
> Remove one newline here.

Done.

[ngeoffray, 2013-05-01 13:29:17]

Committed patchset #3 manually as r22246 (presubmit successful).