Skip to content

Commit

Permalink
Handle the enum option allow_alias = true
Browse files Browse the repository at this point in the history
This option allows several enumerators to have the same numeric
value. For langages where the enum symbol is really an integer value,
this causes no problems, but for gpb, which maps numeric values to
atoms, it has been solved this way:

  Given this example:

    enum e {
       option allow_alias = true;
       x = 1;
       y = 1;
    };
    message m { required e field1 = 1; };

  - encoding accepts all atoms, ie: #m{field1 = x} and #m{field1 = y}
    would encode to the same.
  - on decoding, it decodes to the first defined enum symbol:
    seeing an 1 on the wire would decode to #{field1 = x}.

In the introspection functions, such an option field is a three-tuple
in list of enumerator values returned:

  find_enum_def(e) will return the following:
     [{option, allow_alias, true},
      {x, 1},
      {y, 1}]

The enum_symbol_by_value/2 and enum_symbol_by_value_e/1
will return the atom x for the value 1.

This option is true in gpb, regardless of actual option setting.
It appeared in google protobuf 2.5.0, and the plan to make it default
to true in some future has been advertized. Having it always true in
gpb means we won't be generating a compile time error for the case
that the option is set to false, and more than enum have the same
numeric value.
  • Loading branch information
tomas-abrahamsson committed Jan 23, 2015
1 parent f43b727 commit 34423df
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 3 deletions.
13 changes: 11 additions & 2 deletions src/gpb_compile.erl
Expand Up @@ -1965,7 +1965,7 @@ format_enum_decoders(Defs, #anres{used_types=UsedTypes}) ->
[repeat_clauses('<EnumValue>',
[[replace_term('<EnumValue>', EnumValue),
replace_term('<EnumSym>', EnumSym)]
|| {EnumSym, EnumValue} <- EnumDef])])
|| {EnumSym, EnumValue} <- unalias_enum(EnumDef)])])
|| {{enum, EnumName}, EnumDef} <- Defs,
smember({enum,EnumName}, UsedTypes)].

Expand Down Expand Up @@ -3311,7 +3311,7 @@ format_enum_value_symbol_converters(EnumDefs) when EnumDefs /= [] ->
[repeat_clauses('<Value>',
[[replace_term('<Value>', EnumValue),
replace_term('<Sym>', EnumSym)]
|| {EnumSym, EnumValue} <- EnumDef])]),
|| {EnumSym, EnumValue} <- unalias_enum(EnumDef)])]),
"\n",
gpb_codegen:format_fn(
mk_fn(enum_value_by_symbol_, EnumName),
Expand Down Expand Up @@ -4960,6 +4960,15 @@ map_kvalues(KVars) ->
end
|| {Key, Expr} <- KVars].

%% The "option allow_alias = true;" inside an enum X { ... }
%% says it is ok to have multiple symbols that map to the same numeric value.
%% Appeared in protobuf 2.5.0.
unalias_enum([{_Sym,Value}=Enum | Rest]) ->
[Enum | unalias_enum([E || {_,V}=E <- Rest, V /= Value])];
unalias_enum([{option,_Name,_Value} | Rest]) ->
unalias_enum(Rest);
unalias_enum([]) ->
[].

var_f_n(N) -> var_n("F", N).
var_b_n(N) -> var_n("B", N).
Expand Down
13 changes: 12 additions & 1 deletion src/gpb_parse.yrl
Expand Up @@ -101,6 +101,7 @@ enum_def -> enum identifier '{' enum_fields '}':
{{enum,identifier_name('$2')},'$4'}.

enum_fields -> enum_field enum_fields: ['$1' | '$2'].
enum_fields -> option_def enum_fields: ['$1' | '$2'].
enum_fields -> ';' enum_fields: '$2'.
enum_fields -> '$empty': [].

Expand Down Expand Up @@ -713,7 +714,7 @@ reformat_names(Defs) ->
lists:map(fun({{msg,Name}, Fields}) ->
{{msg,reformat_name(Name)}, reformat_fields(Fields)};
({{enum,Name}, ENs}) ->
{{enum,reformat_name(Name)}, ENs};
{{enum,reformat_name(Name)}, reformat_enum_opt_names(ENs)};
({{extensions,Name}, Exts}) ->
{{extensions,reformat_name(Name)}, Exts};
({{extend,Name}, Fields}) ->
Expand All @@ -738,6 +739,16 @@ reformat_fields(Fields) ->
end,
Fields).

%% `Defs' is expected to be parsed.
reformat_enum_opt_names(Def) ->
[case Item of
{option, Name, Value} ->
{option, reformat_name(Name), Value};
Other ->
Other
end
|| Item <- Def].

reformat_name(Name) ->
list_to_atom(string:join([atom_to_list(P) || P <- Name,
P /= '.'],
Expand Down
9 changes: 9 additions & 0 deletions test/gpb_parse_tests.erl
Expand Up @@ -181,6 +181,15 @@ parses_import_test() ->
parse_lines(["package p1.p2;",
"import \"a/b/c.proto\";"]).

parses_enum_option_test() ->
{ok, Elems} = parse_lines(["enum e1 {",
" option allow_alias = true;",
" ee1 = 1;",
" ee2 = 1;",
"}"]),
[{{enum,e1}, [{option, allow_alias, true}, {ee1,1},{ee2,1}]}] =
do_process_sort_defs(Elems).

generates_correct_absolute_names_test() ->
{ok, Elems} = parse_lines(["message m1 {"
" message m2 { required uint32 x = 1; }",
Expand Down
21 changes: 21 additions & 0 deletions test/gpb_tests.erl
Expand Up @@ -173,6 +173,17 @@ decode_msg_with_negative_enum_value_test() ->
{{enum,e}, [{v1, 100},
{v2, -2}]}]).

decode_msg_with_enum_aliases_test() ->
#m1{a = v1} =
decode_msg(<<8,100>>,
m1,
[{{msg,m1}, [#?gpb_field{name=a, fnum=1, rnum=#m1.a,
type={enum,e}, occurrence=required,
opts=[]}]},
{{enum,e}, [{option, allow_alias, true},
{v1, 100},
{v2, 100}]}]).

decode_msg_with_bool_field_test() ->
#m1{a = true} =
decode_msg(<<8,1>>,
Expand Down Expand Up @@ -462,6 +473,16 @@ encode_msg_with_negative_enum_value_test() ->
<<254,255,255,255,255,255,255,255,255,1>> -> ok %% encoded as 64 bits
end.

encode_msg_with_enum_aliases_test() ->
Defs = [{{msg,m1}, [#?gpb_field{name=a, fnum=1, rnum=#m1.a,
type={enum,e}, occurrence=required,
opts=[]}]},
{{enum,e}, [{option, allow_alias, true},
{v1, 100},
{v2, 100}]}],
<<8,100>> = encode_msg(#m1{a = v1}, Defs),
<<8,100>> = encode_msg(#m1{a = v2}, Defs).

encode_msg_with_bool_field_test() ->
<<8,1>> =
encode_msg(#m1{a = true},
Expand Down

0 comments on commit 34423df

Please sign in to comment.