Bill, could you have another look. I simplified the IsSpecObject implementation in the code generators. It no longer returns true on null and non-object undetectables.
http://codereview.chromium.org/2877018/diff/1/2 File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/2877018/diff/1/2#newcode4776 src/arm/codegen-arm.cc:4776: true_target()->Branch(eq); On 2010/07/15 14:22:06, William Hesse wrote:
This should be false_target, right?
I changed this method eliminating this check http://codereview.chromium.org/2877018/diff/1/2#newcode4780 src/arm/codegen-arm.cc:4780: // If this is an undetectable object return true immediately. On 2010/07/15 14:22:06, William Hesse wrote:
Check for undetectable seems wrong, since it would accept undetectable
strings.
Why do we even check this property at all?
Good point, as stated above I removed a lot of checks from this method, including this http://codereview.chromium.org/2877018/diff/1/2#newcode4788 src/arm/codegen-arm.cc:4788: __ cmp(possible_object, Operand(JS_FUNCTION_TYPE)); On 2010/07/15 14:22:06, William Hesse wrote:
We assume that JS_FUNCTION_TYPE is the last type throughout the code
generator,
so this check can be omitted.
Done. http://codereview.chromium.org/2877018/diff/1/12 File src/v8natives.js (left): http://codereview.chromium.org/2877018/diff/1/12#oldcode228 src/v8natives.js:228: if (!IS_SPEC_OBJECT_OR_NULL(V) && !IS_UNDETECTABLE(V)) return false; On 2010/07/15 14:22:06, William Hesse wrote:
Is the original line correct? Why could an undetectable non-object by
a
prototype?
That is correct, the new version (well, the corrected new version) takes this info account since IsSpecObject no longer recognizes undetectable non-objects (it simply discards if an object is undetectable or not) http://codereview.chromium.org/2877018/diff/1/12#oldcode236 src/v8natives.js:236: if (!IS_SPEC_OBJECT_OR_NULL(this)) return false; On 2010/07/15 14:22:06, William Hesse wrote:
This is all messed up.
Yes, none of these checks are neccesary, I exchanged them with a ToObject call(as specified in the spec). http://codereview.chromium.org/2877018/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
