Fix CollectNonLocals

Fix CollectNonLocals

Now it actually collects free variables of the target function, rather than any reference to a non-stack-allocated variable in any of the inner scopes.

BUG=

Committed: https://crrev.com/58524d6df343911c2ea1f3793718cb012f7e813c
Cr-Commit-Position: refs/heads/master@{#38534}

[Toon Verwaest, 2016-08-10 09:52:44]

verwaest@chromium.org changed reviewers:
+ marja@chromium.org, yangguo@chromium.org

[Toon Verwaest, 2016-08-10 09:52:44]

ptal
will add a test

[marja, 2016-08-10 09:56:53]

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

https://codereview.chromium.org/2229373002/diff/1/src/ast/scopes.h#newcode590
src/ast/scopes.h:590: // If a ParseInfo* is passed, non-free variables will be
resolved.
Can this be done without mutating - I'd need the scopes be sensemaking even
after the free variables has been found. It's also somewhat obscure that
retrieving information from a scope makes it unusable.

[Yang, 2016-08-10 10:34:39]

On 2016/08/10 09:56:53, marja wrote:
> https://codereview.chromium.org/2229373002/diff/1/src/ast/scopes.h
> File src/ast/scopes.h (right):
> 
> https://codereview.chromium.org/2229373002/diff/1/src/ast/scopes.h#newcode590
> src/ast/scopes.h:590: // If a ParseInfo* is passed, non-free variables will be
> resolved.
> Can this be done without mutating - I'd need the scopes be sensemaking even
> after the free variables has been found. It's also somewhat obscure that
> retrieving information from a scope makes it unusable.

src/debug lgtm.

[Toon Verwaest, 2016-08-10 10:52:24]

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

https://codereview.chromium.org/2229373002/diff/1/src/ast/scopes.h#newcode590
src/ast/scopes.h:590: // If a ParseInfo* is passed, non-free variables will be
resolved.
On 2016/08/10 09:56:53, marja wrote:
> Can this be done without mutating - I'd need the scopes be sensemaking even
> after the free variables has been found. It's also somewhat obscure that
> retrieving information from a scope makes it unusable.

As discussed offline, I think you just want to pass in ParseInfo* if you want to
use the scope afterwards. That will partially resolve the scope, and the free
variables returned here should just become unresolved_.

[marja, 2016-08-10 11:10:20]

lgtm (w/ the dcheck_nes we discussed offline)

[Toon Verwaest, 2016-08-10 11:25:28]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-08-10 11:25:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
yangguo@chromium.org, marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2229373002/#ps40001
(title: "Address offline comment")

[commit-bot: I haz the power, 2016-08-10 11:25:35]

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-10 11:49:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-10 11:49:48]

Description was changed from

==========
Fix CollectNonLocals

Now it actually collects free variables of the target function, rather than any
reference to a non-stack-allocated variable in any of the inner scopes.

BUG=
==========

to

==========
Fix CollectNonLocals

Now it actually collects free variables of the target function, rather than any
reference to a non-stack-allocated variable in any of the inner scopes.

BUG=

Committed: https://crrev.com/58524d6df343911c2ea1f3793718cb012f7e813c
Cr-Commit-Position: refs/heads/master@{#38534}
==========

[commit-bot: I haz the power, 2016-08-10 11:49:50]

Patchset 3 (id:??) landed as
https://crrev.com/58524d6df343911c2ea1f3793718cb012f7e813c
Cr-Commit-Position: refs/heads/master@{#38534}