From f846bf70b0c97ce66f29b0ff88a50316924bf34e Mon Sep 17 00:00:00 2001
From: Anthony Ramine <n.oxyde@gmail.com>
Date: Thu, 6 Jun 2013 01:34:25 +0200
Subject: Fix variable usage tracking in erl_lint
When analyzing complex expressions (i.e. comprehensions, cases, tries,
ifs and receives), erl_lint does not forget about old unused variables
when returning the updated variable table. This causes a bug where old
unused variables are not recorded as such:
t(X, Y) ->
#r{a=[ K || K <- Y ],b=[ K || K <- Y ]}.
As erl_lint uses vtmerge_pat/2 to merge the results of the analysis of
the two list comprehensions, X is marked as used and the warning is not
emitted.
The function vtmerge_pat/2 is used instead of the similar vtmerge/2
which does not mark multiple occurrences of a variable as usage to
handle cases like the following one:
t(X, Y) ->
#r{a=A=X,b=A=Y}.
Other simpler expressions like conses, tuples and external fun
references do not correctly follow this behaviour, e.g. A is not marked
as used in the following code:
t(X, Y) ->
{A=X,A=Y}.
This commit fixes both issues and makes erl_lint not return old unused
variables in updated tables and makes all compound expressions use
vtmerge_pat/2.
Reported-by: Anders Ramsell
---
lib/stdlib/src/erl_lint.erl | 42 ++++++++++++++++++++++----------------
lib/stdlib/test/erl_lint_SUITE.erl | 27 ++++++++++++++++++++++--
2 files changed, 49 insertions(+), 20 deletions(-)
diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl
index 8f07750b9b..9284f08b30 100644
--- a/lib/stdlib/src/erl_lint.erl
+++ b/lib/stdlib/src/erl_lint.erl
@@ -1953,12 +1953,10 @@ expr({string,_Line,_S}, _Vt, St) -> {[],St};
expr({nil,_Line}, _Vt, St) -> {[],St};
expr({cons,_Line,H,T}, Vt, St) ->
expr_list([H,T], Vt, St);
-expr({lc,_Line,E,Qs}, Vt0, St0) ->
- {Vt,St} = handle_comprehension(E, Qs, Vt0, St0),
- {vtold(Vt, Vt0),St}; %Don't export local variables
-expr({bc,_Line,E,Qs}, Vt0, St0) ->
- {Vt,St} = handle_comprehension(E, Qs, Vt0, St0),
- {vtold(Vt,Vt0),St}; %Don't export local variables
+expr({lc,_Line,E,Qs}, Vt, St) ->
+ handle_comprehension(E, Qs, Vt, St);
+expr({bc,_Line,E,Qs}, Vt, St) ->
+ handle_comprehension(E, Qs, Vt, St);
expr({tuple,_Line,Es}, Vt, St) ->
expr_list(Es, Vt, St);
expr({record_index,Line,Name,Field}, _Vt, St) ->
@@ -2012,8 +2010,7 @@ expr({'fun',Line,Body}, Vt, St) ->
%%No one can think funs export!
case Body of
{clauses,Cs} ->
- {Bvt, St1} = fun_clauses(Cs, Vt, St),
- {vtupdate(Bvt, Vt), St1};
+ fun_clauses(Cs, Vt, St);
{function,F,A} ->
%% BifClash - Fun expression
%% N.B. Only allows BIFs here as well, NO IMPORTS!!
@@ -2111,12 +2108,12 @@ expr({'try',Line,Es,Scs,Ccs,As}, Vt, St0) ->
{Evt0,St1} = exprs(Es, Vt, St0),
TryLine = {'try',Line},
Uvt = vtunsafe(vtnames(vtnew(Evt0, Vt)), TryLine, []),
- Evt1 = vtupdate(Uvt, vtupdate(Evt0, Vt)),
- {Sccs,St2} = icrt_clauses(Scs++Ccs, TryLine, Evt1, St1),
+ Evt1 = vtupdate(Uvt, Evt0),
+ {Sccs,St2} = icrt_clauses(Scs++Ccs, TryLine, vtupdate(Evt1, Vt), St1),
Rvt0 = Sccs,
Rvt1 = vtupdate(vtunsafe(vtnames(vtnew(Rvt0, Vt)), TryLine, []), Rvt0),
Evt2 = vtmerge(Evt1, Rvt1),
- {Avt0,St} = exprs(As, Evt2, St2),
+ {Avt0,St} = exprs(As, vtupdate(Evt2, Vt), St2),
Avt1 = vtupdate(vtunsafe(vtnames(vtnew(Avt0, Vt)), TryLine, []), Avt0),
Avt = vtmerge(Evt2, Avt1),
{Avt,St};
@@ -2150,10 +2147,11 @@ expr({remote,Line,_M,_F}, _Vt, St) ->
%% {UsedVarTable,State}
expr_list(Es, Vt, St) ->
- foldl(fun (E, {Esvt,St0}) ->
- {Evt,St1} = expr(E, Vt, St0),
- {vtmerge(Evt, Esvt),St1}
- end, {[],St}, Es).
+ {Vt1,St1} = foldl(fun (E, {Esvt,St0}) ->
+ {Evt,St1} = expr(E, Vt, St0),
+ {vtmerge_pat(Evt, Esvt),St1}
+ end, {[],St}, Es),
+ {vtmerge(vtnew(Vt1, Vt), vtold(Vt1, Vt)),St1}.
record_expr(Line, Rec, Vt, St0) ->
St1 = warn_invalid_record(Line, Rec, St0),
@@ -2843,7 +2841,9 @@ icrt_export(Csvt, Vt, In, St) ->
Uvt = vtmerge(Evt, Unused),
%% Make exported and unsafe unused variables unused in subsequent code:
Vt2 = vtmerge(Uvt, vtsubtract(Vt1, Uvt)),
- {Vt2,St}.
+ %% Forget about old variables which were not used:
+ Vt3 = vtmerge(vtnew(Vt2, Vt), vt_no_unused(vtold(Vt2, Vt))),
+ {Vt3,St}.
handle_comprehension(E, Qs, Vt0, St0) ->
{Vt1, Uvt, St1} = lc_quals(Qs, Vt0, St0),
@@ -2856,7 +2856,11 @@ handle_comprehension(E, Qs, Vt0, St0) ->
%% Local variables that have not been shadowed.
{_,St} = check_unused_vars(Vt2, Vt0, St4),
Vt3 = vtmerge(vtsubtract(Vt2, Uvt), Uvt),
- {Vt3,St}.
+ %% Don't export local variables.
+ Vt4 = vtold(Vt3, Vt0),
+ %% Forget about old variables which were not used.
+ Vt5 = vt_no_unused(Vt4),
+ {Vt5,St}.
%% lc_quals(Qualifiers, ImportVarTable, State) ->
%% {VarTable,ShadowedVarTable,State}
@@ -2920,7 +2924,7 @@ fun_clauses(Cs, Vt, St) ->
{Cvt,St1} = fun_clause(C, Vt, St0),
{vtmerge(Cvt, Bvt0),St1}
end, {[],St#lint{recdef_top = false}}, Cs),
- {Bvt,St2#lint{recdef_top = OldRecDef}}.
+ {vt_no_unused(vtold(Bvt, Vt)),St2#lint{recdef_top = OldRecDef}}.
fun_clause({clause,_Line,H,G,B}, Vt0, St0) ->
{Hvt,Binvt,St1} = head(H, Vt0, [], St0), % No imported pattern variables
@@ -3181,6 +3185,8 @@ vt_no_unsafe(Vt) -> [V || {_,{S,_U,_L}}=V <- Vt,
_ -> true
end].
+vt_no_unused(Vt) -> [V || {_,{_,U,_L}}=V <- Vt, U =/= unused].
+
%% vunion(VarTable1, VarTable2) -> [VarName].
%% vunion([VarTable]) -> [VarName].
%% vintersection(VarTable1, VarTable2) -> [VarName].
diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl
index 4dc7a44064..0bcf9b3e01 100644
--- a/lib/stdlib/test/erl_lint_SUITE.erl
+++ b/lib/stdlib/test/erl_lint_SUITE.erl
@@ -151,7 +151,16 @@ unused_vars_warn_basic(Config) when is_list(Config) ->
{22,erl_lint,{unused_var,'N'}},
{23,erl_lint,{shadowed_var,'N','fun'}},
{28,erl_lint,{unused_var,'B'}},
- {29,erl_lint,{unused_var,'B'}}]}}],
+ {29,erl_lint,{unused_var,'B'}}]}},
+ {basic2,
+ <<"-record(r, {x,y}).
+ f({X,Y}) -> {Z=X,Z=Y};
+ f([H|T]) -> [Z=H|Z=T];
+ f(#r{x=X,y=Y}) -> #r{x=A=X,y=A=Y}.
+ g({M, F}) -> (Z=M):(Z=F)();
+ g({M, F, Arg}) -> (Z=M):F(Z=Arg).
+ h(X, Y) -> (Z=X) + (Z=Y).">>,
+ [warn_unused_vars], []}],
?line [] = run(Config, Ts),
ok.
@@ -537,7 +546,21 @@ unused_vars_warn_rec(Config) when is_list(Config) ->
end.
">>,
[warn_unused_vars],
- {warnings,[{22,erl_lint,{unused_var,'Same'}}]}}],
+ {warnings,[{22,erl_lint,{unused_var,'Same'}}]}},
+ {rec2,
+ <<"-record(r, {a,b}).
+ f(X, Y) -> #r{a=[K || K <- Y], b=[K || K <- Y]}.
+ g(X, Y) -> #r{a=lists:map(fun (K) -> K end, Y),
+ b=lists:map(fun (K) -> K end, Y)}.
+ h(X, Y) -> #r{a=case Y of _ when is_list(Y) -> Y end,
+ b=case Y of _ when is_list(Y) -> Y end}.
+ i(X, Y) -> #r{a=if is_list(Y) -> Y end, b=if is_list(Y) -> Y end}.
+ ">>,
+ [warn_unused_vars],
+ {warnings,[{2,erl_lint,{unused_var,'X'}},
+ {3,erl_lint,{unused_var,'X'}},
+ {5,erl_lint,{unused_var,'X'}},
+ {7,erl_lint,{unused_var,'X'}}]}}],
?line [] = run(Config, Ts),
ok.
--
cgit v1.2.3
From 9ce148b1059e4da746a11f1d80a653340216c468 Mon Sep 17 00:00:00 2001
From: Anthony Ramine <n.oxyde@gmail.com>
Date: Mon, 22 Jul 2013 21:41:31 +0200
Subject: Fix unsafe variable tracking in try expressions
Variables used in the body of a try expression were marked as unsafe
*and* used, which makes no sense as an unsafe variable can't be used.
Function vtsubtract/2 is used to forget usage of such unsafe variables.
Reported-by: Paul Davis
---
lib/stdlib/src/erl_lint.erl | 2 +-
lib/stdlib/test/erl_lint_SUITE.erl | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl
index 9284f08b30..708ef44fed 100644
--- a/lib/stdlib/src/erl_lint.erl
+++ b/lib/stdlib/src/erl_lint.erl
@@ -2108,7 +2108,7 @@ expr({'try',Line,Es,Scs,Ccs,As}, Vt, St0) ->
{Evt0,St1} = exprs(Es, Vt, St0),
TryLine = {'try',Line},
Uvt = vtunsafe(vtnames(vtnew(Evt0, Vt)), TryLine, []),
- Evt1 = vtupdate(Uvt, Evt0),
+ Evt1 = vtupdate(Uvt, vtsubtract(Evt0, Uvt)),
{Sccs,St2} = icrt_clauses(Scs++Ccs, TryLine, vtupdate(Evt1, Vt), St1),
Rvt0 = Sccs,
Rvt1 = vtupdate(vtunsafe(vtnames(vtnew(Rvt0, Vt)), TryLine, []), Rvt0),
diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl
index 0bcf9b3e01..a1a9839bae 100644
--- a/lib/stdlib/test/erl_lint_SUITE.erl
+++ b/lib/stdlib/test/erl_lint_SUITE.erl
@@ -1098,7 +1098,24 @@ unsafe_vars_try(Config) when is_list(Config) ->
{10,erl_lint,{unsafe_var,'Ra',{'try',3}}},
{10,erl_lint,{unsafe_var,'Rc',{'try',3}}},
{10,erl_lint,{unsafe_var,'Ro',{'try',3}}}],
- []}}],
+ []}},
+ {unsafe_try5,
+ <<"bang() ->
+ case 1 of
+ nil ->
+ Acc = 2;
+ _ ->
+ try
+ Acc = 3,
+ Acc
+ catch _:_ ->
+ ok
+ end
+ end,
+ Acc.
+ ">>,
+ [],
+ {errors,[{13,erl_lint,{unsafe_var,'Acc',{'try',6}}}],[]}}],
?line [] = run(Config, Ts),
ok.
--
cgit v1.2.3
From 92460f4035e369be8726c88a72055d0fd6c7cdfe Mon Sep 17 00:00:00 2001
From: Anthony Ramine <n.oxyde@gmail.com>
Date: Wed, 31 Jul 2013 10:31:51 +0200
Subject: Fix variable usage tracking in some record errors
When reporting a field redefinition in a record, erl_lint can forget
about some old unused variables.
f() -> X = 1, #r{a=foo,a=bar,a=qux}.
---
lib/stdlib/src/erl_lint.erl | 2 +-
lib/stdlib/test/erl_lint_SUITE.erl | 10 +++++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl
index 708ef44fed..f599881c07 100644
--- a/lib/stdlib/src/erl_lint.erl
+++ b/lib/stdlib/src/erl_lint.erl
@@ -2308,7 +2308,7 @@ check_fields(Fs, Name, Fields, Vt, St0, CheckFun) ->
check_field({record_field,Lf,{atom,La,F},Val}, Name, Fields,
Vt, St, Sfs, CheckFun) ->
case member(F, Sfs) of
- true -> {Sfs,{Vt,add_error(Lf, {redefine_field,Name,F}, St)}};
+ true -> {Sfs,{[],add_error(Lf, {redefine_field,Name,F}, St)}};
false ->
{[F|Sfs],
case find_field(F, Fields) of
diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl
index a1a9839bae..48ddeac478 100644
--- a/lib/stdlib/test/erl_lint_SUITE.erl
+++ b/lib/stdlib/test/erl_lint_SUITE.erl
@@ -560,7 +560,15 @@ unused_vars_warn_rec(Config) when is_list(Config) ->
{warnings,[{2,erl_lint,{unused_var,'X'}},
{3,erl_lint,{unused_var,'X'}},
{5,erl_lint,{unused_var,'X'}},
- {7,erl_lint,{unused_var,'X'}}]}}],
+ {7,erl_lint,{unused_var,'X'}}]}},
+ {rec3,
+ <<"-record(r, {a}).
+ t() -> X = 1, #r{a=foo, a=bar, a=qux}.
+ ">>,
+ [warn_unused_vars],
+ {error,[{2,erl_lint,{redefine_field,r,a}},
+ {2,erl_lint,{redefine_field,r,a}}],
+ [{2,erl_lint,{unused_var,'X'}}]}}],
?line [] = run(Config, Ts),
ok.
--
cgit v1.2.3
From f3b8c6f2ded7659b628afe5eb8e11e7e7de201db Mon Sep 17 00:00:00 2001
From: Fredrik Gustafsson <fredrik@erlang.org>
Date: Tue, 27 Aug 2013 16:29:13 +0200
Subject: Updated primary bootstrap
---
bootstrap/lib/stdlib/ebin/erl_lint.beam | Bin 83616 -> 83860 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
diff --git a/bootstrap/lib/stdlib/ebin/erl_lint.beam b/bootstrap/lib/stdlib/ebin/erl_lint.beam
index ee07e7636c..30c387c53c 100644
Binary files a/bootstrap/lib/stdlib/ebin/erl_lint.beam and b/bootstrap/lib/stdlib/ebin/erl_lint.beam differ
--
cgit v1.2.3