Support maps for definitions

[sasa1977]

This is the second optimization mentioned in #217. As you proposed there, the new code still works with the current format (list). Note that I was deliberately sloppy with the typespec for the new format, because it would require a lot of duplication with the existing def.

Personally, I think that going forward the new format should be preferred. In this case, we could provide a detailed spec for the map, while using the relaxed list() spec for the "legacy" format.

Of course, we could always have two detailed specs, but maintaining this is going to be more difficult and error prone.

I don't have a strong opinion though, so let me know what you prefer.

[tomas-abrahamsson]

Generally looks fine, but I have a few concerns:

• Could you add a function to convert list-defs to a map, since it is maybe not always obvious: the .proto, may result in 3-element tuples for options in some cases, if I remember right, but maps:to_list() would choke on that. How did you do this? Filter for 2-tuples and then maps:to_list? Maybe such a function should go either into gpb_defs.erl or maybe into gpb.erl for now, it don't know what's best. Maybe gpb.erl for now, as the code generator doesn't need maps that much.
• Does the merge_msgs/3 handle maps? During decoding, if the bytes on the wire contains a field twice, the field is to be merged as mandated by the protobuf. For scalars. merging means overwriting, for repeated it means appending, but if the field is a sub-message, merging means recursively merging the fields of the sub-messages. Maybe this is handled already? I haven't digged deep enough yet, but wanted to make you aware of it.
• It would be good with some unit test with a map. Even if the code is correct now, unit tests also makes sure the functionality doesn't get accidentally removed in the future for example in a refactoring. Beware of the somewhat unorthodox structure of the unit tests though: your tests would be in gpb_tests.erl, but gpb_compile_tests.erl actually -include()s the gpb_tests.erl. Yes, the .erl file. The background for this is that most tests for the data-driven encoder/decoder should work also with the generated encoder/decoder. The top and bottom of the gpb_tests.erl contains code that is not shared with gpb_compile_tests.erl. That would be a good place to put your tests.

[tomas-abrahamsson]

I'm ok with | map() being a bit unspecified for the time being, but I would like the | map() to be added to the relevant function in gpb.erl instead of here, since not every function that accept defs() can handle a map().