[modules] Detect all indirect exports and represent them as such.

[modules] Detect all indirect exports and represent them as such.

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

[neis, 2016-08-09 15:07:42]

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

[commit-bot: I haz the power, 2016-08-09 15:07:58]

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

[neis, 2016-08-09 15:08:47]

Description was changed from

==========
[modules] Detect all indirect exports and represent them as such.

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

to

==========
[modules] Detect all indirect exports and represent them as such.

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

[neis, 2016-08-09 15:08:47]

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

[neis, 2016-08-09 15:08:55]

ptal

[commit-bot: I haz the power, 2016-08-09 15:31:30]

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

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

Dry run: This issue passed the CQ dry run.

[adamk, 2016-08-09 16:49:02]

Description was changed from

==========
[modules] Detect all indirect exports and represent them as such.

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

to

==========
[modules] Detect all indirect exports and represent them as such.

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

[adamk, 2016-08-09 16:52:52]

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

https://codereview.chromium.org/2223893004/diff/1/src/ast/modules.h#newcode103
src/ast/modules.h:103: const ZoneList<ModuleEntry*>& exports() const { return
exports_; }
Maybe remove the const-ness of this method? I realize it's still C++-const, but
it's no longer conceptually-const.

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

https://codereview.chromium.org/2223893004/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:1025: module()->MakeIndirectExportsExplicit();
Maybe this should just be called by Validate()? It doesn't seem like something
the Parser should need to know about.

[adamk, 2016-08-09 17:42:22]

lgtm with nits, don't want this to be held up on my review

[neis, 2016-08-10 11:53:30]

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

https://codereview.chromium.org/2223893004/diff/1/src/ast/modules.h#newcode103
src/ast/modules.h:103: const ZoneList<ModuleEntry*>& exports() const { return
exports_; }
On 2016/08/09 16:52:51, adamk wrote:
> Maybe remove the const-ness of this method? I realize it's still C++-const,
but
> it's no longer conceptually-const.

I don't understand, it's just a getter.

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

https://codereview.chromium.org/2223893004/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:1025: module()->MakeIndirectExportsExplicit();
On 2016/08/09 16:52:52, adamk wrote:
> Maybe this should just be called by Validate()? It doesn't seem like something
> the Parser should need to know about.

Yeah, I was unsure about this too. I moved it into Validate for now, but I could
imagine having a separate PrepareForCompilation or such that is called after
parsing.

[neis, 2016-08-10 11:53:38]

The CQ bit was checked by neis@chromium.org

[neis, 2016-08-10 11:53:39]

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/2223893004/#ps20001
(title: "Move call into Validate")

[commit-bot: I haz the power, 2016-08-10 11:53: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-08-10 12:18:19]

Description was changed from

==========
[modules] Detect all indirect exports and represent them as such.

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

to

==========
[modules] Detect all indirect exports and represent them as such.

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

[commit-bot: I haz the power, 2016-08-10 12:18:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-10 12:18:34]

Description was changed from

==========
[modules] Detect all indirect exports and represent them as such.

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

to

==========
[modules] Detect all indirect exports and represent them as such.

BUG=v8:1569

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

[commit-bot: I haz the power, 2016-08-10 12:18:35]

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

[adamk, 2016-08-10 17:30:29]

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

https://codereview.chromium.org/2223893004/diff/1/src/ast/modules.h#newcode103
src/ast/modules.h:103: const ZoneList<ModuleEntry*>& exports() const { return
exports_; }
On 2016/08/10 11:53:30, neis wrote:
> On 2016/08/09 16:52:51, adamk wrote:
> > Maybe remove the const-ness of this method? I realize it's still C++-const,
> but
> > it's no longer conceptually-const.
> 
> I don't understand, it's just a getter.

But it now lets callers mutate the internal state of the class (the ModuleEntry
objects).

Anyway, this really doesn't matter too much, just one of those C++ things that's
easy to have different opinions on.