[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

R=adamk@chromium.org
TBR=bmeurer@chromium.org
BUG=v8:1569
Committed: https://crrev.com/43995e2fa876c9501f7eb9440d33999708f1428b
Cr-Commit-Position: refs/heads/master@{#40754}

[adamk, 2016-10-31 18:14:36]

lgtm % heap verification and a few nits in objects.h

https://codereview.chromium.org/2460353002/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/2460353002/diff/1/src/objects-debug.cc#newcod...
src/objects-debug.cc:1003: void ModuleInfoEntry::ModuleInfoEntryVerify() {
CHECK(IsModuleInfoEntry()); }
Please add more here. It seems like the first three fields should be either
undefined (or is it the hole?) or strings, while the last four fields should be
Smis.

https://codereview.chromium.org/2460353002/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8082
src/objects.h:8082: static const int kExportNameOffset =
HeapObject::kHeaderSize;
Can these all be private still? Seems like the answer should be "yes", since the
factory method is a member. Not a big deal, but it stays consistent with the
previous implementation.

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8090
src/objects.h:8090: };
Please add

private:
 DISALLOW_IMPLICIT_CONSTRUCTORS(ModuleInfoEntry)

while you're at it.

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8123
src/objects.h:8123: };
And might as well add DISALLOW_IMPLICIT... here too

[adamk, 2016-10-31 20:15:19]

https://codereview.chromium.org/2460353002/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8090
src/objects.h:8090: };
On 2016/10/31 18:14:36, adamk wrote:
> Please add
> 
> private:
>  DISALLOW_IMPLICIT_CONSTRUCTORS(ModuleInfoEntry)
> 
> while you're at it.

Er, I guess the modern spelling is DISALLOW_COPY_AND_ASSIGN

[neis, 2016-11-02 09:59:57]

https://codereview.chromium.org/2460353002/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/2460353002/diff/1/src/objects-debug.cc#newcod...
src/objects-debug.cc:1003: void ModuleInfoEntry::ModuleInfoEntryVerify() {
CHECK(IsModuleInfoEntry()); }
On 2016/10/31 18:14:36, adamk wrote:
> Please add more here. It seems like the first three fields should be either
> undefined (or is it the hole?) or strings, while the last four fields should
be
> Smis.

Done.

https://codereview.chromium.org/2460353002/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8082
src/objects.h:8082: static const int kExportNameOffset =
HeapObject::kHeaderSize;
On 2016/10/31 18:14:36, adamk wrote:
> Can these all be private still? Seems like the answer should be "yes", since
the
> factory method is a member. Not a big deal, but it stays consistent with the
> previous implementation.

kSize must be public, that's why I made all public.

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8090
src/objects.h:8090: };
On 2016/10/31 20:15:19, adamk wrote:
> On 2016/10/31 18:14:36, adamk wrote:
> > Please add
> > 
> > private:
> >  DISALLOW_IMPLICIT_CONSTRUCTORS(ModuleInfoEntry)
> > 
> > while you're at it.
> 
> Er, I guess the modern spelling is DISALLOW_COPY_AND_ASSIGN

I'm confused. Those are two different macros.

https://codereview.chromium.org/2460353002/diff/1/src/objects.h#newcode8123
src/objects.h:8123: };
On 2016/10/31 18:14:36, adamk wrote:
> And might as well add DISALLOW_IMPLICIT... here too

Done.

[adamk, 2016-11-02 18:38:24]

still lgtm

[Hannes Payer (out of office), 2016-11-03 10:05:21]

hpayer@chromium.org changed reviewers:
+ hpayer@chromium.org

[Hannes Payer (out of office), 2016-11-03 10:05:21]

heap lgtm

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

The CQ bit was checked by neis@chromium.org

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

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org,
adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2460353002/#ps40001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-11-04 08:53:20]

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-04 08:56:49]

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

[commit-bot: I haz the power, 2016-11-04 08:56:50]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/27831)

[neis, 2016-11-04 08:59:23]

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

[neis, 2016-11-04 08:59:24]

+bmeurer for compiler

[neis, 2016-11-04 09:06:18]

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

[commit-bot: I haz the power, 2016-11-04 09:06:24]

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

[neis, 2016-11-04 09:32:44]

Description was changed from

==========
[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

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

to

==========
[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

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

[commit-bot: I haz the power, 2016-11-04 09:34:48]

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

[commit-bot: I haz the power, 2016-11-04 09:34:51]

Dry run: This issue passed the CQ dry run.

[neis, 2016-11-04 09:35:21]

The CQ bit was checked by neis@chromium.org

[neis, 2016-11-04 09:35:22]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org,
adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2460353002/#ps60001
(title: "Remove extra semicolons after macro use.")

[commit-bot: I haz the power, 2016-11-04 09:35:26]

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-04 09:37:28]

Description was changed from

==========
[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

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

to

==========
[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

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

[commit-bot: I haz the power, 2016-11-04 09:37:29]

Committed patchset #4 (id:60001)

[Benedikt Meurer, 2016-11-04 16:51:06]

LGTM

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

Description was changed from

==========
[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

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

to

==========
[modules] Make ModuleInfoEntry a Struct rather than FixedArray.

It always has the same number of slots.

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

Committed: https://crrev.com/43995e2fa876c9501f7eb9440d33999708f1428b
Cr-Commit-Position: refs/heads/master@{#40754}
==========

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

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