Implement set trap for proxies, and revamp class hierarchy in preparation

Implement set trap for proxies, and revamp class hierarchy in preparation:

- Introduce a class JSReceiver, that is a common superclass of JSObject and
  JSProxy. Use JSReceiver where appropriate (probably lots of places that we
  still have to migrate, but we will find those later with proxy test suite).

- Move appropriate methods to JSReceiver class (SetProperty,
  GetPropertyAttribute, Get/SetPrototype, Lookup, and so on).

- Introduce new JSFunctionProxy subclass of JSProxy. Currently only a stub.

- Overhaul enum InstanceType:
  * Introduce FIRST/LAST_SPEC_OBJECT_TYPE that ranges over all types that
    represent JS objects, and use that consistently to check language types.
  * Rename FIRST/LAST_JS_OBJECT_TYPE and FIRST/LAST_FUNCTION_CLASS_TYPE
    to FIRST/LAST_[NON]CALLABLE_SPEC_OBJECT_TYPE for clarity.
  * Eliminate the overlap over JS_REGEXP_TYPE.
  * Also replace FIRST_JS_OBJECT with FIRST_JS_RECEIVER, but only use it where
    we exclusively talk about the internal representation type.
  * Insert JS_PROXY and JS_FUNCTION_PROXY in the appropriate places.

- Fix all checks concerning classification, especially for functions, to
  use the CALLABLE_SPEC_OBJECT range (that includes funciton proxies).

- Handle proxies in SetProperty (that was the easiest part :) ).

- A few simple test cases.

R=kmillikin@chromium.org

[Kevin Millikin (Chromium), 2011-05-26 09:31:08]

The set trap looks good to me.  I've solicited some more feedback on the name
changes to make sure we get them approximately right, so let's wait a short bit
before committing.

http://codereview.chromium.org/6992072/diff/1/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/6992072/diff/1/include/v8.h#newcode3706
include/v8.h:3706: static const int kJSObjectType = 0xa1;
I love this.

http://codereview.chromium.org/6992072/diff/1/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/6992072/diff/1/src/arm/builtins-arm.cc#newcode951
src/arm/builtins-arm.cc:951: // FIRST_OBJECT_OR_FUNCTION_CLASS_TYPE, it is not
an object in the ECMA sense.
Maybe the terminology is still not quite right.  This catches lots of Objects
that don't have [[Class]] "Object" or "Function" (e.g., "Arguments", "Array",
"Number", etc.).

All of those things do have type Object.

http://codereview.chromium.org/6992072/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1989: ASSERT(LAST_TYPE ==
LAST_FUNCTION_CLASS_TYPE);
Do you think that all these asserts can be STATIC_ASSERT?

[Kevin Millikin (Chromium), 2011-05-26 13:30:10]

A quick round of comments.

http://codereview.chromium.org/6992072/diff/4002/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:953: __ b(ge, &exit);
Not your code, but these comparisons should all be unsigned ones (hi = higher,
lo = lower, hs = higher or same, ls = lower or same).

We can fix them all as a separate change (but this CL nicely highlights a lot of
the places).

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:2440: void
FullCodeGenerator::EmitIsObject(ZoneList<Expression*>* args) {
We should probably name the corresponding runtime function to %_IsSpecObject (as
a separate change).

http://codereview.chromium.org/6992072/diff/4002/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/6992072/diff/4002/src/objects.h#newcode294
src/objects.h:294: V(JS_PROXY_TYPE)                                             
               \
These aren't required by the implementation to be in the same order as in the
enum, but it might be helpful for the reader.

http://codereview.chromium.org/6992072/diff/4002/src/objects.h#newcode590
src/objects.h:590: FIRST_NONCALLABLE_SPEC_OBJECT_TYPE = JS_VALUE_TYPE,
That's not really pithy :(.  How about FIRST_NONCALLABLE_OBJECT_TYPE without the
SPEC?

http://codereview.chromium.org/6992072/diff/4002/src/objects.h#newcode1706
src/objects.h:1706: //  void Lookup(String* name, LookupResult* result);
Can this be removed?

[Kevin Millikin (Chromium), 2011-05-30 16:32:29]

http://codereview.chromium.org/6992072/diff/4002/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:953: __ b(ge, &exit);
On 2011/05/26 13:30:10, Kevin Millikin wrote:
> Not your code, but these comparisons should all be unsigned ones (hi = higher,
> lo = lower, hs = higher or same, ls = lower or same).
> 
> We can fix them all as a separate change (but this CL nicely highlights a lot
of
> the places).

Scratch that.  The byte is zero extended and it's OK.  We should be consistent
though (we have a mix of ge/hs for this).

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:896: __ b(hs, &done_convert);
See, here is 'hs'.

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:2779: __ b(hs, &function);
This should possibly be "hs" --> "ge".

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4083: __ b(lo, if_false);
Likewise, "lo" --> "lt" here and "hi" --> "gt" just below.  It's not consistent
with the other cases :(

http://codereview.chromium.org/6992072/diff/4002/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:2711: DeoptimizeIf(lo, instr->environment());
"lo" --> "lt"

http://codereview.chromium.org/6992072/diff/4002/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:4363: __ b(lo, false_label);
"lo" --> "lt", "hi" --> "gt" just below.

http://codereview.chromium.org/6992072/diff/4002/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:1884: __ j(greater_equal, is_true);
This needs to be above_equal (also just below).

http://codereview.chromium.org/6992072/diff/4002/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/ic.cc#newcode1389
src/ic.cc:1389: HandleScope scope(isolate());
The handle scope should not be needed here.

http://codereview.chromium.org/6992072/diff/4002/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/objects.cc#newcode144
src/objects.cc:144: return result->HandlerResult();
Indentation looks off.

http://codereview.chromium.org/6992072/diff/4002/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:1896: __ j(greater_equal, is_true);
Here: above_equal, and also just below.

http://codereview.chromium.org/6992072/diff/4002/test/harmony/proxies.js
File test/harmony/proxies.js (right):

http://codereview.chromium.org/6992072/diff/4002/test/harmony/proxies.js#newc...
test/harmony/proxies.js:1: 
This needs the license header.

I'd move it into a new subdirectory test/mjsunit/harmony/ and use the mjsunit.js
utilities where it makes sense.

[rossberg, 2011-05-31 14:50:24]

http://codereview.chromium.org/6992072/diff/1/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/6992072/diff/1/src/arm/builtins-arm.cc#newcode951
src/arm/builtins-arm.cc:951: // FIRST_OBJECT_OR_FUNCTION_CLASS_TYPE, it is not
an object in the ECMA sense.
On 2011/05/26 09:31:08, Kevin Millikin wrote:
> Maybe the terminology is still not quite right.  This catches lots of Objects
> that don't have [[Class]] "Object" or "Function" (e.g., "Arguments", "Array",
> "Number", etc.).
> 
> All of those things do have type Object.

Changed according to your suggestion on the list.

http://codereview.chromium.org/6992072/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1989: ASSERT(LAST_TYPE ==
LAST_FUNCTION_CLASS_TYPE);
On 2011/05/26 09:31:08, Kevin Millikin wrote:
> Do you think that all these asserts can be STATIC_ASSERT?

