PreParsing inner funcs: make scope analysis more complete.

PreParsing inner funcs: make scope analysis more complete.

This adds tracking the following:
- Declarations created by catch (potentially destructuring)
- Declarations created by for-each (potentially destructuring)
- Class declarations

BUG=v8:5501, v8:5516
Review-Url: https://codereview.chromium.org/2617923003
Cr-Commit-Position: refs/heads/master@{#42129}
Committed: https://chromium.googlesource.com/v8/v8/+/d17558bc491eed6bbba71da5b05851935a6773a1

[marja, 2017-01-06 15:44:29]

marja@chromium.org changed reviewers:
+ adamk@chromium.org

[marja, 2017-01-06 15:44:30]

adamk, ptal

I'll be online later today if you'd like to discuss this.

https://codereview.chromium.org/2617923003/diff/1/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/2617923003/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:1894:
DCHECK(for_info->parsing_result.declarations.length() == 1);
Unrelated to this CL, but makes sense to add this.

https://codereview.chromium.org/2617923003/diff/1/src/parsing/preparser.h
File src/parsing/preparser.h (right):

https://codereview.chromium.org/2617923003/diff/1/src/parsing/preparser.h#new...
src/parsing/preparser.h:1103: DCHECK_NULL(names);
This DCHECK is also unrelated, but I'm adding it since I'm touching this code
anyway.

[adamk, 2017-01-06 19:05:04]

lgtm

Can you add more to the CL description saying which things improve with this
patch?

https://codereview.chromium.org/2617923003/diff/1/src/parsing/preparser.h
File src/parsing/preparser.h (right):

https://codereview.chromium.org/2617923003/diff/1/src/parsing/preparser.h#new...
src/parsing/preparser.h:1056: // Unlike in the parser, we need to declare the
catch variable as LET
High-level question...would it make sense, once this work is all done, to use
something other than the current Scope classes to do these sorts of
declarations? It scares me a little bit that that code deals with two
increasingly divergent sets of semantics, half of them based on ECMAScript and
the other half based on heuristics.

Not something to deal with in this patch (which seems safe enough), but comments
like this make me think of it.

[marja, 2017-01-09 08:27:58]

Description was changed from

==========
PreParsing inner funcs: make scope analysis more complete.

BUG=v8:5501, v8:5516
==========

to

==========
PreParsing inner funcs: make scope analysis more complete.

This adds tracking the following:
- Declarations created by catch (potentially destructuring)
- Declarations created by for-each (potentially destructuring)
- Class declarations

BUG=v8:5501, v8:5516
==========

[marja, 2017-01-09 08:29:30]

Thanks for reviewing!

Yeah, the catch variable handling is a bit odd all over - it's weird that a
catch scope is not a declaration scope but still it contains the var
declarations for the catch variable. And to be able to add it we don't do the
DeclareVariable (which can't add variable declarations in non-declaration
scopes), but bypass it. I don't have any immediate ideas about how to solve it
but I'll see if I can figure out something in the future...

[marja, 2017-01-09 08:56:48]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2017-01-09 08:57:00]

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, 2017-01-09 09:41:55]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1483952208217500, "parent_rev":
"9e65ecd957e62a1477d21742c61014fd0421521f", "commit_rev":
"d17558bc491eed6bbba71da5b05851935a6773a1"}

[commit-bot: I haz the power, 2017-01-09 09:41:59]

Description was changed from

==========
PreParsing inner funcs: make scope analysis more complete.

This adds tracking the following:
- Declarations created by catch (potentially destructuring)
- Declarations created by for-each (potentially destructuring)
- Class declarations

BUG=v8:5501, v8:5516
==========

to

==========
PreParsing inner funcs: make scope analysis more complete.

This adds tracking the following:
- Declarations created by catch (potentially destructuring)
- Declarations created by for-each (potentially destructuring)
- Class declarations

BUG=v8:5501, v8:5516

Review-Url: https://codereview.chromium.org/2617923003
Cr-Commit-Position: refs/heads/master@{#42129}
Committed:
https://chromium.googlesource.com/v8/v8/+/d17558bc491eed6bbba71da5b05851935a6...
==========

[commit-bot: I haz the power, 2017-01-09 09:41:59]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/v8/v8/+/d17558bc491eed6bbba71da5b05851935a6...