[Interpreter] Unary operators - typeof, void, and logical not.

[Interpreter] Unary operators - typeof, void, and logical not.

Implementations and tests for typeof, void, and logical not.

Add missing string type to Object::TypeOf.

BUG=v8:4280
LOG=NO

Committed: https://crrev.com/565f0d730b1a2cdddc7e048b229d8748d8ca9660
Cr-Commit-Position: refs/heads/master@{#31124}

[oth, 2015-10-05 10:16:39]

oth@chromium.org changed reviewers:
+ bmeurer@chromium.org, rmcilroy@chromium.org, verwaest@chromium.org

[oth, 2015-10-05 10:16:39]

verwaest@chromium.org: Please review changes in runtime.

rmcilroy@chromium.org: Please review changes in the interpreter and tests.

bmeurer@chromium.org: Please review changes in compiler and objects.cc.

Thanks!

[Benedikt Meurer, 2015-10-05 10:19:53]

LGTM from my side.

[rmcilroy, 2015-10-06 10:39:15]

lgtm with some nits.

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (left):

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.cc:461: // runtime calls. This looks like another
candidate.
Looks like you aren't rebased on master - this change is already landed.

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.cc:351: // Load accumulator with string
representating type of object in accumulator.
nit - a string representation of the type of the object in the accumulator

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-bytecode-generator.cc:1316: B(Return),             
 // 19
nit - be consistent (either bytecode numbers for all test-cases in this test or
none)

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-interpreter.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:1303: builder.LoadFalse();
Could you add a tests with a non-boolean parameter sent to LogicalNot please.

[oth, 2015-10-06 12:35:07]

Thanks, all incorporated.

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (left):

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.cc:461: // runtime calls. This looks like another
candidate.
On 2015/10/06 10:39:15, rmcilroy wrote:
> Looks like you aren't rebased on master - this change is already landed.

Acknowledged.

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.cc:351: // Load accumulator with string
representating type of object in accumulator.
On 2015/10/06 10:39:15, rmcilroy wrote:
> nit - a string representation of the type of the object in the accumulator

Done, and added the definite and indefinite articles in a few other descriptions
in this file for good measure.

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-bytecode-generator.cc:1316: B(Return),             
 // 19
On 2015/10/06 10:39:15, rmcilroy wrote:
> nit - be consistent (either bytecode numbers for all test-cases in this test
or
> none)

Done.

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-interpreter.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:1303: builder.LoadFalse();
On 2015/10/06 10:39:15, rmcilroy wrote:
> Could you add a tests with a non-boolean parameter sent to LogicalNot please.

Done.

[rmcilroy, 2015-10-06 12:48:08]

Looks great thanks! lgtm.

[Toon Verwaest, 2015-10-06 13:25:27]

runtime lgtm with some comments

https://codereview.chromium.org/1390483002/diff/40001/src/runtime/runtime-int...
File src/runtime/runtime-interpreter.cc (right):

https://codereview.chromium.org/1390483002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:125: HandleScope scope(isolate);
SealHandleScope shs(isolate); ?

https://codereview.chromium.org/1390483002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:135: CONVERT_ARG_CHECKED(Object, x, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, x, 0); avoids requiring to create the handle
below with handle(x, isolate). You could probably SealHandleScope here.

[oth, 2015-10-06 13:52:10]

Thanks all. Comments incorporated.

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-bytecode-generator.cc (right):

https://codereview.chromium.org/1390483002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-bytecode-generator.cc:1316: B(Return),             
 // 19
On 2015/10/06 10:39:15, rmcilroy wrote:
> nit - be consistent (either bytecode numbers for all test-cases in this test
or
> none)

Done.

https://codereview.chromium.org/1390483002/diff/40001/src/runtime/runtime-int...
File src/runtime/runtime-interpreter.cc (right):

https://codereview.chromium.org/1390483002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:125: HandleScope scope(isolate);
On 2015/10/06 13:25:27, Toon Verwaest wrote:
> SealHandleScope shs(isolate); ?

Done.

https://codereview.chromium.org/1390483002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:135: CONVERT_ARG_CHECKED(Object, x, 0);
On 2015/10/06 13:25:27, Toon Verwaest wrote:
> CONVERT_ARG_HANDLE_CHECKED(Object, x, 0); avoids requiring to create the
handle
> below with handle(x, isolate). You could probably SealHandleScope here.

Done.

[oth, 2015-10-06 13:52:35]

The CQ bit was checked by oth@chromium.org

[oth, 2015-10-06 13:52:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, verwaest@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1390483002/#ps60001
(title: "Incorporate review comments on patch set 3.")

[commit-bot: I haz the power, 2015-10-06 13:52:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390483002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390483002/60001

[commit-bot: I haz the power, 2015-10-06 14:15:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-06 14:15:40]

Patchset 4 (id:??) landed as
https://crrev.com/565f0d730b1a2cdddc7e048b229d8748d8ca9660
Cr-Commit-Position: refs/heads/master@{#31124}