Mojom compiler: Eliminate duplicate representation of enum values in mojom_files.mojom.

Mojom compiler: Eliminate duplicate representation of enum values in mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of an enum value
(including for example the whole DeclarationData) occurred twice: It occurred once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry we have only a constant registry.
References to enum values are now references to the owning EnumType plus an index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field "map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we replaced the field
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then |constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the |enum_type_key| and |enum_value_index|
       are populated. The idea is that for enun values instead of referring to them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so this extra data is not useful.

- In serialization.go we modified the serialization logic based on these changes.
- In mojom_translatory.py we modified the translation logic based on these changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum values. It seemed more natural to just leave the enum values
within their enum. Strategy (b) also makes this CL shorter (believe it or not) because I don't have to make any changes to any runtime type querying code.

BUG=fixes #513
R=azani@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/a60302d6a4532fc3845a5d68a31d227687257b88

[rudominer, 2016-05-06 00:33:25]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this there were two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry.

BUG=fixes #513
==========

[rudominer, 2016-05-06 18:20:42]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this there were two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry.

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

BUG=fixes #513
==========

[rudominer, 2016-05-06 20:01:51]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       is populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

BUG=fixes #513
==========

[rudominer, 2016-05-06 20:15:47]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       is populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       is populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum.

BUG=fixes #513
==========

[rudominer, 2016-05-06 20:16:46]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       is populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum.

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       is populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum. Strategy (b) also makes this CL shorter (believe it or not)
because I don't have to make any changes to any runtime type querying code.

BUG=fixes #513
==========

[rudominer, 2016-05-06 20:17:02]

Patchset #1 (id:1) has been deleted

[rudominer, 2016-05-06 20:17:12]

Patchset #1 (id:20001) has been deleted

[rudominer, 2016-05-06 20:17:22]

Patchset #1 (id:40001) has been deleted

[rudominer, 2016-05-06 20:19:21]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       is populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum. Strategy (b) also makes this CL shorter (believe it or not)
because I don't have to make any changes to any runtime type querying code.

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       are populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum. Strategy (b) also makes this CL shorter (believe it or not)
because I don't have to make any changes to any runtime type querying code.

BUG=fixes #513
==========

[rudominer, 2016-05-06 20:22:54]

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

[rudominer, 2016-05-06 20:22:55]

Alex, Vardhan ptal.

After this lands you will need to make some small changes in the new C and Go
generators you are writing. See the changes in mojom_translator.py for an idea
of what changes you will have to make.

[vardhan, 2016-05-09 22:06:29]

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
File mojo/public/interfaces/bindings/mojom_types.mojom (right):

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
mojo/public/interfaces/bindings/mojom_types.mojom:359: string? enum_type_key;
i was going to suggest that enum_type_key and enum_value_key be put together in
a mojom struct (EnumValueReference or something?), and that constant_key and the
EnumValueReference be in a union.

my reservation though is that this might add extra overhead in the encoded
runtime type info?
technically speaking if you wanted to optimize, it should be possible to replace
all UserValueReferences to LiteralValue for the purposes of RTT info, in order
to save space.

what do you think?

[azani, 2016-05-09 22:10:19]

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
File mojo/public/interfaces/bindings/mojom_types.mojom (right):

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
mojo/public/interfaces/bindings/mojom_types.mojom:256: array<EnumValue> values;
Can you document the order here now that the position of the value in this array
is semantically important?

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
mojo/public/interfaces/bindings/mojom_types.mojom:353: string? constant_key;
Instead of having the logic document the requirement that either constant_key or
enum_type_key and enum_value_index be set, perhaps we should have a union with 2
fields:
string constant_key
EnumValueKey enum_value_key

with

struct EnumValueKey {
string enum_type_key
uint32 enum_value_index
}

[rudominer, 2016-05-10 17:57:59]

ptal

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
File mojo/public/interfaces/bindings/mojom_types.mojom (right):

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
mojo/public/interfaces/bindings/mojom_types.mojom:256: array<EnumValue> values;
On 2016/05/09 22:10:19, azani wrote:
> Can you document the order here now that the position of the value in this
array
> is semantically important?

Done.

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
mojo/public/interfaces/bindings/mojom_types.mojom:353: string? constant_key;
On 2016/05/09 22:10:19, azani wrote:
> Instead of having the logic document the requirement that either constant_key
or
> enum_type_key and enum_value_index be set, perhaps we should have a union with
2
> fields:
> string constant_key
> EnumValueKey enum_value_key
> 
> with
> 
> struct EnumValueKey {
> string enum_type_key
> uint32 enum_value_index
> }

