[modules] Make handling of module info's regular exports more robust.

[modules] Make handling of module info's regular exports more robust.

R=adamk@chromium.org
BUG=v8:1569
Committed: https://crrev.com/9d6c6e5a666af9464935cb1b227186f480ecd264
Cr-Commit-Position: refs/heads/master@{#40795}

[adamk, 2016-11-03 18:20:52]

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

https://codereview.chromium.org/2473993002/diff/1/src/ast/modules.cc#newcode135
src/ast/modules.cc:135: data[index +
ModuleInfo::kRegularExportExportNamesOffset] = export_names;
If you don't like the look of this, you could keep using reserve() and push_back
but add STATIC_ASSERTs about the ordering of these things. Not sure that would
actually look any better (and I'm OK with this approach), but it's an option.

https://codereview.chromium.org/2473993002/diff/1/src/ast/modules.cc#newcode149
src/ast/modules.cc:149: data.resize(index);
When would index be less then data.size()? I don't see a break in the loop
anywhere.

[neis, 2016-11-04 08:30:29]

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

https://codereview.chromium.org/2473993002/diff/1/src/ast/modules.cc#newcode135
src/ast/modules.cc:135: data[index +
ModuleInfo::kRegularExportExportNamesOffset] = export_names;
On 2016/11/03 18:20:52, adamk wrote:
> If you don't like the look of this, you could keep using reserve() and
push_back
> but add STATIC_ASSERTs about the ordering of these things. Not sure that would
> actually look any better (and I'm OK with this approach), but it's an option.

I prefer the current way.

https://codereview.chromium.org/2473993002/diff/1/src/ast/modules.cc#newcode149
src/ast/modules.cc:149: data.resize(index);
On 2016/11/03 18:20:52, adamk wrote:
> When would index be less then data.size()? I don't see a break in the loop
> anywhere.

It's less when there are multiple exports of the same local name.  That's why we
can't create the FixedArray in advance.  (There's no break but 'it' may get
advanced several times in one for-iteration.)

[adamk, 2016-11-04 17:15:29]

Thanks for the explanation, lgtm

[adamk, 2016-11-04 17:22:37]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2016-11-04 17:22:46]

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

[adamk, 2016-11-04 17:22:56]

The CQ bit was unchecked by adamk@chromium.org

[adamk, 2016-11-04 17:23:12]

Oops, unsetting cq bit, might want to change that CL description before landing.

[neis, 2016-11-07 09:21:03]

Description was changed from

==========
[modules] Make handling of module info's regular exports more robust.

(First attempt, let me know what you think. I'm not too happy with it.)

R=adamk@chromium.org
BUG=v8:1569
==========

to

==========
[modules] Make handling of module info's regular exports more robust.

R=adamk@chromium.org
BUG=v8:1569
==========

[neis, 2016-11-07 09:24:40]

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

[commit-bot: I haz the power, 2016-11-07 09:24:50]

Dry run: 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-11-07 09:52:42]

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

[commit-bot: I haz the power, 2016-11-07 09:52:43]

Dry run: This issue passed the CQ dry run.

[neis, 2016-11-07 09:53:03]

The CQ bit was checked by neis@chromium.org

[neis, 2016-11-07 09:53:04]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2473993002/#ps20001
(title: "cast size_t")

[commit-bot: I haz the power, 2016-11-07 09:53:12]

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-11-07 09:55:31]

Description was changed from

==========
[modules] Make handling of module info's regular exports more robust.

R=adamk@chromium.org
BUG=v8:1569
==========

to

==========
[modules] Make handling of module info's regular exports more robust.

R=adamk@chromium.org
BUG=v8:1569
==========

[commit-bot: I haz the power, 2016-11-07 09:55:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-17 22:24:18]

Description was changed from

==========
[modules] Make handling of module info's regular exports more robust.

R=adamk@chromium.org
BUG=v8:1569
==========

to

==========
[modules] Make handling of module info's regular exports more robust.

R=adamk@chromium.org
BUG=v8:1569

Committed: https://crrev.com/9d6c6e5a666af9464935cb1b227186f480ecd264
Cr-Commit-Position: refs/heads/master@{#40795}
==========

[commit-bot: I haz the power, 2016-11-17 22:24:19]

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