[modules] Explicitly keep track of module requests.

[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while
parsing (in ModuleDescriptor). Descriptor entries now refer to that
position rather than the string.  When generating the ModuleInfo, turn
this map into an array of specifiers. We don't need the map anymore
later on, so we do not reconstruct it when deserializing again.

BUG=v8:1569
Committed: https://crrev.com/c5785bfb92c0380b93c4ec57dc2326da90cf0390
Cr-Commit-Position: refs/heads/master@{#39519}

[neis, 2016-09-19 19:18:47]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

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

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

[neis, 2016-09-19 19:19:25]

Description was changed from

==========
[modules] Explicitly keep track of module requests.

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

to

==========
[modules] Explicitly keep track of module requests.

BUG=v8:1569
==========

[neis, 2016-09-19 19:19:25]

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

[neis, 2016-09-19 19:23:46]

Description was changed from

==========
[modules] Explicitly keep track of module requests.

BUG=v8:1569
==========

to

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while parsing (in
ModuleDescriptor). When generating the ModuleInfo, turn this map into an
array of specifiers.

BUG=v8:1569
==========

[commit-bot: I haz the power, 2016-09-19 19:24:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 19:24:28]

Dry run: Try jobs failed on following builders:
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)

[neis, 2016-09-19 19:40:24]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 19:40:27]

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

[neis, 2016-09-19 19:43:53]

Description was changed from

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while parsing (in
ModuleDescriptor). When generating the ModuleInfo, turn this map into an
array of specifiers.

BUG=v8:1569
==========

to

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while
parsing (in ModuleDescriptor). Descriptor entries now refer to that
position rather than the string.  When generating the ModuleInfo, turn
this map into an array of specifiers. We don't need the map anymore
later on, so we do not reconstruct it when deserializing again.

BUG=v8:1569
==========

[neis, 2016-09-19 19:49:34]

ptal

[adamk, 2016-09-19 19:57:21]

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.cc
File src/ast/modules.cc (right):

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.cc#newc...
src/ast/modules.cc:91: CHECK(Smi::IsValid(module_request));
Add a TODO to handle this earlier?

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.h
File src/ast/modules.h (right):

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.h#newco...
src/ast/modules.h:90: module_request(-1) {}
Would be good to document somewhere that "-1" means "uninitialized" while "0"
means "this module". Maybe these could be constants?

https://codereview.chromium.org/2353633002/diff/40001/src/ast/scopeinfo.cc
File src/ast/scopeinfo.cc (right):

https://codereview.chromium.org/2353633002/diff/40001/src/ast/scopeinfo.cc#ne...
src/ast/scopeinfo.cc:860: module_requests->set(static_cast<int>(elem.second),
*elem.first->string());
You shouldn't need a static_cast here, isn't elem.second already an int?

[neis, 2016-09-19 21:13:47]

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.cc
File src/ast/modules.cc (right):

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.cc#newc...
src/ast/modules.cc:91: CHECK(Smi::IsValid(module_request));
On 2016/09/19 19:57:20, adamk wrote:
> Add a TODO to handle this earlier?

Done.

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.h
File src/ast/modules.h (right):

https://codereview.chromium.org/2353633002/diff/40001/src/ast/modules.h#newco...
src/ast/modules.h:90: module_request(-1) {}
On 2016/09/19 19:57:20, adamk wrote:
> Would be good to document somewhere that "-1" means "uninitialized" while "0"
> means "this module". Maybe these could be constants?

I added a comment.  Negative means no module request; non-negative is the
position and used as an index. 0 is not special.

https://codereview.chromium.org/2353633002/diff/40001/src/ast/scopeinfo.cc
File src/ast/scopeinfo.cc (right):

https://codereview.chromium.org/2353633002/diff/40001/src/ast/scopeinfo.cc#ne...
src/ast/scopeinfo.cc:860: module_requests->set(static_cast<int>(elem.second),
*elem.first->string());
On 2016/09/19 19:57:20, adamk wrote:
> You shouldn't need a static_cast here, isn't elem.second already an int?

Yes, forgot to update.

[adamk, 2016-09-19 21:36:16]

lgtm

[neis, 2016-09-19 21:38:43]

The CQ bit was checked by neis@chromium.org

[commit-bot: I haz the power, 2016-09-19 21:38:47]

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-09-19 22:06:51]

Description was changed from

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while
parsing (in ModuleDescriptor). Descriptor entries now refer to that
position rather than the string.  When generating the ModuleInfo, turn
this map into an array of specifiers. We don't need the map anymore
later on, so we do not reconstruct it when deserializing again.

BUG=v8:1569
==========

to

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while
parsing (in ModuleDescriptor). Descriptor entries now refer to that
position rather than the string.  When generating the ModuleInfo, turn
this map into an array of specifiers. We don't need the map anymore
later on, so we do not reconstruct it when deserializing again.

BUG=v8:1569
==========

[commit-bot: I haz the power, 2016-09-19 22:06:52]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-19 22:08:30]

Description was changed from

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while
parsing (in ModuleDescriptor). Descriptor entries now refer to that
position rather than the string.  When generating the ModuleInfo, turn
this map into an array of specifiers. We don't need the map anymore
later on, so we do not reconstruct it when deserializing again.

BUG=v8:1569
==========

to

==========
[modules] Explicitly keep track of module requests.

We must keep track of the exact order in which modules are requested.
To do so, maintain a map from module specifiers to position while
parsing (in ModuleDescriptor). Descriptor entries now refer to that
position rather than the string.  When generating the ModuleInfo, turn
this map into an array of specifiers. We don't need the map anymore
later on, so we do not reconstruct it when deserializing again.

BUG=v8:1569

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

[commit-bot: I haz the power, 2016-09-19 22:08:31]

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