tree 7b380140ae46b5e3fa73f79826342b60680ebb69
parent b5f7a2ab16fc874d382fd877b14e0920ecafe785
author Mitch Rudominer <rudominer@chromium.org> 1462912059 -0700
committer Mitch Rudominer <rudominer@chromium.org> 1462912059 -0700

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

Review URL: https://codereview.chromium.org/1958463003 .