[stubs] Add String and JSReceiver instance type predicates

[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.

Committed: https://crrev.com/f73074195e46699903cb43cf5bb63fef803ef3ee
Cr-Commit-Position: refs/heads/master@{#39978}

[jgruber, 2016-10-04 13:27:08]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 13:27:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-10-04 13:35:53]

jgruber@chromium.org changed reviewers:
+ ishell@chromium.org

[jgruber, 2016-10-04 13:35:54]

+bmeurer for logic inversions in code-stubs.cc.

https://codereview.chromium.org/2395453002/diff/1/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2395453002/diff/1/src/code-stub-assembler.cc#...
src/code-stub-assembler.cc:3107:
GotoUnless(IsStringInstanceType(key_instance_type), if_bailout);
Igor: I'm assuming the original code was bugged (since it did not jump for
FIRST_NONSTRING_TYPE itself), but please double-check.

https://codereview.chromium.org/2395453002/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/2395453002/diff/1/src/code-stubs.cc#newcode768
src/code-stubs.cc:768: assembler->IsJSReceiverInstanceType(rhs_instance_type),
This is an (oft-repeated) example of previously broken JSReceiver logic here.

[jgruber, 2016-10-04 13:36:55]

Description was changed from

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.

Drive-by-fix: Many cases of inverted logic, most notably in
code-stubs.cc.
==========

to

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.

Drive-by-fix: Many cases of inverted logic, most notably in
code-stubs.cc.
==========

[jgruber, 2016-10-04 13:36:55]

jgruber@chromium.org changed reviewers:
+ bmeurer@chromium.org

[commit-bot: I haz the power, 2016-10-04 13:52:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 13:52:45]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2016-10-05 03:41:20]

LGTM from my side.

[Igor Sheludko, 2016-10-05 08:02:28]

lgtm. the old logic was fine, please update the CL's description.

https://codereview.chromium.org/2395453002/diff/1/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2395453002/diff/1/src/code-stub-assembler.cc#...
src/code-stub-assembler.cc:3106: STATIC_ASSERT(FIRST_NAME_TYPE == FIRST_TYPE);
You can now remove this assert from here.

https://codereview.chromium.org/2395453002/diff/1/src/code-stub-assembler.cc#...
src/code-stub-assembler.cc:3107:
GotoUnless(IsStringInstanceType(key_instance_type), if_bailout);
On 2016/10/04 13:35:54, jgruber wrote:
> Igor: I'm assuming the original code was bugged (since it did not jump for
> FIRST_NONSTRING_TYPE itself), but please double-check.

FIRST_NONSTRING_TYPE is actually a SYMBOL_TYPE which was already handled above,
so the old code is ok and your change too.

https://codereview.chromium.org/2395453002/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/2395453002/diff/1/src/code-stubs.cc#newcode768
src/code-stubs.cc:768: assembler->IsJSReceiverInstanceType(rhs_instance_type),
On 2016/10/04 13:35:54, jgruber wrote:
> This is an (oft-repeated) example of previously broken JSReceiver logic here.

The old logic is also ok, it's just the order of arguments in "<=" operator
which is confusing.

[jgruber, 2016-10-05 08:39:11]

Description was changed from

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.

Drive-by-fix: Many cases of inverted logic, most notably in
code-stubs.cc.
==========

to

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.
==========

[jgruber, 2016-10-05 08:39:38]

https://codereview.chromium.org/2395453002/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/2395453002/diff/1/src/code-stubs.cc#newcode768
src/code-stubs.cc:768: assembler->IsJSReceiverInstanceType(rhs_instance_type),
On 2016/10/05 08:02:27, Igor Sheludko wrote:
> On 2016/10/04 13:35:54, jgruber wrote:
> > This is an (oft-repeated) example of previously broken JSReceiver logic
here.
> 
> The old logic is also ok, it's just the order of arguments in "<=" operator
> which is confusing.

Ah, right - that makes more sense.

[jgruber, 2016-10-05 08:39:46]

The CQ bit was checked by jgruber@chromium.org

[commit-bot: I haz the power, 2016-10-05 08:39:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 08:43:01]

Description was changed from

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.
==========

to

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.
==========

[commit-bot: I haz the power, 2016-10-05 08:43:03]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-05 08:43:22]

Description was changed from

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.
==========

to

==========
[stubs] Add String and JSReceiver instance type predicates

These improve readability of relevant code passages.

Committed: https://crrev.com/f73074195e46699903cb43cf5bb63fef803ef3ee
Cr-Commit-Position: refs/heads/master@{#39978}
==========

[commit-bot: I haz the power, 2016-10-05 08:43:23]

Patchset 1 (id:??) landed as
https://crrev.com/f73074195e46699903cb43cf5bb63fef803ef3ee
Cr-Commit-Position: refs/heads/master@{#39978}