Done (here and elsewhere).

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:896: __ b(hs, &done_convert);
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> See, here is 'hs'.

Changed to ge.

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:2440: void
FullCodeGenerator::EmitIsObject(ZoneList<Expression*>* args) {
On 2011/05/26 13:30:10, Kevin Millikin wrote:
> We should probably name the corresponding runtime function to %_IsSpecObject
(as
> a separate change).

Hm, SpecObject includes functions, so that would probably give the wrong idea.
%_IsNoncallableSpecObject then? :)

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:2779: __ b(hs, &function);
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> This should possibly be "hs" --> "ge".

Done.

http://codereview.chromium.org/6992072/diff/4002/src/arm/full-codegen-arm.cc#...
src/arm/full-codegen-arm.cc:4083: __ b(lo, if_false);
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> Likewise, "lo" --> "lt" here and "hi" --> "gt" just below.  It's not
consistent
> with the other cases :(

Done.

http://codereview.chromium.org/6992072/diff/4002/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:2711: DeoptimizeIf(lo, instr->environment());
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> "lo" --> "lt"

Done.

http://codereview.chromium.org/6992072/diff/4002/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:4363: __ b(lo, false_label);
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> "lo" --> "lt", "hi" --> "gt" just below.

Done.

http://codereview.chromium.org/6992072/diff/4002/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:1884: __ j(greater_equal, is_true);
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> This needs to be above_equal (also just below).

Done.

http://codereview.chromium.org/6992072/diff/4002/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/ic.cc#newcode1389
src/ic.cc:1389: HandleScope scope(isolate());
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> The handle scope should not be needed here.

Done.

http://codereview.chromium.org/6992072/diff/4002/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/objects.cc#newcode144
src/objects.cc:144: return result->HandlerResult();
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> Indentation looks off.

Done.

http://codereview.chromium.org/6992072/diff/4002/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/6992072/diff/4002/src/objects.h#newcode294
src/objects.h:294: V(JS_PROXY_TYPE)                                             
               \
On 2011/05/26 13:30:10, Kevin Millikin wrote:
> These aren't required by the implementation to be in the same order as in the
> enum, but it might be helpful for the reader.

Done.

http://codereview.chromium.org/6992072/diff/4002/src/objects.h#newcode590
src/objects.h:590: FIRST_NONCALLABLE_SPEC_OBJECT_TYPE = JS_VALUE_TYPE,
On 2011/05/26 13:30:10, Kevin Millikin wrote:
> That's not really pithy :(.  How about FIRST_NONCALLABLE_OBJECT_TYPE without
the
> SPEC?

Hm, with neither SPEC nor JS, wouldn't that name logically refer to FIRST_TYPE?
(All the types here are "object" types in the V8 sense.)

With the current name, it is immediately clear that it is a subcategory of
SPEC_OBJECT_TYPEs. (Fortunately, we don't use it that often.)

http://codereview.chromium.org/6992072/diff/4002/src/objects.h#newcode1706
src/objects.h:1706: //  void Lookup(String* name, LookupResult* result);
On 2011/05/26 13:30:10, Kevin Millikin wrote:
> Can this be removed?

Oops.

http://codereview.chromium.org/6992072/diff/4002/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

http://codereview.chromium.org/6992072/diff/4002/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:1896: __ j(greater_equal, is_true);
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> Here: above_equal, and also just below.

Done.

http://codereview.chromium.org/6992072/diff/4002/test/harmony/proxies.js
File test/harmony/proxies.js (right):

http://codereview.chromium.org/6992072/diff/4002/test/harmony/proxies.js#newc...
test/harmony/proxies.js:1: 
On 2011/05/30 16:32:29, Kevin Millikin wrote:
> This needs the license header.
> 
> I'd move it into a new subdirectory test/mjsunit/harmony/ and use the
mjsunit.js
> utilities where it makes sense.

Done.