On 2011/10/21 13:59:00, Steven wrote:
Can you please take a look again.
http://codereview.chromium.org/8344082/diff/1/src/compiler.cc
File src/compiler.cc (right):
http://codereview.chromium.org/8344082/diff/1/src/compiler.cc#newcode602
src/compiler.cc:602: shared->set_strict_mode_flag(strict_mode);
Testing again if the parsed function literal is in strict mode. However,
this
should have been a non-issue because the update would only happen if we
didn't
know the mode before and we figured it out for the first time by parsing
in
which case the value from the function literal is correct.
On 2011/10/20 14:18:59, Yang wrote:
> This is not quite equivalent, I guess. The old version only sets if in
strict
> mode, the new version sets in any case. Also, any performance
implications?
http://codereview.chromium.org/8344082/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/8344082/diff/1/src/parser.cc#newcode655
src/parser.cc:655: top_scope_->SetStrictModeFlag(strict_mode);
Because we start with a GLOBAL or EVAL scope the previous scope is null
and
therefore no the created scope won't be in strict mode.
On 2011/10/20 14:18:59, Yang wrote:
> Here again. This only works if you assume the previous state to be not
in
strict
> mode.
http://codereview.chromium.org/8344082/diff/1/src/parser.cc#newcode742
src/parser.cc:742:
scope->SetStrictModeFlag(shared_info->strict_mode_flag());
The move before the if was not correct. I added a test case which catches
this.
Always setting the StrictModeFlag here is a non-issue, however for
non-trivial
reasons. Added ASSERTS to verify this. As far as I can see these are the
possible cases
* the SharedFunctionInfo has the correct value: this function is
non-strict
or
outer scope is strict or the function was preparsed
* the outer scope is non-strict and the non-preparsed strict function is
being
parsed for the first time. In this case ParseFunctionLiteral will
determine
the
correct mode and Compiler::CompileLazy updates the SharedFunctionInfo to
the
correct value.
On 2011/10/20 14:18:59, Yang wrote:
> Ditto.
LGTM if all tests pass.
http://codereview.chromium.org/8344082/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
