Handle ?? in Constantifier

Handle ?? in Constantifier

R=sigmund@google.com

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

[Johnni Winther, 2017-05-05 12:49:45]

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

[Siggi Cherem (dart-lang), 2017-05-05 18:13:37]

lgtm

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/ir_element_map.dart (right):

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/ir_element_map.dart:741: left,
BinaryOperator.IF_NULL, right);
nothing to change here, but I just wonder if it is worth desugaring. Since the
subexpressions are constants with no side-effects, we can also do `left == null
? right : left`.

[Johnni Winther, 2017-05-08 09:01:06]

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/ir_element_map.dart (right):

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/ir_element_map.dart:741: left,
BinaryOperator.IF_NULL, right);
On 2017/05/05 18:13:37, Siggi Cherem (dart-lang) wrote:
> nothing to change here, but I just wonder if it is worth desugaring. Since the
> subexpressions are constants with no side-effects, we can also do `left ==
null
> ? right : left`.

I'd like the expressions to be structurally equivalence at least until we delete
the old front end.

[Johnni Winther, 2017-05-08 09:05:54]

Description was changed from

==========
Handle ?? in Constantifier
==========

to

==========
Handle ?? in Constantifier

R=sigmund@google.com

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

[Johnni Winther, 2017-05-08 09:05:58]

Committed patchset #1 (id:1) manually as
f11e4bbac64ef3449ea2cdce5b32cb945a0b5550 (presubmit successful).

[sra1, 2017-05-08 15:58:52]

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

[sra1, 2017-05-08 15:58:52]

DBC

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/ir_element_map.dart (right):

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/ir_element_map.dart:725: ir.ConditionalExpression
conditional = node.body;
The conversion here (implicit as-check in strong mode) is a bit of an
anti-pattern

It would be better to name node.body and rely on type inference:

    ir.Expression body = node.body;
    if (body is ir.ConditionalExpression) {
      ir.Expression condition = body.condition;
      if (condition is ir.MethodInvocation) {
        ...

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/ir_element_map.dart:741: left,
BinaryOperator.IF_NULL, right);
On 2017/05/08 09:01:06, Johnni Winther wrote:
> On 2017/05/05 18:13:37, Siggi Cherem (dart-lang) wrote:
> > nothing to change here, but I just wonder if it is worth desugaring. Since
the
> > subexpressions are constants with no side-effects, we can also do `left ==
> null
> > ? right : left`.
> 
> I'd like the expressions to be structurally equivalence at least until we
delete
> the old front end.

It would be nice to have a TODO that explains this so we can fix it up later.

[Johnni Winther, 2017-05-09 10:20:25]

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/ir_element_map.dart (right):

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/ir_element_map.dart:725: ir.ConditionalExpression
conditional = node.body;
On 2017/05/08 15:58:52, sra1 wrote:
> The conversion here (implicit as-check in strong mode) is a bit of an
> anti-pattern
> 
> It would be better to name node.body and rely on type inference:
> 
>     ir.Expression body = node.body;
>     if (body is ir.ConditionalExpression) {
>       ir.Expression condition = body.condition;
>       if (condition is ir.MethodInvocation) {
>         ...
> 

OK, I'll rewrite it.

https://codereview.chromium.org/2866463005/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/ir_element_map.dart:741: left,
BinaryOperator.IF_NULL, right);
On 2017/05/08 15:58:52, sra1 wrote:
> On 2017/05/08 09:01:06, Johnni Winther wrote:
> > On 2017/05/05 18:13:37, Siggi Cherem (dart-lang) wrote:
> > > nothing to change here, but I just wonder if it is worth desugaring. Since
> the
> > > subexpressions are constants with no side-effects, we can also do `left ==
> > null
> > > ? right : left`.
> > 
> > I'd like the expressions to be structurally equivalence at least until we
> delete
> > the old front end.
> 
> It would be nice to have a TODO that explains this so we can fix it up later.

Will add a TODO