Fix calculation of 'scope_calls_eval' when 'eval' is within a nested catch.

Fix calculation of 'scope_calls_eval' when 'eval' is within a nested catch.

BUG=
TEST=mjsunit/scope-calls-eval.js

Committed: http://code.google.com/p/v8/source/detail?r=8756

[Steven, 2011-07-21 15:46:02]

PTAL

[Kevin Millikin (Chromium), 2011-07-28 08:53:40]

http://codereview.chromium.org/7464030/diff/1/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/7464030/diff/1/src/parser.cc#newcode2159
src/parser.cc:2159: if(catch_scope->calls_eval()) top_scope_->RecordEvalCall();
Style nit: there should be a space after the keyword if (also for, while, etc.)
to visually distinguish it from a function call.

http://codereview.chromium.org/7464030/diff/1/src/parser.cc#newcode2759
src/parser.cc:2759: top_scope_->RecordEvalCall();
I think it's more straightforward to do, instead,

top_scope_->DeclarationScope()->RecordEvalCall()

here.  That avoids recording eval call for the potential try scope and we have
only one call to Scope::RecordEvalCall exactly where needed.  What do you think?

[Steven, 2011-07-28 11:12:12]

Does it LGTY?

http://codereview.chromium.org/7464030/diff/1/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/7464030/diff/1/src/parser.cc#newcode2159
src/parser.cc:2159: if(catch_scope->calls_eval()) top_scope_->RecordEvalCall();
Removed.
On 2011/07/28 08:53:40, Kevin Millikin wrote:
> Style nit: there should be a space after the keyword if (also for, while,
etc.)
> to visually distinguish it from a function call.

http://codereview.chromium.org/7464030/diff/1/src/parser.cc#newcode2759
src/parser.cc:2759: top_scope_->RecordEvalCall();
On 2011/07/28 08:53:40, Kevin Millikin wrote:
> I think it's more straightforward to do, instead,
> 
> top_scope_->DeclarationScope()->RecordEvalCall()
> 
> here.  That avoids recording eval call for the potential try scope and we have
> only one call to Scope::RecordEvalCall exactly where needed.  What do you
think?

Done.

[Kevin Millikin (Chromium), 2011-07-28 12:06:41]

Yes, LGTM with the discussed changes.