Mojom frontend: Implement computeFieldOffsets().

Mojom frontend: Implement computeFieldOffsets().

We compute and serialize the |offset| and |bit| fields of StructField and the |num_bytes| field of StructVersion. In a nutshell we have ported the field packing logic from pack.py in the backend to the frontend.

This is part 2 of the work initiated in https://codereview.chromium.org/1805743003/.

We do not yet consume the data in the backend.

- We moved ComputeVersionInfo() out of user_defined_types.go into computed_data.go because we intend for computed_data.go to encapsulate all of the final data computation.

- In computed_data.go we implement computeFieldOffsets.

- We added the methods SerializationSize() and SerializationAlignment() to the interfaces TypeRef and UserDefinedType.

BUG=#713
R=azani@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/0bbfd71ec4b7dbc48d3673b25b513082b17792c3

[rudominer, 2016-03-21 16:57:30]

Description was changed from

==========
Mojom compiler frontend: Implement computeFieldOffsets().

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

to

==========
Mojom frontend: Implement computeFieldOffsets().

We compute and serialize the |offset| and |bit| fields of StructField and the
|num_bytes| field of StructVersion. In a nutshell we have ported the field
packing logic from pack.py in the backend to the frontend.

This is part 2 of the work initiated in
https://codereview.chromium.org/1805743003/.

We do not yet consume the data in the backend.

- We moved ComputeVersionInfo() out of user_defined_types.go into
computed_data.go because we intend for computed_data.go to encapsulate all of
the final data computation.

- In computed_data.go we implement computeFieldOffsets.

- We added the methods SerializationSize() and SerializationAlignment() to the
interfaces TypeRef and UserDefinedType.

BUG=#713
==========

[rudominer, 2016-03-21 16:58:58]

Patchset #1 (id:1) has been deleted

[rudominer, 2016-03-21 17:01:25]

rudominer@chromium.org changed reviewers:
+ azani@chromium.org, vardhan@google.com

[rudominer, 2016-03-21 17:01:26]

Alex, Vardhan ptal.

Vardhan I'm including you since you will be consuming this data soon in your C++
generator.

I'm still working on adding more test cases but you guys can review now.

[azani, 2016-03-21 20:13:04]

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
File mojom/mojom_parser/mojom/computed_data.go (right):

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
mojom/mojom_parser/mojom/computed_data.go:47: // computeFieldOffsets is invoked
by ComputeFinalData after the
Can you document the algorithm in english?

My understanding is something like this:

