Merge branch 'anders/diameter/augment_inherited_enums/OTP-9469' into dev

* anders/diameter/augment_inherited_enums/OTP-9469:
  Allow @enum when AVP is defined in an inherited dictionary.

3 files changed, 84 insertions, 33 deletions

diff --git a/lib/diameter/doc/src/diameter_dict.xml b/lib/diameter/doc/src/diameter_dict.xml
index 749b518a06..e7c530f1b8 100644
--- a/lib/diameter/doc/src/diameter_dict.xml
+++ b/lib/diameter/doc/src/diameter_dict.xml
@@ -371,7 +371,11 @@ Integer values can be prefixed with 0x to be interpreted as
 hexidecimal.</p>
 
 <p>
-Can occur 0 or more times (with different values of Name).</p>
+Can occur 0 or more times (with different values of Name).
+The AVP in question can be defined in an inherited dictionary in order
+to introduce additional values.
+An AVP so extended must be referenced by in a <c>@messages</c> or
+<c>@grouped</c> section.</p>
 
 <p>
 Example:</p>
diff --git a/lib/diameter/src/compiler/diameter_codegen.erl b/lib/diameter/src/compiler/diameter_codegen.erl
index 6895f38248..30caebc544 100644
--- a/lib/diameter/src/compiler/diameter_codegen.erl
+++ b/lib/diameter/src/compiler/diameter_codegen.erl
@@ -459,9 +459,10 @@ avp(Spec) ->
     Native   = get_value(avp_types, Spec),
     Custom   = get_value(custom_types, Spec),
     Imported = get_value(import_avps, Spec),
-    avp([{N,T} || {N,_,T,_,_} <- Native], Imported, Custom).
+    Enums    = get_value(enums, Spec),
+    avp([{N,T} || {N,_,T,_,_} <- Native], Imported, Custom, Enums).
 
-avp(Native, Imported, Custom) ->
+avp(Native, Imported, Custom, Enums) ->
     Dict = orddict:from_list(Native),
 
     report(native, Dict),
@@ -475,8 +476,8 @@ avp(Native, Imported, Custom) ->
                                    false == lists:member(N, CustomNames)
                            end,
                            Native))
