dart2js cps: Use more precise type inference for direct index access.

dart2js cps: Use more precise type inference for direct index access.

R=karlklose@google.com

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

[asgerf, 2015-09-15 08:08:42]

asgerf@google.com changed reviewers:
+ karlklose@google.com

[karlklose, 2015-09-15 08:17:07]

LGTM!

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_mask_system.dart (right):

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_mask_system.dart:302: if (type is
ContainerTypeMask) {
We should maybe use a predicate instead of the is check because we want to make
the interface for type inference more explicit so that we are able to change the
internal representation and inference algorithm.

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:2161: } else if (uri ==
Uris.dart__native_typed_data) {
This is already done in Compiler.onLibraryScanned (the field is called
'typedDataClass').

[asgerf, 2015-09-15 10:46:14]

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_mask_system.dart (right):

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_mask_system.dart:302: if (type is
ContainerTypeMask) {
On 2015/09/15 08:17:07, karlklose wrote:
> We should maybe use a predicate instead of the is check because we want to
make
> the interface for type inference more explicit so that we are able to change
the
> internal representation and inference algorithm.

The idea is that TypeMaskSystem should be that interface (from the CPS's
viewpoint). I.e. its implementation depends on TypeMask, but its interface
should be implementable by an alternate type inference and given to the CPS.

It's not quite there yet, but that's the idea.

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:2161: } else if (uri ==
Uris.dart__native_typed_data) {
On 2015/09/15 08:17:07, karlklose wrote:
> This is already done in Compiler.onLibraryScanned (the field is called
> 'typedDataClass').

NativeTypeData is not the same as NativeTypedArray.

[sra1, 2015-09-15 15:59:32]

sra@google.com changed reviewers:
+ sra@google.com

[sra1, 2015-09-15 15:59:32]

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_mask_system.dart (right):

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_mask_system.dart:301: TypeMask
getIndexType(TypeMask type) {
Is there a nice way to write this kind of function so that it works correctly
with unions and other composite representations?

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_mask_system.dart:301: TypeMask
getIndexType(TypeMask type) {
'getIndexType' suggests to me the argument type of the indexer.

This function is not general purpose (used for GetIndex only, I assume that is
lowered only for indexable)
Perhaps rename to  elementTypeOfIndexable()

[asgerf, 2015-09-15 16:34:08]

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_mask_system.dart (right):

https://codereview.chromium.org/1340093002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_mask_system.dart:301: TypeMask
getIndexType(TypeMask type) {
On 2015/09/15 15:59:32, sra1 wrote:
> Is there a nice way to write this kind of function so that it works correctly
> with unions and other composite representations?

I don't entirely understand the use of forwarding type masks, but the union
alone is almost a one-liner so I went ahead and added it here.

It would be nice with a map() function on TypeMask for something like this,
though, if only to signal that you are actually allowed to do it outside the
type inference phase.

@second comment: I agree, renamed.

[asgerf, 2015-09-16 09:00:56]

Committed patchset #3 (id:40001) manually as
4055f6fca37dec2eb97b96fac392eeb3f06c8e82 (presubmit successful).