dart2js: Support new const operations on strings.

dart2js: Support new const operations on strings.

BUG= http://dartbug.com/21001
R=johnniwinther@google.com

Committed: https://code.google.com/p/dart/source/detail?r=42737

[floitsch, 2014-09-20 00:14:33]

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

[Johnni Winther, 2014-09-24 09:29:22]

lgtm

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
File
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart
(right):

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart:107:
if (result == null || result.isString) return result;
This is not needed since convertToJavaScriptConstant is the identity function on
strings.

[floitsch, 2014-09-24 12:53:19]

Note: I won't commit this CL until the spec is ready for it.

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
File
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart
(right):

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart:107:
if (result == null || result.isString) return result;
On 2014/09/24 09:29:22, Johnni Winther wrote:
> This is not needed since convertToJavaScriptConstant is the identity function
on
> strings.

True, but it is typed to take a NumConstant and return a NumConstant. I could
either change that, or have the test here.
What do you prefer?

[Johnni Winther, 2014-09-25 09:07:56]

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
File
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart
(right):

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart:107:
if (result == null || result.isString) return result;
On 2014/09/24 12:53:19, floitsch wrote:
> On 2014/09/24 09:29:22, Johnni Winther wrote:
> > This is not needed since convertToJavaScriptConstant is the identity
function
> on
> > strings.
> 
> True, but it is typed to take a NumConstant and return a NumConstant. I could
> either change that, or have the test here.
> What do you prefer?

Then I think I'd prefer to only call convertToJavaScriptConstant `if (result !=
null && result.isNum) ...`

[floitsch, 2014-09-25 12:29:05]

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
File
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart
(right):

https://codereview.chromium.org/585173002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart:107:
if (result == null || result.isString) return result;
On 2014/09/25 09:07:56, Johnni Winther wrote:
> On 2014/09/24 12:53:19, floitsch wrote:
> > On 2014/09/24 09:29:22, Johnni Winther wrote:
> > > This is not needed since convertToJavaScriptConstant is the identity
> function
> > on
> > > strings.
> > 
> > True, but it is typed to take a NumConstant and return a NumConstant. I
could
> > either change that, or have the test here.
> > What do you prefer?
> 
> Then I think I'd prefer to only call convertToJavaScriptConstant `if (result
!=
> null && result.isNum) ...`

Done.

[floitsch, 2015-01-09 11:57:05]

Reviving.
The spec has been updated.
PTAL.

[Johnni Winther, 2015-01-09 14:19:51]

Still LGTM

[floitsch, 2015-01-09 16:08:15]

Committed patchset #4 (id:60001) manually as 42737 (presubmit successful).