Iterate over fields. For each field, place it at the earliest location possible
given its alignment requirement and existing gaps.

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
mojom/mojom_parser/mojom/computed_data.go:124: func (s *MojomStruct)
computeVersionInfo() error {
Can you check here that the offsets have been computer already? I think you
should be able to do that by looking at the second field in the struct.

If the second field is not a bool, its offset should be greater than 0.
If the second field is a bool, either field.offset or field.bit should be
greater than 0.

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/util...
File mojom/mojom_parser/utils/math_utils.go (left):

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/util...
mojom/mojom_parser/utils/math_utils.go:5: #include <vector>
Why did this used to be C++?

[rudominer, 2016-03-21 22:27:18]

Patchset #2 (id:40001) has been deleted

[rudominer, 2016-03-21 22:35:12]

Hi Alex, ptal.

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
File mojom/mojom_parser/mojom/computed_data.go (right):

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
mojom/mojom_parser/mojom/computed_data.go:47: // computeFieldOffsets is invoked
by ComputeFinalData after the
On 2016/03/21 20:13:04, azani wrote:
> Can you document the algorithm in english?
> 
> My understanding is something like this:
> 
> Iterate over fields. For each field, place it at the earliest location
possible
> given its alignment requirement and existing gaps.

Done.

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
mojom/mojom_parser/mojom/computed_data.go:124: func (s *MojomStruct)
computeVersionInfo() error {
On 2016/03/21 20:13:04, azani wrote:
> Can you check here that the offsets have been computer already? I think you
> should be able to do that by looking at the second field in the struct.
> 
> If the second field is not a bool, its offset should be greater than 0.
> If the second field is a bool, either field.offset or field.bit should be
> greater than 0.
field offsets are initialized to -1 so it is easy to tell whether or not
coputeFieldOffsets() has been invoked yet. The accessor method
StructField.Offset() defined in user_defined_types.go is designed to check this
condition and panic if offset has not been set. This accessor method is being
used within the  body of computePayloadSizeSoFar() which is where the field
offsets are consumed via this method.

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/util...
File mojom/mojom_parser/utils/math_utils.go (left):

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/util...
mojom/mojom_parser/utils/math_utils.go:5: #include <vector>
On 2016/03/21 20:13:04, azani wrote:
> Why did this used to be C++?

I believe this must be an artifact of the code review tool using a 50%
similarity rule to decide when a new file is a modification of an existing file.
Some short existing C++ file and this new Go file have the same copyright header
and so are 50% similar.

[azani, 2016-03-21 23:05:39]

lgtm but for a nit

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
File mojom/mojom_parser/mojom/computed_data.go (right):

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
mojom/mojom_parser/mojom/computed_data.go:124: func (s *MojomStruct)
computeVersionInfo() error {
On 2016/03/21 22:35:12, rudominer wrote:
> On 2016/03/21 20:13:04, azani wrote:
> > Can you check here that the offsets have been computer already? I think you
> > should be able to do that by looking at the second field in the struct.
> > 
> > If the second field is not a bool, its offset should be greater than 0.
> > If the second field is a bool, either field.offset or field.bit should be
> > greater than 0.
> field offsets are initialized to -1 so it is easy to tell whether or not
> coputeFieldOffsets() has been invoked yet. The accessor method
> StructField.Offset() defined in user_defined_types.go is designed to check
this
> condition and panic if offset has not been set. This accessor method is being
> used within the  body of computePayloadSizeSoFar() which is where the field
> offsets are consumed via this method.

Maybe document that at the top of this function then.

[rudominer, 2016-03-21 23:39:24]

Vardhan, I haven't heard from you all day and I gather you may be sick so I'm
going to go ahead and land this with Alex's lgtm.

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
File mojom/mojom_parser/mojom/computed_data.go (right):

https://codereview.chromium.org/1823563002/diff/20001/mojom/mojom_parser/mojo...
mojom/mojom_parser/mojom/computed_data.go:124: func (s *MojomStruct)
computeVersionInfo() error {
On 2016/03/21 23:05:39, azani wrote:
> On 2016/03/21 22:35:12, rudominer wrote:
> > On 2016/03/21 20:13:04, azani wrote:
> > > Can you check here that the offsets have been computer already? I think
you
> > > should be able to do that by looking at the second field in the struct.
> > > 
> > > If the second field is not a bool, its offset should be greater than 0.
> > > If the second field is a bool, either field.offset or field.bit should be
> > > greater than 0.
> > field offsets are initialized to -1 so it is easy to tell whether or not
> > coputeFieldOffsets() has been invoked yet. The accessor method
> > StructField.Offset() defined in user_defined_types.go is designed to check
> this
> > condition and panic if offset has not been set. This accessor method is
being
> > used within the  body of computePayloadSizeSoFar() which is where the field
> > offsets are consumed via this method.
> 
> Maybe document that at the top of this function then.

Done.

[rudominer, 2016-03-21 23:41:09]

Description was changed from

==========
Mojom frontend: Implement computeFieldOffsets().

We compute and serialize the |offset| and |bit| fields of StructField and the
|num_bytes| field of StructVersion. In a nutshell we have ported the field
packing logic from pack.py in the backend to the frontend.

This is part 2 of the work initiated in
https://codereview.chromium.org/1805743003/.

We do not yet consume the data in the backend.

- We moved ComputeVersionInfo() out of user_defined_types.go into
computed_data.go because we intend for computed_data.go to encapsulate all of
the final data computation.

- In computed_data.go we implement computeFieldOffsets.

- We added the methods SerializationSize() and SerializationAlignment() to the
interfaces TypeRef and UserDefinedType.

BUG=#713
==========

to

==========
Mojom frontend: Implement computeFieldOffsets().

We compute and serialize the |offset| and |bit| fields of StructField and the
|num_bytes| field of StructVersion. In a nutshell we have ported the field
packing logic from pack.py in the backend to the frontend.

This is part 2 of the work initiated in
https://codereview.chromium.org/1805743003/.

We do not yet consume the data in the backend.

- We moved ComputeVersionInfo() out of user_defined_types.go into
computed_data.go because we intend for computed_data.go to encapsulate all of
the final data computation.

- In computed_data.go we implement computeFieldOffsets.

- We added the methods SerializationSize() and SerializationAlignment() to the
interfaces TypeRef and UserDefinedType.

BUG=#713
R=azani@chromium.org

Committed:
https://chromium.googlesource.com/external/mojo/+/0bbfd71ec4b7dbc48d3673b25b5...
==========

[rudominer, 2016-03-21 23:41:11]

Committed patchset #3 (id:80001) manually as
0bbfd71ec4b7dbc48d3673b25b513082b17792c3 (presubmit successful).