Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:Ledest:erlang:25
erlang
1751-maps-raise-a-badmap-error-if-an-iterator-f...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 1751-maps-raise-a-badmap-error-if-an-iterator-fails-later.patch of Package erlang
From e913d16df30ecb08844c3e63ed5d4132baf38254 Mon Sep 17 00:00:00 2001 From: Maria Scott <maria-12648430@hnc-agency.org> Date: Thu, 2 Feb 2023 11:59:22 +0100 Subject: [PATCH] maps: raise a badmap error if an iterator fails later The implementation of filter/2, filtermap/2, fold/3, foreach/2 and map/2 raises a badmap exception if and only if the given iterator is not an iterator at all, ie when the first call to next/1 on it fails. If a later subsequent call to next/1 fails (ie, if something like {x, y, z} is given as an iterator), the result will be a badarg error instead. The changes in this commit change this behavior so that a badmap error is always raised in such circumstances, that is, even if the given iterator fails only on later subsequent calls. Likewise, the stdlib error reporting did not complain about an iterator being faulty if the first call to maps:next/1 on it succeeded. This has also been addressed by the changes in this commit. --- lib/stdlib/src/erl_stdlib_errors.erl | 8 +- lib/stdlib/src/maps.erl | 174 +++++++++++++++------------ lib/stdlib/test/maps_SUITE.erl | 74 +++++++++++- 3 files changed, 171 insertions(+), 85 deletions(-) diff --git a/lib/stdlib/src/erl_stdlib_errors.erl b/lib/stdlib/src/erl_stdlib_errors.erl index 2d2fe03d29..37e86c9e7b 100644 --- a/lib/stdlib/src/erl_stdlib_errors.erl +++ b/lib/stdlib/src/erl_stdlib_errors.erl @@ -982,11 +982,9 @@ must_be_map_iterator_order(_) -> must_be_map_or_iter(Map) when is_map(Map) -> []; must_be_map_or_iter(Iter) -> - try maps:next(Iter) of - _ -> [] - catch - error:_ -> - not_map_or_iterator + case maps:is_iterator_valid(Iter) of + true -> []; + false -> not_map_or_iterator end. must_be_number(N) -> diff --git a/lib/stdlib/src/maps.erl b/lib/stdlib/src/maps.erl index 98bff5c12d..12d6b47c8b 100644 --- a/lib/stdlib/src/maps.erl +++ b/lib/stdlib/src/maps.erl @@ -30,6 +30,9 @@ merge_with/3, groups_from_list/2, groups_from_list/3]). +%% Internal +-export([is_iterator_valid/1]). + %% BIFs -export([get/2, find/2, from_list/1, from_keys/2, is_key/2, keys/1, merge/2, @@ -313,31 +316,28 @@ get(Key,Map,Default) -> MapOrIter :: #{Key => Value} | iterator(Key, Value), Map :: #{Key => Value}. -filter(Pred, MapOrIter) when is_function(Pred, 2) -> - Iter = if - is_map(MapOrIter) -> - iterator(MapOrIter); - true -> - MapOrIter - end, - try next(Iter) of - Next -> - maps:from_list(filter_1(Pred, Next)) +filter(Pred, Map) when is_map(Map), is_function(Pred, 2) -> + maps:from_list(filter_1(Pred, next(iterator(Map)), undefined)); +filter(Pred, Iter) when is_function(Pred, 2) -> + ErrorTag = make_ref(), + try filter_1(Pred, try_next(Iter, ErrorTag), ErrorTag) of + Result -> + maps:from_list(Result) catch - error:_ -> - error_with_info({badmap,MapOrIter}, [Pred, MapOrIter]) + error:ErrorTag -> + error_with_info({badmap, Iter}, [Pred, Iter]) end; filter(Pred, Map) -> badarg_with_info([Pred, Map]). -filter_1(Pred, {K, V, Iter}) -> +filter_1(Pred, {K, V, Iter}, ErrorTag) -> case Pred(K, V) of true -> - [{K,V} | filter_1(Pred, next(Iter))]; + [{K,V} | filter_1(Pred, try_next(Iter, ErrorTag), ErrorTag)]; false -> - filter_1(Pred, next(Iter)) + filter_1(Pred, try_next(Iter, ErrorTag), ErrorTag) end; -filter_1(_Pred, none) -> +filter_1(_Pred, none, _ErrorTag) -> []. -spec filtermap(Fun, MapOrIter) -> Map when @@ -345,57 +345,52 @@ filter_1(_Pred, none) -> MapOrIter :: #{Key => Value1} | iterator(Key, Value1), Map :: #{Key => Value1 | Value2}. -filtermap(Fun, MapOrIter) when is_function(Fun, 2) -> - Iter = if - is_map(MapOrIter) -> - iterator(MapOrIter); - true -> - MapOrIter - end, - try next(Iter) of - Next -> - maps:from_list(filtermap_1(Fun, Next)) +filtermap(Fun, Map) when is_map(Map), is_function(Fun, 2) -> + maps:from_list(filtermap_1(Fun, next(iterator(Map)), undefined)); +filtermap(Fun, Iter) when is_function(Fun, 2) -> + ErrorTag = make_ref(), + try filtermap_1(Fun, try_next(Iter, ErrorTag), ErrorTag) of + Result -> + maps:from_list(Result) catch - error:_ -> - error_with_info({badmap,MapOrIter}, [Fun, MapOrIter]) + error:ErrorTag -> + error_with_info({badmap, Iter}, [Fun, Iter]) end; filtermap(Fun, Map) -> badarg_with_info([Fun, Map]). -filtermap_1(Pred, {K, V, Iter}) -> - case Pred(K, V) of +filtermap_1(Fun, {K, V, Iter}, ErrorTag) -> + case Fun(K, V) of true -> - [{K, V} | filtermap_1(Pred, next(Iter))]; + [{K, V} | filtermap_1(Fun, try_next(Iter, ErrorTag), ErrorTag)]; {true, NewV} -> - [{K, NewV} | filtermap_1(Pred, next(Iter))]; + [{K, NewV} | filtermap_1(Fun, try_next(Iter, ErrorTag), ErrorTag)]; false -> - filtermap_1(Pred, next(Iter)) + filtermap_1(Fun, try_next(Iter, ErrorTag), ErrorTag) end; -filtermap_1(_Pred, none) -> +filtermap_1(_Fun, none, _ErrorTag) -> []. -spec foreach(Fun,MapOrIter) -> ok when Fun :: fun((Key, Value) -> term()), MapOrIter :: #{Key => Value} | iterator(Key, Value). -foreach(Fun, MapOrIter) when is_function(Fun, 2) -> - Iter = if is_map(MapOrIter) -> iterator(MapOrIter); - true -> MapOrIter - end, - try next(Iter) of - Next -> - foreach_1(Fun, Next) +foreach(Fun, Map) when is_map(Map), is_function(Fun, 2) -> + foreach_1(Fun, next(iterator(Map)), undefined); +foreach(Fun, Iter) when is_function(Fun, 2) -> + ErrorTag = make_ref(), + try foreach_1(Fun, try_next(Iter, ErrorTag), ErrorTag) catch - error:_ -> - error_with_info({badmap, MapOrIter}, [Fun, MapOrIter]) + error:ErrorTag -> + error_with_info({badmap, Iter}, [Fun, Iter]) end; -foreach(Pred, Map) -> - badarg_with_info([Pred, Map]). +foreach(Fun, Map) -> + badarg_with_info([Fun, Map]). -foreach_1(Fun, {K, V, Iter}) -> +foreach_1(Fun, {K, V, Iter}, ErrorTag) -> Fun(K,V), - foreach_1(Fun, next(Iter)); -foreach_1(_Fun, none) -> + foreach_1(Fun, try_next(Iter, ErrorTag), ErrorTag); +foreach_1(_Fun, none, _ErrorTag) -> ok. -spec fold(Fun,Init,MapOrIter) -> Acc when @@ -405,26 +400,21 @@ foreach_1(_Fun, none) -> AccIn :: Init | AccOut, MapOrIter :: #{Key => Value} | iterator(Key, Value). -fold(Fun, Init, MapOrIter) when is_function(Fun, 3) -> - Iter = if - is_map(MapOrIter) -> - iterator(MapOrIter); - true -> - MapOrIter - end, - try next(Iter) of - Next -> - fold_1(Fun, Init, Next) +fold(Fun, Init, Map) when is_map(Map), is_function(Fun, 3) -> + fold_1(Fun, Init, next(iterator(Map)), undefined); +fold(Fun, Init, Iter) when is_function(Fun, 3) -> + ErrorTag = make_ref(), + try fold_1(Fun, Init, try_next(Iter, ErrorTag), ErrorTag) catch - error:_ -> - error_with_info({badmap,MapOrIter}, [Fun, Init, MapOrIter]) + error:ErrorTag -> + error_with_info({badmap, Iter}, [Fun, Init, Iter]) end; fold(Fun, Init, Map) -> badarg_with_info([Fun, Init, Map]). -fold_1(Fun, Acc, {K, V, Iter}) -> - fold_1(Fun, Fun(K,V,Acc), next(Iter)); -fold_1(_Fun, Acc, none) -> +fold_1(Fun, Acc, {K, V, Iter}, ErrorTag) -> + fold_1(Fun, Fun(K,V,Acc), try_next(Iter, ErrorTag), ErrorTag); +fold_1(_Fun, Acc, none, _ErrorTag) -> Acc. -spec map(Fun,MapOrIter) -> Map when @@ -432,27 +422,24 @@ fold_1(_Fun, Acc, none) -> MapOrIter :: #{Key => Value1} | iterator(Key, Value1), Map :: #{Key => Value2}. -map(Fun, MapOrIter) when is_function(Fun, 2) -> - Iter = if - is_map(MapOrIter) -> - iterator(MapOrIter); - true -> - MapOrIter - end, - try next(Iter) of - Next -> - maps:from_list(map_1(Fun, Next)) +map(Fun, Map) when is_map(Map), is_function(Fun, 2) -> + maps:from_list(map_1(Fun, next(iterator(Map)), undefined)); +map(Fun, Iter) when is_function(Fun, 2) -> + ErrorTag = make_ref(), + try map_1(Fun, try_next(Iter, ErrorTag), ErrorTag) of + Result -> + maps:from_list(Result) catch - error:_ -> - error_with_info({badmap,MapOrIter}, [Fun, MapOrIter]) + error:ErrorTag -> + error_with_info({badmap, Iter}, [Fun, Iter]) end; map(Fun, Map) -> badarg_with_info([Fun, Map]). -map_1(Fun, {K, V, Iter}) -> - [{K, Fun(K, V)} | map_1(Fun, next(Iter))]; -map_1(_Fun, none) -> +map_1(Fun, {K, V, Iter}, ErrorTag) -> + [{K, Fun(K, V)} | map_1(Fun, try_next(Iter, ErrorTag), ErrorTag)]; +map_1(_Fun, none, _ErrorTag) -> []. -spec size(Map) -> non_neg_integer() when @@ -622,3 +609,34 @@ badarg_with_info(Args) -> error_with_info(Reason, Args) -> erlang:error(Reason, Args, [{error_info, #{module => erl_stdlib_errors}}]). + +-spec is_iterator_valid(MaybeIter) -> boolean() when + MaybeIter :: iterator() | term(). + +is_iterator_valid(Iter) -> + try is_iterator_valid_1(Iter) + catch + error:badarg -> + false + end. + +is_iterator_valid_1(none) -> + true; +is_iterator_valid_1({_, _, Next}) -> + is_iterator_valid_1(next(Next)); +is_iterator_valid_1(Iter) -> + _ = next(Iter), + true. + +try_next({_, _, _} = KVI, _ErrorTag) -> + KVI; +try_next(none, _ErrorTag) -> + none; +try_next(Iter, undefined) -> + next(Iter); +try_next(Iter, ErrorTag) -> + try next(Iter) + catch + error:badarg -> + error(ErrorTag) + end. diff --git a/lib/stdlib/test/maps_SUITE.erl b/lib/stdlib/test/maps_SUITE.erl index 7ab669c155..0ed0a59192 100644 --- a/lib/stdlib/test/maps_SUITE.erl +++ b/lib/stdlib/test/maps_SUITE.erl @@ -31,7 +31,9 @@ -export([t_update_with_3/1, t_update_with_4/1, t_get_3/1, t_filter_2/1, t_filtermap_2/1, t_fold_3/1,t_map_2/1,t_size_1/1, t_foreach_2/1, - t_iterator_1/1, t_put_opt/1, t_merge_opt/1, + t_iterator_1/1, + t_iterator_valid/1, + t_put_opt/1, t_merge_opt/1, t_with_2/1,t_without_2/1, t_intersect/1, t_intersect_with/1, t_merge_with/1, t_from_keys/1, @@ -59,7 +60,9 @@ all() -> [t_update_with_3,t_update_with_4, t_get_3,t_filter_2,t_filtermap_2, t_fold_3,t_map_2,t_size_1,t_foreach_2, - t_iterator_1,t_put_opt,t_merge_opt, + t_iterator_1, + t_iterator_valid, + t_put_opt,t_merge_opt, t_with_2,t_without_2, t_intersect, t_intersect_with, t_merge_with, t_from_keys, @@ -396,7 +398,8 @@ t_filter_2(Config) when is_list(Config) -> #{a := 2,c := 4} = maps:filter(Pred1,maps:iterator(M)), #{"b" := 2,"c" := 4} = maps:filter(Pred2,maps:iterator(M)), %% error case - ?badmap(a,filter,[_,a]) = (catch maps:filter(fun(_,_) -> ok end,id(a))), + ?badmap(a,filter,[_,a]) = (catch maps:filter(fun(_,_) -> true end,id(a))), + ?badmap({a,b,c},filter,[_,{a,b,c}]) = (catch maps:filter(fun(_,_) -> true end,id({a,b,c}))), ?badarg(filter,[<<>>,#{}]) = (catch maps:filter(id(<<>>),#{})), ok. @@ -410,7 +413,8 @@ t_filtermap_2(Config) when is_list(Config) -> false = maps:is_key(20, M1), true = M1 =:= M2, %% error case - ?badmap(a,filtermap,[_,a]) = (catch maps:filtermap(fun(_,_) -> ok end,id(a))), + ?badmap(a,filtermap,[_,a]) = (catch maps:filtermap(fun(_,_) -> true end,id(a))), + ?badmap({a,b,c},filtermap,[_,{a,b,c}]) = (catch maps:filtermap(fun(_,_) -> true end,id({a,b,c}))), ?badarg(filtermap,[<<>>,#{}]) = (catch maps:filtermap(id(<<>>),#{})), ok. @@ -426,6 +430,7 @@ t_fold_3(Config) when is_list(Config) -> %% error case ?badmap(a,fold,[_,0,a]) = (catch maps:fold(fun(_,_,_) -> ok end,0,id(a))), + ?badmap({a,b,c},fold,[_,0,{a,b,c}]) = (catch maps:fold(fun(_,_,_) -> ok end,0,id({a,b,c}))), ?badarg(fold,[<<>>,0,#{}]) = (catch maps:fold(id(<<>>),0,#{})), ok. @@ -440,6 +445,7 @@ t_map_2(Config) when is_list(Config) -> %% error case ?badmap(a,map,[_,a]) = (catch maps:map(fun(_,_) -> ok end, id(a))), + ?badmap({a,b,c},map,[_,{a,b,c}]) = (catch maps:map(fun(_,_) -> ok end, id({a,b,c}))), ?badarg(map,[<<>>,#{}]) = (catch maps:map(id(<<>>),#{})), ok. @@ -450,6 +456,7 @@ t_foreach_2(Config) when is_list(Config) -> ?badmap({},foreach,[_,{}]) = (catch maps:foreach(fun(_,_) -> ok end, id({}))), ?badmap(42,foreach,[_,42]) = (catch maps:foreach(fun(_,_) -> ok end, id(42))), ?badmap(<<>>,foreach,[_,<<>>]) = (catch maps:foreach(fun(_,_) -> ok end, id(<<>>))), + ?badmap({a,b,c},foreach,[_,{a,b,c}]) = (catch maps:foreach(fun(_,_) -> ok end, id({a,b,c}))), ?badarg(foreach,[<<>>,#{}]) = (catch maps:foreach(id(<<>>),#{})), F0 = fun() -> ok end, @@ -593,6 +600,59 @@ iter_kv(I) -> [{K,V} | iter_kv(NI)] end. +t_iterator_valid(Config) when is_list(Config) -> + %% good iterators created via maps:iterator + true = maps:is_iterator_valid(maps:iterator(#{})), + true = maps:is_iterator_valid(maps:iterator(#{a => b})), + true = maps:is_iterator_valid(maps:iterator(#{a => b, c => d})), + true = maps:is_iterator_valid(maps:iterator(#{}, undefined)), + true = maps:is_iterator_valid(maps:iterator(#{a => b}, undefined)), + true = maps:is_iterator_valid(maps:iterator(#{a => b, c => d}, undefined)), + true = maps:is_iterator_valid(maps:iterator(#{}, ordered)), + true = maps:is_iterator_valid(maps:iterator(#{a => b}, ordered)), + true = maps:is_iterator_valid(maps:iterator(#{a => b, c => d}, ordered)), + true = maps:is_iterator_valid(maps:iterator(#{}, reversed)), + true = maps:is_iterator_valid(maps:iterator(#{a => b}, reversed)), + true = maps:is_iterator_valid(maps:iterator(#{a => b, c => d}, reversed)), + true = maps:is_iterator_valid(maps:iterator(#{}, fun erlang:'=<'/2)), + true = maps:is_iterator_valid(maps:iterator(#{a => b}, fun erlang:'=<'/2)), + true = maps:is_iterator_valid(maps:iterator(#{a => b, c => d}, fun erlang:'=<'/2)), + + %% good makeshift iterators + true = maps:is_iterator_valid(none), + true = maps:is_iterator_valid({a, b, none}), + true = maps:is_iterator_valid({a, b, {c, d, none}}), + true = maps:is_iterator_valid({a, b, {c, d, {e, f, none}}}), + true = maps:is_iterator_valid({a, b, maps:iterator(#{})}), + true = maps:is_iterator_valid({a, b, maps:iterator(#{c => d})}), + + %% not iterators + false = maps:is_iterator_valid(no_iter), + false = maps:is_iterator_valid(1), + false = maps:is_iterator_valid(1.0), + false = maps:is_iterator_valid([]), + false = maps:is_iterator_valid("foo"), + false = maps:is_iterator_valid(<<"foo">>), + false = maps:is_iterator_valid(fun() -> ok end), + false = maps:is_iterator_valid(self()), + false = maps:is_iterator_valid(make_ref()), + false = maps:is_iterator_valid(#{}), + false = maps:is_iterator_valid({}), + false = maps:is_iterator_valid({a}), + false = maps:is_iterator_valid({a, b}), + false = maps:is_iterator_valid({a, b, c, d}), + + %% bad makeshift iterators that only fail on later (ie, subsequent) calls to maps:next/1 + %% maps:next({a, b, c}) -> {a, b, c} + %% maps:next(c) -> badarg + false = maps:is_iterator_valid({a, b, c}), + %% maps:next({a, b, {c, d, e}}) -> {a, b, {c, d, e}} + %% maps:next({c, d, e}) -> {c, d, e} + %% maps:next(e) -> badarg + false = maps:is_iterator_valid({a, b, {c, d, e}}), + + ok. + t_put_opt(Config) when is_list(Config) -> Value = id(#{complex => map}), Small = id(#{a => Value}), @@ -890,11 +950,14 @@ error_info(_Config) -> L = [{filter, [fun(_, _) -> true end, abc]}, {filter, [fun(_, _) -> true end, BadIterator]}, + {filter, [fun(_, _) -> true end, BadIterator2]}, {filter, [bad_fun, BadIterator],[{1,".*"},{2,".*"}]}, + {filter, [bad_fun, BadIterator2],[{1,".*"},{2,".*"}]}, {filter, [bad_fun, GoodIterator]}, {filtermap, [fun(_, _) -> true end, abc]}, {filtermap, [fun(_, _) -> true end, BadIterator]}, + {filtermap, [fun(_, _) -> true end, BadIterator2]}, {filtermap, [fun(_) -> true end, GoodIterator]}, {filtermap, [fun(_) -> ok end, #{}]}, @@ -902,11 +965,13 @@ error_info(_Config) -> {fold, [fun(_, _, _) -> true end, init, abc]}, {fold, [fun(_, _, _) -> true end, init, BadIterator]}, + {fold, [fun(_, _, _) -> true end, init, BadIterator2]}, {fold, [fun(_) -> true end, init, GoodIterator]}, {fold, [fun(_) -> ok end, init, #{}]}, {foreach, [fun(_, _) -> ok end, no_map]}, {foreach, [fun(_, _) -> ok end, BadIterator]}, + {foreach, [fun(_, _) -> ok end, BadIterator2]}, {foreach, [fun(_) -> ok end, GoodIterator]}, {foreach, [fun(_) -> ok end, #{}]}, @@ -936,6 +1001,10 @@ error_info(_Config) -> {intersect_with, [fun(_, _, _) -> ok end, x, y],[{2,".*"},{3,".*"}]}, {intersect_with, [fun(_, _) -> ok end, #{}, #{}]}, + {is_iterator_valid, [GoodIterator], [no_fail]}, + {is_iterator_valid, [BadIterator], [no_fail]}, + {is_iterator_valid, [BadIterator2], [no_fail]}, + {is_key,[key, no_map]}, {iterator,[{no,map}]}, @@ -951,6 +1020,7 @@ error_info(_Config) -> {map, [fun(_, _) -> true end, abc]}, {map, [fun(_, _) -> true end, BadIterator]}, + {map, [fun(_, _) -> true end, BadIterator2]}, {map, [fun(_) -> true end, GoodIterator]}, {map, [fun(_) -> ok end, #{}]}, -- 2.35.3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
