Canonicalize expressions that may be used as constants

Canonicalize expressions that may be used as constants

Evaluate and canonicalize the parameters to the built-in function
identical(a,b) if they are strings. This guarantees that
identical(“ab”, “a”+”b”) evaluates to true.

Note: when the parser parses “a”+”b”, it doesn’t know whether the
expression will be used in a context that may require a
constant value. The canonicalization is thus deferred until it is
known that it is required.

BUG=25024
R=rmacnak@google.com

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

[hausner, 2016-01-04 23:31:28]

Description was changed from

==========
Canonicalize expressions that may be used as constants

Evaluate and canonicalize the parameters to the built-in function
identical(a,b) if they are strings. This guarantees that
identical(“ab”, “a”+”b”) evaluates to true.

Note: when the parser parses “a”+”b”, it doesn’t know whether the
expression will be used in a context that may require a
constant value. The canonicalization is thus deferred until it is
known that it is required.

BUG=25024
==========

to

==========
Canonicalize expressions that may be used as constants

Evaluate and canonicalize the parameters to the built-in function
identical(a,b) if they are strings. This guarantees that
identical(“ab”, “a”+”b”) evaluates to true.

Note: when the parser parses “a”+”b”, it doesn’t know whether the
expression will be used in a context that may require a
constant value. The canonicalization is thus deferred until it is
known that it is required.

BUG=25024
==========

[hausner, 2016-01-04 23:31:28]

hausner@google.com changed reviewers:
+ rmacnak@google.com

[rmacnak, 2016-01-05 00:51:12]

lgtm

[hausner, 2016-01-05 16:51:55]

Description was changed from

==========
Canonicalize expressions that may be used as constants

Evaluate and canonicalize the parameters to the built-in function
identical(a,b) if they are strings. This guarantees that
identical(“ab”, “a”+”b”) evaluates to true.

Note: when the parser parses “a”+”b”, it doesn’t know whether the
expression will be used in a context that may require a
constant value. The canonicalization is thus deferred until it is
known that it is required.

BUG=25024
==========

to

==========
Canonicalize expressions that may be used as constants

Evaluate and canonicalize the parameters to the built-in function
identical(a,b) if they are strings. This guarantees that
identical(“ab”, “a”+”b”) evaluates to true.

Note: when the parser parses “a”+”b”, it doesn’t know whether the
expression will be used in a context that may require a
constant value. The canonicalization is thus deferred until it is
known that it is required.

BUG=25024
R=rmacnak@google.com

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

[hausner, 2016-01-05 16:51:57]

Committed patchset #2 (id:20001) manually as
11a101a325e62aef5f568c8536ede77a6dc2bf94 (presubmit successful).

[Lasse Reichstein Nielsen, 2016-01-05 18:55:13]

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

[Lasse Reichstein Nielsen, 2016-01-05 18:55:13]

https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
File tests/language/const_string_test.dart (right):

https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
tests/language/const_string_test.dart:50: Expect.isTrue(d);
Maybe also test without the const declarations.
An expression on the form `identical("ab", "a" + "b")` is a compile-time
constant expression in itself, and should be be the compile-time constant true.
The specification doesn't distinguish between whether it's used as a constant,
so `Expect.isTrue(identical("ab", "a" + "b"));` should also succeed.

[Lasse Reichstein Nielsen, 2016-01-05 18:56:15]

https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
File tests/language/const_string_test.dart (right):

https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
tests/language/const_string_test.dart:50: Expect.isTrue(d);
(I think it will succeed after this patch, I just want the test to cover it :)

[hausner, 2016-01-05 19:05:35]

https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
File tests/language/const_string_test.dart (right):

https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
tests/language/const_string_test.dart:50: Expect.isTrue(d);
On 2016/01/05 18:55:13, Lasse Reichstein Nielsen wrote:
> Maybe also test without the const declarations.
> An expression on the form `identical("ab", "a" + "b")` is a compile-time
> constant expression in itself, and should be be the compile-time constant
true.
> The specification doesn't distinguish between whether it's used as a constant,
> so `Expect.isTrue(identical("ab", "a" + "b"));` should also succeed.

I've talked to Gilad about this. He said that the Spec is not mandating that
"a"+"b" be treated as a constant when not used in a context that requiring a
constant. Consider:

var a = "a"+"b";
...
identical("ab", a);

The language should (and does) not require that the value in a is canonicalized,
even though it is a compile-time constant. Thus the identical() call may return
false.

[Lasse Reichstein Nielsen, 2016-01-05 19:35:03]

On 2016/01/05 19:05:35, hausner wrote:
>
https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
> File tests/language/const_string_test.dart (right):
> 
>
https://codereview.chromium.org/1558033002/diff/20001/tests/language/const_st...
> tests/language/const_string_test.dart:50: Expect.isTrue(d);
> On 2016/01/05 18:55:13, Lasse Reichstein Nielsen wrote:
> > Maybe also test without the const declarations.
> > An expression on the form `identical("ab", "a" + "b")` is a compile-time
> > constant expression in itself, and should be be the compile-time constant
> true.
> > The specification doesn't distinguish between whether it's used as a
constant,
> > so `Expect.isTrue(identical("ab", "a" + "b"));` should also succeed.
> 
> I've talked to Gilad about this. He said that the Spec is not mandating that
> "a"+"b" be treated as a constant when not used in a context that requiring a
> constant. Consider:
> 
> var a = "a"+"b";
> ...
> identical("ab", a);

That is correct.
"a" + "b" is a compile-time constant expression here, but that doesn't guarantee
canonicalization of strings (even if many people have insisted that it was
intended to :).

The expression  identical("ab", a)  here is not a compile-time constant
expression, so the spec-rules for constant identical (16.0.1) do not apply.

In the expression:
 var a;
 a = identical("ab", "a" + "b");
the "ab", "a" + "b" and identical("ab", "a" + "b") sub-expressions are
compile-time constant (16.1), and the constant identical specification say that
the result should be the compile-time constant true: "c1 and c2 are constant
strings and c1 == c2" (16.0.1).

(That skips the definition of what a "constant string" is, but I'm assuming it
is a compile-time constant expression with a string value).

> The language should (and does) not require that the value in a is
canonicalized,
> even though it is a compile-time constant. Thus the identical() call may
return
> false.

That one may, the compile-time constant call to identical may not. The leaves us
in the odd state where:
  var a = "a" + "b";
  print(identical("ab", "a" + "b") == identical("ab", a));  
may print false because compile-time constant identical calls have a
specification different from the actual runtime function.