Change strict mode poison pill to be the samme type error function (fixes issue 1387).

Change strict mode poison pill to be the samme type error function (fixes issue 1387).

We are now following the spec, and with regards to the error message we are following firefox (webkit still has different type errors in their nightly)
Committed: http://code.google.com/p/v8/source/detail?r=8026

[Mads Ager (chromium), 2011-05-24 07:57:44]

Redirecting. Lasse, can you have a look?

[Lasse Reichstein, 2011-05-24 09:44:10]

LGTM if the function isn't strict.

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode557
src/bootstrapper.cc:557: factory()->NewFunctionWithoutPrototype(name,
kStrictMode);
It's should not be a strict function.
If it's a strict function, it throws if you do func.arguments. It shouldn't for
builtin functions.

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode557
src/bootstrapper.cc:557: factory()->NewFunctionWithoutPrototype(name,
kStrictMode);
Does this create a function with no .prototype property? It seems to have a
.prototype property if you check in the current version.

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode560
src/bootstrapper.cc:560: Builtins::kStrictModePoisonPill));
Just use an initializer instead of repeating "Handle<Code>":
 Handle<Code> code(isolate()-> ...);

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode563
src/bootstrapper.cc:563: global_context()->strict_mode_function_map());
Still not a strict function.

http://codereview.chromium.org/7067017/diff/1/src/messages.js
File src/messages.js (right):

http://codereview.chromium.org/7067017/diff/1/src/messages.js#newcode238
src/messages.js:238: strict_poison_pill:           ["'caller', 'callee', and
'arguments' properties may not be accessed on strict mode functions or the
arguments objects for calls to them"],
Imprecise description.
You can't access "arguments" or "caller" on function objects, and not "caller"
or "callee" on arguments objects.

http://codereview.chromium.org/7067017/diff/1/test/mjsunit/regress/regress-13...
File test/mjsunit/regress/regress-1387.js (right):

http://codereview.chromium.org/7067017/diff/1/test/mjsunit/regress/regress-13...
test/mjsunit/regress/regress-1387.js:38: assertEquals(get1, get2);
Also read get3 and get4 from foo.caller and foo.arguments, and see that they are
all the same.

[Rico, 2011-05-24 10:46:07]

Comments addressed

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode557
src/bootstrapper.cc:557: factory()->NewFunctionWithoutPrototype(name,
kStrictMode);
On 2011/05/24 09:44:10, Lasse Reichstein wrote:
> It's should not be a strict function.
> If it's a strict function, it throws if you do func.arguments. It shouldn't
for
> builtin functions.
Done

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode557
src/bootstrapper.cc:557: factory()->NewFunctionWithoutPrototype(name,
kStrictMode);
On 2011/05/24 09:44:10, Lasse Reichstein wrote:
> Does this create a function with no .prototype property? It seems to have a
> .prototype property if you check in the current version.
It creates an empty function with a function without a prototype as prototype
(you might have to read that line a few times)

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode560
src/bootstrapper.cc:560: Builtins::kStrictModePoisonPill));
On 2011/05/24 09:44:10, Lasse Reichstein wrote:
> Just use an initializer instead of repeating "Handle<Code>":
>  Handle<Code> code(isolate()-> ...);

Done.

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode563
src/bootstrapper.cc:563: global_context()->strict_mode_function_map());
On 2011/05/24 09:44:10, Lasse Reichstein wrote:
> Still not a strict function.

Done.

[Lasse Reichstein, 2011-05-24 11:00:19]

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

http://codereview.chromium.org/7067017/diff/1/src/bootstrapper.cc#newcode557
src/bootstrapper.cc:557: factory()->NewFunctionWithoutPrototype(name,
kStrictMode);
That *sounds* incorrect. It should have the standard built-in Function prototype
as [[Prototype]], and it does when I check it in the current version:
  Object.GetOwnPropertyDescriptor(foo, "arguments").get.__proto__ ==
Function.prototype // => true.
where foo is a strict function.

(But I misread, it should have a .prototype property).

I.e., I think it's correct.