ES6 symbols: Implement Symbol intrinsic and basic functionality

ES6 symbols: Implement Symbol intrinsic and basic functionality

- Add --harmony-symbols flag.
- Add Symbol constructor; allow symbols as (unreplaced) return value from constructors.
- Introduce %CreateSymbol and %_IsSymbol natives and respective instructions.
- Extend 'typeof' code generation to handle symbols.
- Extend CompareIC with a UNIQUE_NAMES state that (uniformly) handles internalized strings and symbols.
- Property lookup delegates to SymbolDelegate object for symbols, which only carries the toString method.
- Extend Object.prototype.toString to recognise symbols.

Per the current draft spec, symbols are actually pseudo objects that are frozen with a null prototype and only one property (toString). For simplicity, we do not treat them as proper objects for now, although typeof will return "object". Only property access works as if they were (frozen) objects (via the internal delegate object).

(Baseline CL: https://codereview.chromium.org/12223071/)

R=mstarzinger@chromium.org
BUG=v8:2158
Committed: http://code.google.com/p/v8/source/detail?r=13786

[Michael Starzinger, 2013-02-25 13:25:22]

This is already looking good. Just one high-level comment: There is code missing
to also recognize Symbols as "objects" while returning from an inlined
constructor call. This requires special handing in VisitReturn of the hydrogen
graph builder.

[rossberg, 2013-02-26 10:00:01]

On 2013/02/25 13:25:22, Michael Starzinger wrote:
> This is already looking good. Just one high-level comment: There is code
missing
> to also recognize Symbols as "objects" while returning from an inlined
> constructor call. This requires special handing in VisitReturn of the hydrogen
> graph builder.

Done.

[Michael Starzinger, 2013-02-26 20:41:49]

LGTM with a few nits. Just one real comment in the hydrogen.cc file. Also I just
reviewed ia32 and rubber-stamped the other architecture ports.

https://codereview.chromium.org/12296026/diff/9001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/12296026/diff/9001/src/hydrogen.cc#newcode4638
src/hydrogen.cc:4638: HHasInstanceTypeAndBranch* symbolcheck =
For the inlining in Crankshaft we only need this part of the change when
FLAG_harmony_symbols is set. Could you measure the performance impact of this
additional check? If it has an impact we should hide it behind the flag.

https://codereview.chromium.org/12296026/diff/9001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/12296026/diff/9001/src/runtime.cc#newcode682
src/runtime.cc:682: RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateSymbol) {
Let's play it safe and add "NoHandleAllocation ha;" to the top of this function.
IMHO, all runtime functions should have either that or their own HandleScope.

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
File test/mjsunit/harmony/symbols.js (right):

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
test/mjsunit/harmony/symbols.js:1: // Copyright 2012 the V8 project authors. All
rights reserved.
The copyright header should say 2013 here.

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
test/mjsunit/harmony/symbols.js:44: indirect()
Could you add a comment like "// Call once before GC to preserve type feedback."
to the end of this line which explains why indirect() is called here?

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
test/mjsunit/harmony/symbols.js:122: for (var i in symbols) {
Could you also add a assertTrue(map.has(symbols[i])) check here?

[rossberg, 2013-02-27 13:12:23]

https://codereview.chromium.org/12296026/diff/9001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/12296026/diff/9001/src/hydrogen.cc#newcode4638
src/hydrogen.cc:4638: HHasInstanceTypeAndBranch* symbolcheck =
On 2013/02/26 20:41:49, Michael Starzinger wrote:
> For the inlining in Crankshaft we only need this part of the change when
> FLAG_harmony_symbols is set. Could you measure the performance impact of this
> additional check? If it has an impact we should hide it behind the flag.

Done.

https://codereview.chromium.org/12296026/diff/9001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/12296026/diff/9001/src/runtime.cc#newcode682
src/runtime.cc:682: RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateSymbol) {
On 2013/02/26 20:41:49, Michael Starzinger wrote:
> Let's play it safe and add "NoHandleAllocation ha;" to the top of this
function.
> IMHO, all runtime functions should have either that or their own HandleScope.

Done.

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
File test/mjsunit/harmony/symbols.js (right):

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
test/mjsunit/harmony/symbols.js:1: // Copyright 2012 the V8 project authors. All
rights reserved.
On 2013/02/26 20:41:49, Michael Starzinger wrote:
> The copyright header should say 2013 here.

Done.

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
test/mjsunit/harmony/symbols.js:44: indirect()
On 2013/02/26 20:41:49, Michael Starzinger wrote:
> Could you add a comment like "// Call once before GC to preserve type
feedback."
> to the end of this line which explains why indirect() is called here?

Done.

https://codereview.chromium.org/12296026/diff/9001/test/mjsunit/harmony/symbo...
test/mjsunit/harmony/symbols.js:122: for (var i in symbols) {
On 2013/02/26 20:41:49, Michael Starzinger wrote:
> Could you also add a assertTrue(map.has(symbols[i])) check here?

Done.