Implement ?? as compile-time constant in VM.

Implement ?? as compile-time constant in VM.

Extracted from https://codereview.chromium.org/1493693002/

R=hausner@google.com

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

[Lasse Reichstein Nielsen, 2016-01-12 11:41:32]

lrn@google.com changed reviewers:
+ hausner@google.com, iposva@google.com

[hausner, 2016-01-12 20:46:48]

I haven't seen the exact Spec for this yet, but intuitively it seems right.

LGTM w/comment.

https://codereview.chromium.org/1575383002/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/1575383002/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:10506: if ((lhs->IsPotentiallyConst()) &&
(lhs->EvalConstExpr() != NULL)) {
The first test, IsPotentiallyConst(), is not needed. That check is only used in
constructor initializer expressions. It is essentially a much more permissive
version of EvalConstExpr().

[hausner, 2016-01-12 21:48:35]

Looking at this again, I think this doesn't LGTM yet. I think you need to
evaluate the ?? node and cache the result, not just the left side.

https://codereview.chromium.org/1575383002/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/1575383002/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:10506: if ((lhs->IsPotentiallyConst()) &&
(lhs->EvalConstExpr() != NULL)) {
On 2016/01/12 20:46:47, hausner wrote:
> The first test, IsPotentiallyConst(), is not needed. That check is only used
in
> constructor initializer expressions. It is essentially a much more permissive
> version of EvalConstExpr().

On second thought: you also need to check that rhs is a constant expression.

If both sides are constants, would it not make more sense to evaluate the lhs ??
rhs expression right here and cache the result, rather than the rhs value? The
return value from this branch would then be a LiteralNode with the calculated
(or cache-retrieved) value.

[Lasse Reichstein Nielsen, 2016-01-13 13:40:35]

https://codereview.chromium.org/1575383002/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/1575383002/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:10506: if ((lhs->IsPotentiallyConst()) &&
(lhs->EvalConstExpr() != NULL)) {
Done. 
I still set left_temp to final below because that is necessary to handle the
potentially constant expressions correctly (they are not both constant here, but
will eventually be when invoked with "const").

[Lasse Reichstein Nielsen, 2016-01-15 14:49:43]

Ping.

[hausner, 2016-01-21 18:00:00]

LGTM.

[Lasse Reichstein Nielsen, 2016-01-22 11:39:20]

Description was changed from

==========
Implement ?? as compile-time constant in VM.

Extracted from https://codereview.chromium.org/1493693002/
==========

to

==========
Implement ?? as compile-time constant in VM.

Extracted from https://codereview.chromium.org/1493693002/

R=hausner@google.com

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

[Lasse Reichstein Nielsen, 2016-01-22 11:39:21]

Committed patchset #3 (id:40001) manually as
693628e99e3e4c8c975345cd5937f920633dd0b3 (presubmit successful).