Done.

https://codereview.chromium.org/1958463003/diff/60001/mojo/public/interfaces/...
mojo/public/interfaces/bindings/mojom_types.mojom:359: string? enum_type_key;
On 2016/05/09 22:06:29, vardhan wrote:
> i was going to suggest that enum_type_key and enum_value_key be put together
in
> a mojom struct (EnumValueReference or something?), and that constant_key and
the
> EnumValueReference be in a union.
> 
> my reservation though is that this might add extra overhead in the encoded
> runtime type info?
> technically speaking if you wanted to optimize, it should be possible to
replace
> all UserValueReferences to LiteralValue for the purposes of RTT info, in order
> to save space.
> 
> what do you think?

Done.

[azani, 2016-05-10 18:38:57]

lgtm

[rudominer, 2016-05-10 20:27:42]

Description was changed from

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field 
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       are populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum. Strategy (b) also makes this CL shorter (believe it or not)
because I don't have to make any changes to any runtime type querying code.

BUG=fixes #513
==========

to

==========
Mojom compiler: Eliminate duplicate representation of enum values in
mojom_types.mojom.

This cl addresses the problem that in mojom_files.mojom, the full definition of
an enum value
(including for example the whole DeclarationData) occurred twice: It occurred
once in the value
registry (i.e. the |resolved_values| map) and once within the owning MojomEnum
strcut that lived
in the type registry (i.e. the |resolved_types| map.)

In order to address this I considered two possible strategies:
(a) Replace the field "array<EnumValue> values" within MojomEnum with a new
field
"array<string> value_keys;"

or

(b) Take the enum values out of the value registry. Instead of a value registry
we have only a constant registry.
References to enum values are now references to the owning EnumType plus an
index into the list of values within the enum.

I first tried (a) In a different patch but I didn't like it (for reasons I will
elaborate below*). In this patch strategy (b)
is used.

Summary of changes:

- In mojom_files.mojom in the struct MojomFileGraph replace the field
"map<string, UserDefinedValue> resolved_values;"
with the field "map<string, DeclaredConstant> resolved_constants;" (I also fixed
up some comments.)

- In mojom_types.mojom we made three changes:
  (i) We deleted |UserDefinedValue|. This was meant as a "super-type" to
EnumValue and DeclaredConstant.
      We no longer need this abstraction.
  (ii) In struct UserValueReference (which represents a reference to a value) we
replaced the field
       string? value_key; with three fields:
              - string? constant_key;
              - string? enum_type_key;
              - uint32 enum_value_index;
       If the UserValueReference is a reference to a constant then
|constant_key| is populated. If the
       UserValueReference is a reference to an enum value then the
|enum_type_key| and |enum_value_index|
       are populated. The idea is that for enun values instead of referring to
them via a key to a value registry
       we refer to them via a key to the enum in the type registry and an index
into the list of enum values.
  (iii) In struct EnumValue we deleted the field |enum_type_key|. Previously it
was possible to access an enum
       value indirectly via its value key and so it was useful to be able to get
the key to the containing enum.
       In the new scheme an enum value is always accessed via its enum and so
this extra data is not useful.

- In serialization.go we modified the serialization logic based on these
changes.
- In mojom_translatory.py we modified the translation logic based on these
changes.
- All the other files are tests or generated files.

*Why did I end up not liking strategy (a)? If you think only about the use of
mojom_files.mojom and mojom_types.mojom in the context
of the Mojom compiler then strategy (a) sounds good, which is why I initially
pursued it. But I soon realized it is not great for
the runtime type querying use case. In this use case mojom_files.mojom is not
used, only mojom_types.mojom along with service_describer.mojom.
Runtime type querying currently has no notion of a value registry, and it does
not deal with constants at all--constants don't seem to be
useful at runtime. (At least I am not aware of a use for them at this time.) If
I had taken the enum values out of the enum then for the
runtime type querying use case I would have added a registry of only enum
values. It seemed more natural to just leave the enum values
within their enum. Strategy (b) also makes this CL shorter (believe it or not)
because I don't have to make any changes to any runtime type querying code.

BUG=fixes #513
R=azani@chromium.org

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

[rudominer, 2016-05-10 20:27:43]

Committed patchset #2 (id:80001) manually as
a60302d6a4532fc3845a5d68a31d227687257b88 (presubmit successful).