Dont track scope_inside_with_ explicitly

Dont track scope_inside_with_ explicitly

This is only needed to set a flag on the SharedFunctionInfo generated for a function literal. Hence we only need it in 1 scope, and only read it once. Recursing in that case will be more efficient than always tracking it.

BUG=v8:5209
Committed: https://crrev.com/d814ca8d291404baf5413de4d581f5da49a0d97a
Cr-Commit-Position: refs/heads/master@{#38761}

[Toon Verwaest, 2016-08-19 14:18:18]

verwaest@chromium.org changed reviewers:
+ neis@chromium.org

[Toon Verwaest, 2016-08-19 14:18:19]

ptal

[neis, 2016-08-19 14:25:24]

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.h
File src/ast/scopes.h (right):

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.h#newcod...
src/ast/scopes.h:603: 
This will return true for any with scope. Is that intended?

[neis, 2016-08-19 14:33:33]

On 2016/08/19 14:25:24, neis wrote:
> https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.h
> File src/ast/scopes.h (right):
> 
>
https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.h#newcod...
> src/ast/scopes.h:603: 
> This will return true for any with scope. Is that intended?

From looking at the rest of the code, it is.  Maybe add a comment?

Anyway, lgtm.

[neis, 2016-08-19 14:33:41]

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:896: return !is_with_scope() &&
outer_scope_->HasTrivialContext();
Now the code doesn't quite match the comment anymore, but the comment sounds
very odd to me anyway. Could you clarify it?

[Toon Verwaest, 2016-08-19 15:58:23]

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.h
File src/ast/scopes.h (right):

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.h#newcod...
src/ast/scopes.h:603: 
On 2016/08/19 14:25:24, neis wrote:
> This will return true for any with scope. Is that intended?

Yes? That's what the flag used to do

[Toon Verwaest, 2016-08-19 18:00:35]

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2261693002/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:896: return !is_with_scope() &&
outer_scope_->HasTrivialContext();
On 2016/08/19 14:33:41, neis wrote:
> Now the code doesn't quite match the comment anymore, but the comment sounds
> very odd to me anyway. Could you clarify it?

I'll do so in a follow-up

[Toon Verwaest, 2016-08-19 18:00:39]

The CQ bit was checked by verwaest@chromium.org

[commit-bot: I haz the power, 2016-08-19 18:00:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 18:36:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-19 18:37:07]

Description was changed from

==========
Dont track scope_inside_with_ explicitly

This is only needed to set a flag on the SharedFunctionInfo generated for a
function literal. Hence we only need it in 1 scope, and only read it once.
Recursing in that case will be more efficient than always tracking it.

BUG=v8:5209
==========

to

==========
Dont track scope_inside_with_ explicitly

This is only needed to set a flag on the SharedFunctionInfo generated for a
function literal. Hence we only need it in 1 scope, and only read it once.
Recursing in that case will be more efficient than always tracking it.

BUG=v8:5209

Committed: https://crrev.com/d814ca8d291404baf5413de4d581f5da49a0d97a
Cr-Commit-Position: refs/heads/master@{#38761}
==========

[commit-bot: I haz the power, 2016-08-19 18:37:09]

Patchset 2 (id:??) landed as
https://crrev.com/d814ca8d291404baf5413de4d581f5da49a0d97a
Cr-Commit-Position: refs/heads/master@{#38761}