From 7ffc96b68ae0f2fec4a430ee31da4e2aa9904427 Mon Sep 17 00:00:00 2001 From: Jarvis Carroll Date: Wed, 24 Sep 2025 14:10:29 +1000 Subject: [PATCH] Refactor type normalization Some of these checks were redundant, and we probably don't actually need substitution to wrap success/failure, since it isn't expected to fail anyway... Now the logic is much simpler, and adding more built-in type definitions should be easy. --- src/hz.erl | 162 ++++++++++++++++++++++++----------------------------- 1 file changed, 72 insertions(+), 90 deletions(-) diff --git a/src/hz.erl b/src/hz.erl index b48b891..0187fae 100644 --- a/src/hz.erl +++ b/src/hz.erl @@ -1652,18 +1652,14 @@ annotate_variants([{Name, Elems} | Rest], Types, Acc) -> annotate_variants([], _Types, Acc) -> {ok, lists:reverse(Acc)}. -normalize_opaque_type(T, Types) -> - case type_is_expanded(T) of - false -> normalize_opaque_type(T, Types, true); - true -> {ok, true, T, T} - end. - % This function evaluates type aliases in a loop, until eventually a usable % definition is found. % % It also evaluates built-in and standard library types such as options and % names, to their defined variant representation, as well as evaluating % certain binary types like hash, fp, and fr, into their byte representations. +normalize_opaque_type(T, Types) -> normalize_opaque_type(T, Types, true). + % FIXME detect infinite loops % FIXME detect builtins with the wrong number of arguments % FIXME should nullary types have an empty list of arguments added before now? @@ -1675,106 +1671,92 @@ normalize_opaque_type(hash, _Types, IsFirst) -> % For coercion purposes, hash is indistinguishable from bytes(32), so we % treat it like a type alias. {ok, IsFirst, hash, {bytes, [32]}}; +normalize_opaque_type(T, _Types, IsFirst) when is_atom(T) -> + % Once we have eliminated the above rewrite cases, all other cases are + % handled explicitly by the coerce logic, and so are considered normalized. + {ok, IsFirst, T, T}; +normalize_opaque_type(Type = {T, _}, _Types, IsFirst) when is_atom(T) -> + % Once we have eliminated the above rewrite cases, all other cases are + % handled explicitly by the coerce logic, and so are considered normalized. + {ok, IsFirst, Type, Type}; normalize_opaque_type(T, Types, IsFirst) when is_list(T) -> + % Lists/strings indicate userspace types, which may require arg + % substitutions. Convert to an explicit but empty arg list, for uniformity. normalize_opaque_type({T, []}, Types, IsFirst); normalize_opaque_type({T, TypeArgs}, Types, IsFirst) when is_list(T) -> case maps:find(T, Types) of - %{error, invalid_aci}; % FIXME more info error -> - {ok, IsFirst, {T, TypeArgs}, {unknown_type, TypeArgs}}; + % We couldn't find this named type... Keep building the AACI, but + % mark this type expression as unknown, so that FATE coercions + % aren't attempted. + {ok, IsFirst, {T, TypeArgs}, unknown_type}; {ok, {TypeParamNames, Definition}} -> - Bindings = lists:zip(TypeParamNames, TypeArgs), - normalize_opaque_type2(T, TypeArgs, Types, IsFirst, Bindings, Definition) + % We have a definition for this type, including names for whatever + % args we have been given. Subtitute our args into this. + NewType = substitute_opaque_type(TypeParamNames, Definition, TypeArgs), + % Now continue on to see if we need to restart the loop or not. + normalize_opaque_type2(IsFirst, {T, TypeArgs}, NewType, Types) end. -normalize_opaque_type2(T, TypeArgs, Types, IsFirst, Bindings, Definition) -> - SubResult = - case Bindings of - [] -> {ok, Definition}; - _ -> substitute_opaque_type(Bindings, Definition) - end, - case SubResult of - % Type names were already normalized if they were ADTs or records, - % since for those connectives the name is considered part of the type. - {ok, NextT = {variant, _}} -> - {ok, IsFirst, {T, TypeArgs}, NextT}; - {ok, NextT = {record, _}} -> - {ok, IsFirst, {T, TypeArgs}, NextT}; - % Everything else has to be substituted down to a built-in connective - % to be considered normalized. - {ok, NextT} -> - normalize_opaque_type3(NextT, Types); - Error -> - Error - end. +normalize_opaque_type2(IsFirst, PrevType, NextType = {variant, _}, _) -> + % We have reduced to a variant. Report the type name as the normalized + % type, but also provide the variant definition itself as the candidate + % flattened type for further annotation. + {ok, IsFirst, PrevType, NextType}; +normalize_opaque_type2(IsFirst, PrevType, NextType = {record, _}, _) -> + % We have reduced to a record. Report the type name as the normalized + % type, but also provide the record definition itself as the candidate + % flattened type for further annotation. + {ok, IsFirst, PrevType, NextType}; +normalize_opaque_type2(_, _, NextType, Types) -> + % Not a variant or record yet, so go back to the start of the loop. + % It will no longer be the first iteration. + normalize_opaque_type(NextType, Types, false). -% while this does look like normalize_opaque_type/2, it sets IsFirst to false -% instead of true, and is part of the loop, instead of being an initial -% condition for the loop. -normalize_opaque_type3(NextT, Types) -> - case type_is_expanded(NextT) of - false -> normalize_opaque_type(NextT, Types, false); - true -> {ok, false, NextT, NextT} - end. +% Perform a beta-reduction on a type expression. +substitute_opaque_type([], Definition, _) -> + % There are no parameters to substitute. This is the simplest way of + % defining type aliases, records, and variants, so we should make sure to + % short circuit all the recursive descent logic, since it won't actually + % do anything. + Definition; +substitute_opaque_type(TypeParamNames, Definition, TypeArgs) -> + % Bundle the param names alongside the args that we want to substitute, so + % that we can keyfind the one list. + Bindings = lists:zip(TypeParamNames, TypeArgs), + substitute_opaque_type(Bindings, Definition). -% Strings indicate names that should be substituted. Atoms indicate built in -% types, which don't need to be expanded, except for option. -% TODO: Stop calling this, so that we can stop redundantly enumerating all the -% built in types. -type_is_expanded({option, _}) -> false; -type_is_expanded(hash) -> false; -type_is_expanded(X) when is_atom(X) -> true; -type_is_expanded({X, _}) when is_atom(X) -> true; -type_is_expanded(_) -> false. - -% Skip traversal if there is nothing to substitute. This will often be the -% most common case. substitute_opaque_type(Bindings, {var, VarName}) -> case lists:keyfind(VarName, 1, Bindings) of - false -> {error, invalid_aci}; - {_, TypeArg} -> {ok, TypeArg} - end; -substitute_opaque_type(Bindings, {variant, Args}) -> - case substitute_variant_types(Bindings, Args, []) of - {ok, Result} -> {ok, {variant, Result}}; - Error -> Error - end; -substitute_opaque_type(Bindings, {record, Args}) -> - case substitute_record_types(Bindings, Args, []) of - {ok, Result} -> {ok, {record, Result}}; - Error -> Error + {_, TypeArg} -> TypeArg; + % No valid ACI will create this case. Regardless, the user should + % still be able to specify arbitrary gmb FATE terms for whatever this + % is meant to be. + false -> unknown_type end; +substitute_opaque_type(Bindings, {variant, Variants}) -> + Each = fun({VariantName, Elements}) -> + NewElements = substitute_opaque_types(Bindings, Elements), + {VariantName, NewElements} + end, + NewVariants = lists:map(Each, Variants), + {variant, NewVariants}; +substitute_opaque_type(Bindings, {record, Fields}) -> + Each = fun({FieldName, FieldType}) -> + NewType = substitute_opaque_type(Bindings, FieldType), + {FieldName, NewType} + end, + NewFields = lists:map(Each, Fields), + {record, NewFields}; substitute_opaque_type(Bindings, {Connective, Args}) -> - case substitute_opaque_types(Bindings, Args, []) of - {ok, Result} -> {ok, {Connective, Result}}; - Error -> Error - end; + NewArgs = substitute_opaque_types(Bindings, Args), + {Connective, NewArgs}; substitute_opaque_type(_Bindings, Type) -> - {ok, Type}. + Type. -substitute_variant_types(Bindings, [{VariantName, Elements} | Rest], Acc) -> - case substitute_opaque_types(Bindings, Elements, []) of - {ok, Result} -> substitute_variant_types(Bindings, Rest, [{VariantName, Result} | Acc]); - Error -> Error - end; -substitute_variant_types(_Bindings, [], Acc) -> - {ok, lists:reverse(Acc)}. - -substitute_record_types(Bindings, [{ElementName, Type} | Rest], Acc) -> - case substitute_opaque_type(Bindings, Type) of - {ok, Result} -> substitute_record_types(Bindings, Rest, [{ElementName, Result} | Acc]); - Error -> Error - end; -substitute_record_types(_Bindings, [], Acc) -> - {ok, lists:reverse(Acc)}. - -substitute_opaque_types(Bindings, [Next | Rest], Acc) -> - case substitute_opaque_type(Bindings, Next) of - {ok, Result} -> substitute_opaque_types(Bindings, Rest, [Result | Acc]); - Error -> Error - end; -substitute_opaque_types(_Bindings, [], Acc) -> - {ok, lists:reverse(Acc)}. +substitute_opaque_types(Bindings, Types) -> + Each = fun(Type) -> substitute_opaque_type(Bindings, Type) end, + lists:map(Each, Types). coerce_bindings(VarTypes, Terms, Direction) -> DefLength = length(VarTypes),