Thanks for looking at the draft, Mads.
I changed the code to:
- pass the strict mode flag as an additional argument (changed all 6
codegens)
- eval then passes the flag back to compiler to compile in strict mode
- eval compiled code cache must take strict mode flag into consideration
also.
http://codereview.chromium.org/6286043/diff/1/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/6286043/diff/1/src/runtime.cc#newcode7647
src/runtime.cc:7647: Handle<JSFunction>
caller(JSFunction::cast(frame->function()));
On 2011/02/02 10:30:03, Mads Ager wrote:
On 2011/02/02 05:07:26, Martin Maly wrote:
> Alternative to walking the stack frames is passing additional
argument (strict
> mode flag). This works well but am not sure whether it is the right
way to do
> this. Open to suggestions.
When you are compiling the caller you know whether or not it is strict
mode so I
would pass that in as an extra argument. Iterating stack frames is
less direct
and it is also not cheap.
Done.
http://codereview.chromium.org/6286043/diff/1/src/runtime.cc#newcode7671
src/runtime.cc:7671: JavaScriptFrame* frame =
locator.FindJavaScriptFrame(0);
On 2011/02/02 10:30:03, Mads Ager wrote:
On 2011/02/02 05:07:26, Martin Maly wrote:
> Alternative to walking the stack frames is passing additional
argument (strict
> mode flag). This works well but am not sure whether it is the right
way to do
> this. Open to suggestions.
I would go for the additional argument here too.
Done.
http://codereview.chromium.org/6286043/diff/1009/src/compilation-cache.h
File src/compilation-cache.h (right):
http://codereview.chromium.org/6286043/diff/1009/src/compilation-cache.h#newcode52
src/compilation-cache.h:52: static Handle<SharedFunctionInfo>
LookupEval(Handle<String> source,
compiled pieces of eval code are cached so we must consider strict as
part of the equality comparison on the compiled snippets. Consider:
a.js:
eval("function arguments(){};");
b.js:
"use strict";
eval("function arguments(){};");
When executing both files (d8 a.js b.js) the second eval would
experience cache hit and code would never be compiled, error never
reported etc.
http://codereview.chromium.org/6286043/diff/1009/src/objects.cc
File src/objects.cc (right):
http://codereview.chromium.org/6286043/diff/1009/src/objects.cc#newcode8011
src/objects.cc:8011: hash ^= strict ? 0x8000 : 0x800;
Just a guess, there may be better (or more V8 code compliant) hash
tricks. Open to any suggestions.
http://codereview.chromium.org/6286043/diff/1009/src/objects.cc#newcode9006
src/objects.cc:9006: StringSharedKey key(src,
context->closure()->shared(), is_strict);
It seems weird that this function hashes by:
context->closure()->shared()
shouldn't we want to hash here by executing function? As far as I can
see, context->closure()->shared() is not always the executing JSFunction
(if ever).
http://codereview.chromium.org/6286043/diff/1009/src/objects.h
File src/objects.h (right):
http://codereview.chromium.org/6286043/diff/1009/src/objects.h#newcode4847
src/objects.h:4847: SharedFunctionInfo* value);
All callers passed SharedFunctionInfo so it seems better to use stronger
type. Plus, the PutEval function needs to add the strict mode-ness of
the SharedFunctionInfo into the hash key.
http://codereview.chromium.org/6286043/diff/1009/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6286043/diff/1009/src/parser.cc#newcode667
src/parser.cc:667: if (strict_mode) temp_scope.EnableStrictMode();
Restore strict mode (the eval call path)
http://codereview.chromium.org/6286043/diff/1009/src/parser.cc#newcode755
src/parser.cc:755: }
In lazy compilation, strict flag is stored on SharedFunctionInfo so here
we restore it back for the parser.
http://codereview.chromium.org/6286043/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