-        ++ lists:flatmap(fun c_imported_avp/1, Imported)
-        ++ lists:flatmap(fun(C) -> c_custom_avp(C, Dict) end, Custom).
+        ++ lists:flatmap(fun(I) -> cs_imported_avp(I, Enums) end, Imported)
+        ++ lists:flatmap(fun(C) -> cs_custom_avp(C, Dict) end, Custom).
 
 c_base_avp({AvpName, T}) ->
     {?clause, [?VAR('T'), ?VAR('Data'), ?ATOM(AvpName)],
@@ -492,23 +493,35 @@ base_avp(AvpName, 'Grouped') ->
 base_avp(_, Type) ->
     ?APPLY(diameter_types, Type, [?VAR('T'), ?VAR('Data')]).
 
-c_imported_avp({Mod, Avps}) ->
-    lists:map(fun(A) -> imported_avp(Mod, A) end, Avps).
+cs_imported_avp({Mod, Avps}, Enums) ->
+    lists:map(fun(A) -> imported_avp(Mod, A, Enums) end, Avps).
 
-imported_avp(_Mod, {AvpName, _, 'Grouped' = T, _, _}) ->
+imported_avp(_Mod, {AvpName, _, 'Grouped' = T, _, _}, _) ->
     c_base_avp({AvpName, T});
 
-imported_avp(Mod, {AvpName, _, _, _, _}) ->
+imported_avp(Mod, {AvpName, _, 'Enumerated' = T, _, _}, Enums) ->
+    case lists:keymember(AvpName, 1, Enums) of
+        true ->
+            c_base_avp({AvpName, T});
+        false ->
+            c_imported_avp(Mod, AvpName)
+    end;
+
+imported_avp(Mod, {AvpName, _, _, _, _}, _) ->
+    c_imported_avp(Mod, AvpName).
+
+c_imported_avp(Mod, AvpName) ->
     {?clause, [?VAR('T'), ?VAR('Data'), ?ATOM(AvpName)],
      [],
      [?APPLY(Mod, avp, [?VAR('T'),
                         ?VAR('Data'),
                         ?ATOM(AvpName)])]}.
 
-c_custom_avp({Mod, Avps}, Dict) ->
-    lists:map(fun(N) -> custom_avp(Mod, N, orddict:fetch(N, Dict)) end, Avps).
+cs_custom_avp({Mod, Avps}, Dict) ->
+    lists:map(fun(N) -> c_custom_avp(Mod, N, orddict:fetch(N, Dict)) end,
+              Avps).
 
-custom_avp(Mod, AvpName, Type) ->
+c_custom_avp(Mod, AvpName, Type) ->
     {?clause, [?VAR('T'), ?VAR('Data'), ?ATOM(AvpName)],
      [],
      [?APPLY(Mod, AvpName, [?VAR('T'), ?ATOM(Type), ?VAR('Data')])]}.
@@ -521,9 +534,25 @@ f_enumerated_avp(Spec) ->
     {?function, enumerated_avp, 3, enumerated_avp(Spec) ++ [?UNEXPECTED(3)]}.
 
 enumerated_avp(Spec) ->
-    lists:flatmap(fun c_enumerated_avp/1, get_value(enums, Spec)).
+    Enums = get_value(enums, Spec),
+    lists:flatmap(fun cs_enumerated_avp/1, Enums)
+        ++ lists:flatmap(fun({M,Es}) -> enumerated_avp(M, Es, Enums) end,
+                         get_value(import_enums, Spec)).
+
+enumerated_avp(Mod, Es, Enums) ->
+    lists:flatmap(fun({N,_}) ->
+                          cs_enumerated_avp(lists:keymember(N, 1, Enums),
+                                            Mod,
+                                            N)
+                  end,
+                  Es).
+
+cs_enumerated_avp(true, Mod, Name) ->
+    [c_imported_avp(Mod, Name)];
+cs_enumerated_avp(false, _, _) ->
+    [].
 
-c_enumerated_avp({AvpName, Values}) ->
+cs_enumerated_avp({AvpName, Values}) ->
     lists:flatmap(fun(V) -> c_enumerated_avp(AvpName, V) end, Values).
 
 c_enumerated_avp(AvpName, {I,_}) ->
@@ -625,10 +654,12 @@ f_empty_value(Spec) ->
     {?function, empty_value, 1, empty_value(Spec)}.
 
 empty_value(Spec) ->
+    Imported = lists:flatmap(fun avps/1, get_value(import_enums, Spec)),
     Groups = get_value(grouped, Spec)
         ++ lists:flatmap(fun avps/1, get_value(import_groups, Spec)),
-    Enums = get_value(enums, Spec)
-        ++ lists:flatmap(fun avps/1, get_value(import_enums, Spec)),
+    Enums = [T || {N,_} = T <- get_value(enums, Spec),
+                  not lists:keymember(N, 1, Imported)]
+        ++ Imported,
     lists:map(fun c_empty_value/1, Groups ++ Enums)
         ++ [{?clause, [?VAR('Name')], [], [?CALL(empty, [?VAR('Name')])]}].
 
diff --git a/lib/diameter/src/compiler/diameter_spec_util.erl b/lib/diameter/src/compiler/diameter_spec_util.erl
index 322d53a199..b60886b678 100644
--- a/lib/diameter/src/compiler/diameter_spec_util.erl
+++ b/lib/diameter/src/compiler/diameter_spec_util.erl
@@ -39,11 +39,11 @@ parse(Path, Options) ->
     {ok, B} = file:read_file(Path),
     Chunks = chunk(B),
     Spec = make_spec(Chunks),
-    true = enums_defined(Spec),   %% sanity checks
-    true = groups_defined(Spec),  %%
+    true = groups_defined(Spec),  %% sanity checks
     true = customs_defined(Spec), %%
     Full = import_enums(import_groups(import_avps(insert_codes(Spec),
                                                   Options))),
+    true = enums_defined(Full),   %% sanity checks
     true = v_flags_set(Spec),
     Full.
 
@@ -243,35 +243,48 @@ get_value(Key, Spec) ->
 %% with an appropriate type.
 
 enums_defined(Spec) ->
-    is_defined(Spec, 'Enumerated', enums).
+    Avps = get_value(avp_types, Spec),
+    Import = get_value(import_enums, Spec),
+    lists:all(fun({N,_}) ->
+                      true = enum_defined(N, Avps, Import)
+              end,
+              get_value(enums, Spec)).
 
-groups_defined(Spec) ->
-    is_defined(Spec, 'Grouped', grouped).
+enum_defined(Name, Avps, Import) ->
+    case lists:keyfind(Name, 1, Avps) of
+        {Name, _, 'Enumerated', _, _} ->
+            true;
+        {Name, _, T, _, _} ->
+            ?ERROR({avp_has_wrong_type, Name, 'Enumerated', T});
+        false ->
+            lists:any(fun({_,Is}) -> lists:keymember(Name, 1, Is) end, Import)
+                orelse ?ERROR({avp_not_defined, Name, 'Enumerated'})
+    end.
+%% Note that an AVP is imported only if referenced by a message or
+%% grouped AVP, so the final branch will fail if an enum definition is
+%% extended without this being the case.
 
-is_defined(Spec, Type, Key) ->
+groups_defined(Spec) ->
     Avps = get_value(avp_types, Spec),
-    lists:all(fun(T) -> true = is_local(name(Key, T), Type, Avps) end,
-              get_value(Key, Spec)).
+    lists:all(fun({N,_,_,_}) -> true = group_defined(N, Avps) end,
+              get_value(grouped, Spec)).
 
-name(enums,   {N,_})     -> N;
-name(grouped, {N,_,_,_}) -> N.
-
-is_local(Name, Type, Avps) ->
+group_defined(Name, Avps) ->
     case lists:keyfind(Name, 1, Avps) of
-        {Name, _, Type, _, _} ->
+        {Name, _, 'Grouped', _, _} ->
             true;
         {Name, _, T, _, _} ->
-            ?ERROR({avp_has_wrong_type, Name, Type, T});
+            ?ERROR({avp_has_wrong_type, Name, 'Grouped', T});
         false ->
-            ?ERROR({avp_not_defined, Name, Type})
+            ?ERROR({avp_not_defined, Name, 'Grouped'})
     end.
 
 customs_defined(Spec) ->
     Avps = get_value(avp_types, Spec),
-    lists:all(fun(A) -> true = is_local(A, Avps) end,
+    lists:all(fun(A) -> true = custom_defined(A, Avps) end,
               lists:flatmap(fun last/1, get_value(custom_types, Spec))).
 
-is_local(Name, Avps) ->
+custom_defined(Name, Avps) ->
     case lists:keyfind(Name, 1, Avps) of
         {Name, _, T, _, _} when T == 'Grouped';
                                 T == 'Enumerated' ->
@@ -510,6 +523,9 @@ choose(false, _, X) -> X.
 %% ------------------------------------------------------------------------
 %% import_groups/1
 %% import_enums/1
+%%
+%% For each inherited module, store the content of imported AVP's of
+%% type grouped/enumerated in a new key.
 
 import_groups(Spec) ->
     orddict:store(import_groups, import(grouped, Spec), Spec).