aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBjörn-Egil Dahlberg <[email protected]>2016-05-03 14:20:37 +0200
committerBjörn-Egil Dahlberg <[email protected]>2016-05-03 14:20:37 +0200
commit175d11f5171a36bfdfacfe61f734a1d256a03a59 (patch)
treea94c3281e676262007d128ccda0e33a26ff5102b
parent54825baea574a20441acc40948527c74a524c4ff (diff)
parentbbb4d3f3af2ed7b836db1b038cc7d2f96794428a (diff)
downloadotp-175d11f5171a36bfdfacfe61f734a1d256a03a59.tar.gz
otp-175d11f5171a36bfdfacfe61f734a1d256a03a59.tar.bz2
otp-175d11f5171a36bfdfacfe61f734a1d256a03a59.zip
Merge branch 'egil/stdlib/fix-maps-alias/ERL-135/OTP-13534'
* egil/stdlib/fix-maps-alias/ERL-135/OTP-13534: stdlib: Strengthen map pattern tests stdlib: Add lint tests for parallel match of maps stdlib: Refactor erl_lint_SUITE stdlib: Fix linting of map key variables
-rw-r--r--lib/stdlib/src/erl_lint.erl97
-rw-r--r--lib/stdlib/test/erl_lint_SUITE.erl156
2 files changed, 162 insertions, 91 deletions
diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl
index b14102ac38..2508f96b91 100644
--- a/lib/stdlib/src/erl_lint.erl
+++ b/lib/stdlib/src/erl_lint.erl
@@ -1502,7 +1502,7 @@ pattern({op,_Line,'++',{string,_Li,_S},R}, Vt, Old, Bvt, St) ->
pattern({match,_Line,Pat1,Pat2}, Vt, Old, Bvt, St0) ->
{Lvt,Bvt1,St1} = pattern(Pat1, Vt, Old, Bvt, St0),
{Rvt,Bvt2,St2} = pattern(Pat2, Vt, Old, Bvt, St1),
- St3 = reject_bin_alias(Pat1, Pat2, St2),
+ St3 = reject_invalid_alias(Pat1, Pat2, Vt, St2),
{vtmerge_pat(Lvt, Rvt),vtmerge_pat(Bvt1,Bvt2),St3};
%% Catch legal constant expressions, including unary +,-.
pattern(Pat, _Vt, _Old, _Bvt, St) ->
@@ -1517,56 +1517,77 @@ pattern_list(Ps, Vt, Old, Bvt0, St) ->
{vtmerge_pat(Pvt, Psvt),vtmerge_pat(Bvt,Bvt1),St1}
end, {[],[],St}, Ps).
-%% reject_bin_alias(Pat, Expr, St) -> St'
+
+
+%% reject_invalid_alias(Pat, Expr, Vt, St) -> St'
%% Reject aliases for binary patterns at the top level.
+%% Reject aliases for maps patterns at the top level.
+%% The variables table (Vt) are for maps checkking.
+
+reject_invalid_alias_expr({bin,_,_}=P, {match,_,P0,E}, Vt, St0) ->
+ St = reject_invalid_alias(P, P0, Vt, St0),
+ reject_invalid_alias_expr(P, E, Vt, St);
+reject_invalid_alias_expr({map,_,_}=P, {match,_,P0,E}, Vt, St0) ->
+ St = reject_invalid_alias(P, P0, Vt, St0),
+ reject_invalid_alias_expr(P, E, Vt, St);
+reject_invalid_alias_expr({match,_,_,_}=P, {match,_,P0,E}, Vt, St0) ->
+ St = reject_invalid_alias(P, P0, Vt, St0),
+ reject_invalid_alias_expr(P, E, Vt, St);
+reject_invalid_alias_expr(_, _, _, St) -> St.
-reject_bin_alias_expr({bin,_,_}=P, {match,_,P0,E}, St0) ->
- St = reject_bin_alias(P, P0, St0),
- reject_bin_alias_expr(P, E, St);
-reject_bin_alias_expr({match,_,_,_}=P, {match,_,P0,E}, St0) ->
- St = reject_bin_alias(P, P0, St0),
- reject_bin_alias_expr(P, E, St);
-reject_bin_alias_expr(_, _, St) -> St.
-%% reject_bin_alias(Pat1, Pat2, St) -> St'
+%% reject_invalid_alias(Pat1, Pat2, St) -> St'
%% Aliases of binary patterns, such as <<A:8>> = <<B:4,C:4>> or even
%% <<A:8>> = <<A:8>>, are not allowed. Traverse the patterns in parallel
%% and generate an error if any binary aliases are found.
%% We generate an error even if is obvious that the overall pattern can't
%% possibly match, for instance, {a,<<A:8>>,c}={x,<<A:8>>} WILL generate an
%% error.
+%% Maps should reject unbound variables here.
-reject_bin_alias({bin,Line,_}, {bin,_,_}, St) ->
+reject_invalid_alias({bin,Line,_}, {bin,_,_}, _, St) ->
add_error(Line, illegal_bin_pattern, St);
-reject_bin_alias({cons,_,H1,T1}, {cons,_,H2,T2}, St0) ->
- St = reject_bin_alias(H1, H2, St0),
- reject_bin_alias(T1, T2, St);
-reject_bin_alias({tuple,_,Es1}, {tuple,_,Es2}, St) ->
- reject_bin_alias_list(Es1, Es2, St);
-reject_bin_alias({record,_,Name1,Pfs1}, {record,_,Name2,Pfs2},
+reject_invalid_alias({map,_Line,Ps1}, {map,_,Ps2}, Vt, St0) ->
+ Fun = fun ({map_field_exact,L,{var,_,K},_V}, Sti) ->
+ case is_var_bound(K,Vt) of
+ true ->
+ Sti;
+ false ->
+ add_error(L, {unbound_var,K}, Sti)
+ end;
+ ({map_field_exact,_L,_K,_V}, Sti) ->
+ Sti
+ end,
+ foldl(Fun, foldl(Fun, St0, Ps1), Ps2);
+reject_invalid_alias({cons,_,H1,T1}, {cons,_,H2,T2}, Vt, St0) ->
+ St = reject_invalid_alias(H1, H2, Vt, St0),
+ reject_invalid_alias(T1, T2, Vt, St);
+reject_invalid_alias({tuple,_,Es1}, {tuple,_,Es2}, Vt, St) ->
+ reject_invalid_alias_list(Es1, Es2, Vt, St);
+reject_invalid_alias({record,_,Name1,Pfs1}, {record,_,Name2,Pfs2}, Vt,
#lint{records=Recs}=St) ->
case {dict:find(Name1, Recs),dict:find(Name2, Recs)} of
{{ok,{_Line1,Fields1}},{ok,{_Line2,Fields2}}} ->
- reject_bin_alias_rec(Pfs1, Pfs2, Fields1, Fields2, St);
+ reject_invalid_alias_rec(Pfs1, Pfs2, Fields1, Fields2, Vt, St);
{_,_} ->
%% One or more non-existing records. (An error messages has
%% already been generated, so we are done here.)
St
end;
-reject_bin_alias({match,_,P1,P2}, P, St0) ->
- St = reject_bin_alias(P1, P, St0),
- reject_bin_alias(P2, P, St);
-reject_bin_alias(P, {match,_,_,_}=M, St) ->
- reject_bin_alias(M, P, St);
-reject_bin_alias(_P1, _P2, St) -> St.
-
-reject_bin_alias_list([E1|Es1], [E2|Es2], St0) ->
- St = reject_bin_alias(E1, E2, St0),
- reject_bin_alias_list(Es1, Es2, St);
-reject_bin_alias_list(_, _, St) -> St.
-
-reject_bin_alias_rec(PfsA0, PfsB0, FieldsA0, FieldsB0, St) ->
+reject_invalid_alias({match,_,P1,P2}, P, Vt, St0) ->
+ St = reject_invalid_alias(P1, P, Vt, St0),
+ reject_invalid_alias(P2, P, Vt, St);
+reject_invalid_alias(P, {match,_,_,_}=M, Vt, St) ->
+ reject_invalid_alias(M, P, Vt, St);
+reject_invalid_alias(_P1, _P2, _Vt, St) -> St.
+
+reject_invalid_alias_list([E1|Es1], [E2|Es2], Vt, St0) ->
+ St = reject_invalid_alias(E1, E2, Vt, St0),
+ reject_invalid_alias_list(Es1, Es2, Vt, St);
+reject_invalid_alias_list(_, _, _, St) -> St.
+
+reject_invalid_alias_rec(PfsA0, PfsB0, FieldsA0, FieldsB0, Vt, St) ->
%% We treat records as if they have been converted to tuples.
PfsA1 = rbia_field_vars(PfsA0),
PfsB1 = rbia_field_vars(PfsB0),
@@ -1582,7 +1603,7 @@ reject_bin_alias_rec(PfsA0, PfsB0, FieldsA0, FieldsB0, St) ->
D = sofs:projection({external,fun({_,_,P1,_,P2}) -> {P1,P2} end}, C),
E = sofs:to_external(D),
{Ps1,Ps2} = lists:unzip(E),
- reject_bin_alias_list(Ps1, Ps2, St).
+ reject_invalid_alias_list(Ps1, Ps2, Vt, St).
rbia_field_vars(Fs) ->
[{Name,Pat} || {record_field,_,{atom,_,Name},Pat} <- Fs].
@@ -2284,7 +2305,7 @@ expr({'catch',Line,E}, Vt, St0) ->
expr({match,_Line,P,E}, Vt, St0) ->
{Evt,St1} = expr(E, Vt, St0),
{Pvt,Bvt,St2} = pattern(P, vtupdate(Evt, Vt), St1),
- St = reject_bin_alias_expr(P, E, St2),
+ St = reject_invalid_alias_expr(P, E, Vt, St2),
{vtupdate(Bvt, vtmerge(Evt, Pvt)),St};
%% No comparison or boolean operators yet.
expr({op,_Line,_Op,A}, Vt, St) ->
@@ -2381,7 +2402,7 @@ is_valid_call(Call) ->
_ -> true
end.
-%% is_valid_map_key(K,St) -> true | false
+%% is_valid_map_key(K) -> true | false
%% variables are allowed for patterns only at the top of the tree
is_valid_map_key({var,_,_}) -> true;
@@ -3413,6 +3434,14 @@ warn_unused_vars(U, Vt, St0) ->
UVt = map(fun ({V,{State,_,Ls}}) -> {V,{State,used,Ls}} end, U),
{vtmerge(Vt, UVt), St1}.
+
+is_var_bound(V, Vt) ->
+ case orddict:find(V, Vt) of
+ {ok,{bound,_Usage,_}} -> true;
+ _ -> false
+ end.
+
+
%% vtupdate(UpdVarTable, VarTable) -> VarTable.
%% Add the variables in the updated vartable to VarTable. The variables
%% will be updated with their property in UpdVarTable. The state of
diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl
index 6fea198af3..b0214e5238 100644
--- a/lib/stdlib/test/erl_lint_SUITE.erl
+++ b/lib/stdlib/test/erl_lint_SUITE.erl
@@ -33,47 +33,38 @@
-define(privdir, proplists:get_value(priv_dir, Conf)).
-endif.
--export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1,
- init_per_group/2,end_per_group/2,
- init_per_testcase/2, end_per_testcase/2]).
-
--export([
- unused_vars_warn_basic/1,
- unused_vars_warn_lc/1,
- unused_vars_warn_rec/1,
- unused_vars_warn_fun/1,
- unused_vars_OTP_4858/1,
- unused_unsafe_vars_warn/1,
- export_vars_warn/1,
- shadow_vars/1,
- unused_import/1,
- unused_function/1,
- unsafe_vars/1,unsafe_vars2/1,
- unsafe_vars_try/1,
- unsized_binary_in_bin_gen_pattern/1,
- guard/1, otp_4886/1, otp_4988/1, otp_5091/1, otp_5276/1, otp_5338/1,
- otp_5362/1, otp_5371/1, otp_7227/1, otp_5494/1, otp_5644/1, otp_5878/1,
- otp_5917/1, otp_6585/1, otp_6885/1, otp_10436/1, otp_11254/1,
- otp_11772/1, otp_11771/1, otp_11872/1,
- export_all/1,
- bif_clash/1,
- behaviour_basic/1, behaviour_multiple/1, otp_11861/1,
- otp_7550/1,
- otp_8051/1,
- format_warn/1,
- on_load_successful/1, on_load_failing/1,
- too_many_arguments/1,
- basic_errors/1,bin_syntax_errors/1,
- predef/1,
- maps/1,maps_type/1,otp_11851/1,otp_11879/1,otp_13230/1,
- record_errors/1
- ]).
-
-init_per_testcase(_Case, Config) ->
- Config.
-
-end_per_testcase(_Case, _Config) ->
- ok.
+-export([all/0, suite/0, groups/0]).
+
+-export([unused_vars_warn_basic/1,
+ unused_vars_warn_lc/1,
+ unused_vars_warn_rec/1,
+ unused_vars_warn_fun/1,
+ unused_vars_OTP_4858/1,
+ unused_unsafe_vars_warn/1,
+ export_vars_warn/1,
+ shadow_vars/1,
+ unused_import/1,
+ unused_function/1,
+ unsafe_vars/1,unsafe_vars2/1,
+ unsafe_vars_try/1,
+ unsized_binary_in_bin_gen_pattern/1,
+ guard/1, otp_4886/1, otp_4988/1, otp_5091/1, otp_5276/1, otp_5338/1,
+ otp_5362/1, otp_5371/1, otp_7227/1, otp_5494/1, otp_5644/1, otp_5878/1,
+ otp_5917/1, otp_6585/1, otp_6885/1, otp_10436/1, otp_11254/1,
+ otp_11772/1, otp_11771/1, otp_11872/1,
+ export_all/1,
+ bif_clash/1,
+ behaviour_basic/1, behaviour_multiple/1, otp_11861/1,
+ otp_7550/1,
+ otp_8051/1,
+ format_warn/1,
+ on_load_successful/1, on_load_failing/1,
+ too_many_arguments/1,
+ basic_errors/1,bin_syntax_errors/1,
+ predef/1,
+ maps/1,maps_type/1,maps_parallel_match/1,
+ otp_11851/1,otp_11879/1,otp_13230/1,
+ record_errors/1]).
suite() ->
[{ct_hooks,[ts_install_cth]},
@@ -91,7 +82,8 @@ all() ->
bif_clash, behaviour_basic, behaviour_multiple, otp_11861,
otp_7550, otp_8051, format_warn, {group, on_load},
too_many_arguments, basic_errors, bin_syntax_errors, predef,
- maps, maps_type, otp_11851, otp_11879, otp_13230,
+ maps, maps_type, maps_parallel_match,
+ otp_11851, otp_11879, otp_13230,
record_errors].
groups() ->
@@ -101,19 +93,6 @@ groups() ->
unused_vars_OTP_4858, unused_unsafe_vars_warn]},
{on_load, [], [on_load_successful, on_load_failing]}].
-init_per_suite(Config) ->
- Config.
-
-end_per_suite(_Config) ->
- ok.
-
-init_per_group(_GroupName, Config) ->
- Config.
-
-end_per_group(_GroupName, Config) ->
- Config.
-
-
%% Warnings for unused variables in some simple cases.
unused_vars_warn_basic(Config) when is_list(Config) ->
@@ -3583,8 +3562,6 @@ predef(Config) when is_list(Config) ->
ok.
maps(Config) ->
- %% TODO: test key patterns, not done because map patterns are going to be
- %% changed a lot.
Ts = [{illegal_map_construction,
<<"t() ->
#{ a := b,
@@ -3626,6 +3603,26 @@ maps(Config) ->
{errors,[{1,erl_lint,illegal_map_construction},
{1,erl_lint,{unbound_var,'X'}}],
[]}},
+ {legal_map_pattern,
+ <<"
+ -record(mapkey, {a=1,b=2}).
+ t(M,K1) ->
+ #{ a := 1,
+ $a := 1, $z := 99,
+ #{a=>val} := 2,
+ K1 := 1337,
+ #mapkey{a = 10} := wat,
+ #{{a,val}=>val} := 2,
+ #{ \"hi\" => wazzup, hi => ho } := yep,
+ ok := 1.0,
+ [3+3] := nope,
+ 1.0 := yep,
+ {3.0+3} := nope,
+ {yep} := yep
+ } = M.
+ ">>,
+ [],
+ []},
{legal_map_construction,
<<"t(V) -> #{ a => 1,
#{a=>V} => 2,
@@ -3692,6 +3689,51 @@ maps_type(Config) when is_list(Config) ->
[] = run(Config, Ts),
ok.
+maps_parallel_match(Config) when is_list(Config) ->
+ Ts = [{parallel_map_patterns_unbound1,
+ <<"
+ t(#{} = M) ->
+ #{K := V} = #{k := K} = M,
+ V.
+ ">>,
+ [],
+ {errors,[{3,erl_lint,{unbound_var,'K'}}],[]}},
+ {parallel_map_patterns_unbound2,
+ <<"
+ t(#{} = M) ->
+ #{K1 := V1} =
+ #{K2 := V2} =
+ #{k1 := K1,k2 := K2} = M,
+ [V1,V2].
+ ">>,
+ [],
+ {errors,[{3,erl_lint,{unbound_var,'K1'}},
+ {3,erl_lint,{unbound_var,'K1'}},
+ {4,erl_lint,{unbound_var,'K2'}},
+ {4,erl_lint,{unbound_var,'K2'}}],[]}},
+ {parallel_map_patterns_bound,
+ <<"
+ t(#{} = M,K1,K2) ->
+ #{K1 := V1} =
+ #{K2 := V2} =
+ #{k1 := K1,k2 := K2} = M,
+ [V1,V2].
+ ">>,
+ [],
+ []},
+ {parallel_map_patterns_literal,
+ <<"
+ t(#{} = M) ->
+ #{k1 := V1} =
+ #{k2 := V2} =
+ #{k1 := V1,k2 := V2} = M,
+ [V1,V2].
+ ">>,
+ [],
+ []}],
+ [] = run(Config, Ts),
+ ok.
+
%% OTP-11851: More atoms can be used as type names + bug fixes.
otp_11851(Config) when is_list(Config) ->
Ts = [