dart2js cps: Clean up and avoid processing unreachable code.

dart2js cps: Clean up and avoid processing unreachable code.

Primitives now have a getter 'hasValue' to distinguish primitives that
don't return a value, such as SetField.

The interceptor for TypeTestViaFlag is now inserted on-demand.

Non-instantiated intercepted classes are removed the set of
intercepted classes. This avoids an assertion failure, although it
would be better to avoid adding them in the first place.

The type of closure fields for boxes and type variables no longer go
through the global type inference.

A test fails because the type inference infers an empty type for a
closure field for a torn-off `.call` method. It's an existing problem
that we now have at least one failing test for.

CreateFunction has been removed from CPS and Tree.

BUG=
R=sra@google.com

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

[asgerf, 2015-11-24 15:40:10]

asgerf@google.com changed reviewers:
+ kmillikin@google.com, sra@google.com

[asgerf, 2015-11-27 17:21:29]

On 2015/11/24 15:40:10, asgerf wrote:

Friendly ping.

[sra1, 2015-11-27 19:16:45]

lgtm

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:684: // If one of the
parameters have no value, the function is unreachable.
one ... has

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:687: // TODO(asgerf): Can we
avoid emitting the function is the first place?
With trust-type-annotations we can have an apparently reachable call that has
the wrong type arguments. In checked mode the function would fail on entry with
a type error on the argument tests. If the program might be 'correct' and never
throw due to a correlation between state and type, e.g. CircleShape has a kind
field that is always ShapeKind.CIRCLE and the enum is used to dispatch (Box2D
does this).

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:690: replaceSubtree(root.body,
new Unreachable());
Since we have known type inference bugs, I'd rather we generate a throw.
There is an unreachable code error for this purpose (used by SSA under a flag),
but throwing a string "unreachable" is OK.

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:2444: if
(!hasParameterWithoutValue) { // Don't analyze unreachable code.
Maybe this handling should be a tiny pass that runs very early that rewrites the
body to empty or a throw.  The inliner would see the rewritten code and move the
tiny body up the call graph, removing references to the unreachable function.

[asgerf, 2015-11-30 14:27:20]

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:684: // If one of the
parameters have no value, the function is unreachable.
On 2015/11/27 19:16:45, sra1 wrote:
> one ... has

Done.

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:687: // TODO(asgerf): Can we
avoid emitting the function is the first place?
On 2015/11/27 19:16:45, sra1 wrote:
> With trust-type-annotations we can have an apparently reachable call that has
> the wrong type arguments. In checked mode the function would fail on entry
with
> a type error on the argument tests. If the program might be 'correct' and
never
> throw due to a correlation between state and type, e.g. CircleShape has a kind
> field that is always ShapeKind.CIRCLE and the enum is used to dispatch (Box2D
> does this).

So was this a yes or a no? Should I remove the TODO?

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:690: replaceSubtree(root.body,
new Unreachable());
On 2015/11/27 19:16:45, sra1 wrote:
> Since we have known type inference bugs, I'd rather we generate a throw.
> There is an unreachable code error for this purpose (used by SSA under a
flag),
> but throwing a string "unreachable" is OK.
> 

Done.

https://codereview.chromium.org/1474713002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:2444: if
(!hasParameterWithoutValue) { // Don't analyze unreachable code.
On 2015/11/27 19:16:45, sra1 wrote:
> Maybe this handling should be a tiny pass that runs very early that rewrites
the
> body to empty or a throw.  The inliner would see the rewritten code and move
the
> tiny body up the call graph, removing references to the unreachable function.

Acknowledged.

[asgerf, 2015-11-30 14:28:49]

Description was changed from

==========
dart2js cps: Clean up and avoid processing unreachable code.

Primitives now have a getter 'hasValue' to distinguish primitives that
don't return a value, such as SetField.

The interceptor for TypeTestViaFlag is now inserted on-demand.

Non-instantiated intercepted classes are removed the set of
intercepted classes. This avoids an assertion failure, although it
would be better to avoid adding them in the first place.

The type of closure fields for boxes and type variables no longer go
through the global type inference.

A test fails because the type inference infers an empty type for a
closure field for a torn-off `.call` method. It's an existing problem
that we now have at least one failing test for.

CreateFunction has been removed from CPS and Tree.

BUG=
==========

to

==========
dart2js cps: Clean up and avoid processing unreachable code.

Primitives now have a getter 'hasValue' to distinguish primitives that
don't return a value, such as SetField.

The interceptor for TypeTestViaFlag is now inserted on-demand.

Non-instantiated intercepted classes are removed the set of
intercepted classes. This avoids an assertion failure, although it
would be better to avoid adding them in the first place.

The type of closure fields for boxes and type variables no longer go
through the global type inference.

A test fails because the type inference infers an empty type for a
closure field for a torn-off `.call` method. It's an existing problem
that we now have at least one failing test for.

CreateFunction has been removed from CPS and Tree.

BUG=
R=sra@google.com

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

[asgerf, 2015-11-30 14:28:51]

Committed patchset #5 (id:80001) manually as
f77a6815c6a6776480a9d5ab765c26dc0db32501 (presubmit successful).