From 8c0bebb66ba01b174e6482cd81949eaf08748bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Sat, 31 Jan 2015 08:17:25 +0100 Subject: trace_bif_SUITE: Ensure that a call to time/0 is not removed Assigning the result of a BIF call to a variable used to be sufficient to prevent the call from being optimized away. With the more aggressive optimization that will be introduced in a future commit it will not be sufficient. Matching the return value will prevent the compiler from doing the optimization. --- erts/emulator/test/trace_bif_SUITE.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erts/emulator/test/trace_bif_SUITE.erl b/erts/emulator/test/trace_bif_SUITE.erl index 2c78aa394f..063e348836 100644 --- a/erts/emulator/test/trace_bif_SUITE.erl +++ b/erts/emulator/test/trace_bif_SUITE.erl @@ -260,7 +260,9 @@ bif_process() -> apply(erlang, Name, Args), bif_process(); {do_time_bif} -> - _ = time(), %Assignment tells compiler to keep call. + %% Match the return value to ensure that the time() call + %% is not optimized away. + {_,_,_} = time(), bif_process(); {do_statistics_bif} -> statistics(runtime), -- cgit v1.2.3 From 8c3baeb1275c2e6a316d3b5203e0598906785cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 29 Jan 2015 06:33:52 +0100 Subject: Suppress warnings for expressions that are assigned to '_' In c34ad2d5, the compiler learned to silence some warnings for expressions that were explicitly assigned to the '_' variable, as in this example: _ = list_to_integer(S), ok That commit intentionally only made it possible to silence warnings for BIFs that could cause an exception. Warnings would still be produced for: _ = date(), ok because date/0 can never fail and thus making the call completely useless. The reasoning was that such warnings can always be eliminated by eliminating the offending code. While that is true, there is the question about rules and their consistency. It is surprising that '_' can be used to silence some warnings, but has no effect on other warnings. Therefore, we will teach the compiler to silence warnings for the following constructs: * Calls to safe BIFs such as date/0 * Expressions that will cause an exception such as 'X/0' * Terms that are built but not used, such as '{x,X}' --- lib/compiler/src/sys_core_fold.erl | 28 +++++++++++----------- lib/compiler/src/v3_core.erl | 45 ++++++++++++++++++------------------ lib/compiler/test/warnings_SUITE.erl | 44 +++++++++++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 38 deletions(-) diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl index 7066cebb2b..eeb3535936 100644 --- a/lib/compiler/src/sys_core_fold.erl +++ b/lib/compiler/src/sys_core_fold.erl @@ -680,23 +680,15 @@ count_bits_1(Int, Bits) -> count_bits_1(Int bsr 64, Bits+64). %% a rewritten expression consisting of a sequence of %% the arguments only is returned. -useless_call(effect, #c_call{anno=Anno, - module=#c_literal{val=Mod}, +useless_call(effect, #c_call{module=#c_literal{val=Mod}, name=#c_literal{val=Name}, args=Args}=Call) -> A = length(Args), case erl_bifs:is_safe(Mod, Name, A) of false -> case erl_bifs:is_pure(Mod, Name, A) of - true -> - case member(result_not_wanted, Anno) of - false -> - add_warning(Call, result_ignored); - true -> - ok - end; - false -> - ok + true -> add_warning(Call, result_ignored); + false -> ok end, no; true -> @@ -3101,7 +3093,7 @@ add_bin_opt_info(Core, Term) -> end. add_warning(Core, Term) -> - case is_compiler_generated(Core) of + case suppress_warning(Core) of true -> ok; false -> @@ -3126,9 +3118,17 @@ get_file([{file,File}|_]) -> File; get_file([_|T]) -> get_file(T); get_file([]) -> "no_file". % should not happen +suppress_warning(Core) -> + is_compiler_generated(Core) orelse + is_result_unwanted(Core). + is_compiler_generated(Core) -> - Anno = core_lib:get_anno(Core), - member(compiler_generated, Anno). + Ann = cerl:get_ann(Core), + member(compiler_generated, Ann). + +is_result_unwanted(Core) -> + Ann = cerl:get_ann(Core), + member(result_not_wanted, Ann). get_warnings() -> ordsets:from_list((erase({?MODULE,warnings}))). diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl index a9b54005d1..f0b90ff31c 100644 --- a/lib/compiler/src/v3_core.erl +++ b/lib/compiler/src/v3_core.erl @@ -515,16 +515,16 @@ exprs([], St) -> {[],St}. %% Generate an internal core expression. expr({var,L,V}, St) -> {#c_var{anno=lineno_anno(L, St),name=V},[],St}; -expr({char,L,C}, St) -> {#c_literal{anno=lineno_anno(L, St),val=C},[],St}; -expr({integer,L,I}, St) -> {#c_literal{anno=lineno_anno(L, St),val=I},[],St}; -expr({float,L,F}, St) -> {#c_literal{anno=lineno_anno(L, St),val=F},[],St}; -expr({atom,L,A}, St) -> {#c_literal{anno=lineno_anno(L, St),val=A},[],St}; -expr({nil,L}, St) -> {#c_literal{anno=lineno_anno(L, St),val=[]},[],St}; -expr({string,L,S}, St) -> {#c_literal{anno=lineno_anno(L, St),val=S},[],St}; +expr({char,L,C}, St) -> {#c_literal{anno=full_anno(L, St),val=C},[],St}; +expr({integer,L,I}, St) -> {#c_literal{anno=full_anno(L, St),val=I},[],St}; +expr({float,L,F}, St) -> {#c_literal{anno=full_anno(L, St),val=F},[],St}; +expr({atom,L,A}, St) -> {#c_literal{anno=full_anno(L, St),val=A},[],St}; +expr({nil,L}, St) -> {#c_literal{anno=full_anno(L, St),val=[]},[],St}; +expr({string,L,S}, St) -> {#c_literal{anno=full_anno(L, St),val=S},[],St}; expr({cons,L,H0,T0}, St0) -> {H1,Hps,St1} = safe(H0, St0), {T1,Tps,St2} = safe(T0, St1), - A = lineno_anno(L, St2), + A = full_anno(L, St2), {annotate_cons(A, H1, T1, St2),Hps ++ Tps,St2}; expr({lc,L,E,Qs0}, St0) -> {Qs1,St1} = preprocess_quals(L, Qs0, St0), @@ -536,7 +536,7 @@ expr({tuple,L,Es0}, St0) -> A = record_anno(L, St1), {annotate_tuple(A, Es1, St1),Eps,St1}; expr({map,L,Es0}, St0) -> - map_build_pairs(#c_literal{val=#{}}, Es0, lineno_anno(L, St0), St0); + map_build_pairs(#c_literal{val=#{}}, Es0, full_anno(L, St0), St0); expr({map,L,M0,Es0}, St0) -> try expr_map(M0,Es0,lineno_anno(L, St0),St0) of {_,_,_}=Res -> Res @@ -551,7 +551,7 @@ expr({map,L,M0,Es0}, St0) -> args=As},[],St} end; expr({bin,L,Es0}, St0) -> - try expr_bin(Es0, lineno_anno(L, St0), St0) of + try expr_bin(Es0, full_anno(L, St0), St0) of {_,_,_}=Res -> Res catch throw:bad_binary -> @@ -641,11 +641,11 @@ expr({'catch',L,E0}, St0) -> Lanno = lineno_anno(L, St1), {#icatch{anno=#a{anno=Lanno},body=Eps ++ [E1]},[],St1}; expr({'fun',L,{function,F,A},{_,_,_}=Id}, St) -> - Lanno = lineno_anno(L, St), + Lanno = full_anno(L, St), {#c_var{anno=Lanno++[{id,Id}],name={F,A}},[],St}; expr({'fun',L,{function,M,F,A}}, St0) -> {As,Aps,St1} = safe_list([M,F,A], St0), - Lanno = lineno_anno(L, St1), + Lanno = full_anno(L, St1), {#icall{anno=#a{anno=Lanno}, module=#c_literal{val=erlang}, name=#c_literal{val=make_fun}, @@ -656,13 +656,9 @@ expr({named_fun,L,'_',Cs,Id}, St) -> fun_tq(Id, Cs, L, St, unnamed); expr({named_fun,L,Name,Cs,Id}, St) -> fun_tq(Id, Cs, L, St, {named,Name}); -expr({call,L,{remote,_,M,F},As0}, #core{wanted=Wanted}=St0) -> +expr({call,L,{remote,_,M,F},As0}, St0) -> {[M1,F1|As1],Aps,St1} = safe_list([M,F|As0], St0), - Lanno = lineno_anno(L, St1), - Anno = case Wanted of - false -> [result_not_wanted|Lanno]; - true -> Lanno - end, + Anno = full_anno(L, St1), {#icall{anno=#a{anno=Anno},module=M1,name=F1,args=As1},Aps,St1}; expr({call,Lc,{atom,Lf,F},As0}, St0) -> {As1,Aps,St1} = safe_list(As0, St0), @@ -724,13 +720,13 @@ expr({op,L,'orelse',E1,E2}, St0) -> expr(E, St); expr({op,L,Op,A0}, St0) -> {A1,Aps,St1} = safe(A0, St0), - LineAnno = lineno_anno(L, St1), + LineAnno = full_anno(L, St1), {#icall{anno=#a{anno=LineAnno}, %Must have an #a{} module=#c_literal{anno=LineAnno,val=erlang}, name=#c_literal{anno=LineAnno,val=Op},args=[A1]},Aps,St1}; expr({op,L,Op,L0,R0}, St0) -> {As,Aps,St1} = safe_list([L0,R0], St0), - LineAnno = lineno_anno(L, St1), + LineAnno = full_anno(L, St1), {#icall{anno=#a{anno=LineAnno}, %Must have an #a{} module=#c_literal{anno=LineAnno,val=erlang}, name=#c_literal{anno=LineAnno,val=Op},args=As},Aps,St1}. @@ -971,7 +967,7 @@ fun_tq({_,_,Name}=Id, Cs0, L, St0, NameInfo) -> {Cs1,Ceps,St1} = clauses(Cs0, St0), {Args,St2} = new_vars(Arity, St1), {Ps,St3} = new_vars(Arity, St2), %Need new variables here - Anno = lineno_anno(L, St3), + Anno = full_anno(L, St3), Fc = function_clause(Ps, Anno, {Name,Arity}), Fun = #ifun{anno=#a{anno=Anno}, id=[{id,Id}], %We KNOW! @@ -2341,16 +2337,21 @@ record_anno(L, St) when L >= ?REC_OFFSET -> true -> [record | lineno_anno(L - ?REC_OFFSET, St)]; false -> - lineno_anno(L, St) + full_anno(L, St) end; record_anno(L, St) when L < -?REC_OFFSET -> case member(dialyzer, St#core.opts) of true -> [record | lineno_anno(L + ?REC_OFFSET, St)]; false -> - lineno_anno(L, St) + full_anno(L, St) end; record_anno(L, St) -> + full_anno(L, St). + +full_anno(L, #core{wanted=false}=St) -> + [result_not_wanted|lineno_anno(L, St)]; +full_anno(L, #core{wanted=true}=St) -> lineno_anno(L, St). lineno_anno(L, St) -> diff --git a/lib/compiler/test/warnings_SUITE.erl b/lib/compiler/test/warnings_SUITE.erl index 6e5a7d35e9..d59c89b264 100644 --- a/lib/compiler/test/warnings_SUITE.erl +++ b/lib/compiler/test/warnings_SUITE.erl @@ -39,7 +39,7 @@ guard/1,bad_arith/1,bool_cases/1,bad_apply/1, files/1,effect/1,bin_opt_info/1,bin_construction/1, comprehensions/1,maps/1,redundant_boolean_clauses/1, - latin1_fallback/1]). + latin1_fallback/1,underscore/1]). % Default timetrap timeout (set in init_per_testcase). -define(default_timeout, ?t:minutes(2)). @@ -64,7 +64,8 @@ groups() -> [pattern,pattern2,pattern3,pattern4,guard, bad_arith,bool_cases,bad_apply,files,effect, bin_opt_info,bin_construction,comprehensions,maps, - redundant_boolean_clauses,latin1_fallback]}]. + redundant_boolean_clauses,latin1_fallback, + underscore]}]. init_per_suite(Config) -> Config. @@ -678,6 +679,45 @@ latin1_fallback(Conf) when is_list(Conf) -> ok. +underscore(Config) when is_list(Config) -> + S0 = <<"f(A) -> + _VAR1 = <>, + _VAR2 = {ok,A}, + _VAR3 = [A], + ok. + g(A) -> + _VAR1 = A/0, + _VAR2 = date(), + ok. + h() -> + _VAR1 = fun() -> ok end, + ok. + i(A) -> + _VAR1 = #{A=>42}, + ok. + ">>, + Ts0 = [{underscore0, + S0, + [], + {warnings,[{2,sys_core_fold,useless_building}, + {3,sys_core_fold,useless_building}, + {4,sys_core_fold,useless_building}, + {7,sys_core_fold,result_ignored}, + {8,sys_core_fold,{no_effect,{erlang,date,0}}}, + {11,sys_core_fold,useless_building}, + {14,sys_core_fold,useless_building} + ]}}], + [] = run(Config, Ts0), + + %% Replace all "_VAR" variables with a plain underscore. + %% Now there should be no warnings. + S1 = re:replace(S0, "_VAR\\d+", "_", [global]), + io:format("~s\n", [S1]), + Ts1 = [{underscore1,S1,[],[]}], + [] = run(Config, Ts1), + + ok. + %%% %%% End of test cases. %%% -- cgit v1.2.3 From cd1eaf0116190ab72f3a792b74be99eda5dd31eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 23 Jan 2015 13:12:44 +0100 Subject: sys_core_fold: Optimize let statements more aggressively I originally decided that in 'value' context, rewriting a let statement where the variables were not in the body to a sequence was not worth it, because the variables would be unused in only one let in a thousand lets (roughly). I have reconsidered. The main reason is that if we do the rewrite, core_lib:is_var_used/2 will be used much more frequently, which will help us to find bugs in it sooner. Another reason is that the way letify/2 is currently implemented with its own calls to core_lib:is_var_used/2 is only safe as long as all the bindings are independent of each other. We could make letify/2 smarter, but if we introduce this new optimization there is no need. Measuring compilation speed, I have not seen any significant slowdown. It seems that although core_lib:is_var_used/2 is called much more frequently, most calls will be fast because is_var_used/2 will quickly find a use of the variable. Also add a test case to cover a line opt_guard_try/1 that was no longer covered. --- lib/compiler/src/sys_core_fold.erl | 27 ++++++++++++++------------- lib/compiler/test/compilation_SUITE.erl | 6 ++---- lib/compiler/test/core_fold_SUITE.erl | 13 +++++++++++++ lib/compiler/test/trycatch_SUITE.erl | 1 - lib/compiler/test/warnings_SUITE.erl | 3 ++- 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl index eeb3535936..72509947d6 100644 --- a/lib/compiler/src/sys_core_fold.erl +++ b/lib/compiler/src/sys_core_fold.erl @@ -2246,18 +2246,11 @@ make_var_name() -> list_to_atom("fol"++integer_to_list(N)). letify(Bs, Body) -> + Ann = cerl:get_ann(Body), foldr(fun({V,Val}, B) -> - letify(V, Val, B) + cerl:ann_c_let(Ann, [V], Val, B) end, Body, Bs). -letify(#c_var{name=Vname}=Var, Val, Body) -> - case core_lib:is_var_used(Vname, Body) of - true -> - A = element(2, Body), - #c_let{anno=A,vars=[Var],arg=Val,body=Body}; - false -> Body - end. - %% opt_case_in_let(LetExpr) -> LetExpr' opt_case_in_let(#c_let{vars=Vs,arg=Arg,body=B}=Let, Sub) -> @@ -2622,7 +2615,7 @@ opt_simple_let_2(Let0, Vs0, Arg0, Body0, effect, Sub) -> opt_case_in_let_arg(opt_case_in_let(Let, Sub), effect, Sub) end end; -opt_simple_let_2(Let, Vs0, Arg0, Body, value, Sub) -> +opt_simple_let_2(Let0, Vs0, Arg0, Body, value, Sub) -> case {Vs0,Arg0,Body} of {[#c_var{name=N1}],Arg,#c_var{name=N2}} -> case N1 =:= N2 of @@ -2641,9 +2634,17 @@ opt_simple_let_2(Let, Vs0, Arg0, Body, value, Sub) -> %% can be evaluated in effect context to simplify it. expr(#c_seq{arg=Arg,body=Body}, value, sub_new_preserve_types(Sub)); {Vs,Arg,Body} -> - opt_case_in_let_arg( - opt_case_in_let(Let#c_let{vars=Vs,arg=Arg,body=Body}, Sub), - value, Sub) + %% If none of the variables are used in the body, we can rewrite the + %% let to a sequence: + %% let = Arg in BodyWithoutVar ==> seq Arg BodyWithoutVar + case is_any_var_used(Vs, Body) of + false -> + expr(#c_seq{arg=Arg,body=Body}, value, + sub_new_preserve_types(Sub)); + true -> + Let = Let0#c_let{vars=Vs,arg=Arg,body=Body}, + opt_case_in_let_arg(opt_case_in_let(Let, Sub), value, Sub) + end end. move_case_into_arg(#c_case{arg=#c_let{vars=OuterVars0,arg=OuterArg, diff --git a/lib/compiler/test/compilation_SUITE.erl b/lib/compiler/test/compilation_SUITE.erl index 8711f35e8e..296774e083 100644 --- a/lib/compiler/test/compilation_SUITE.erl +++ b/lib/compiler/test/compilation_SUITE.erl @@ -611,12 +611,10 @@ otp_7345(Config) when is_list(Config) -> otp_7345(ObjRef, _RdEnv, Args) -> Cid = ObjRef#contextId.cid, - _DpRef = - #dpRef{cid = Cid, + _ = #dpRef{cid = Cid, ms_device_context_id = cid_id, tlli = #ptmsi{value = 0}}, - _QosProfile = - #qosProfileBssgp{peak_bit_rate_msb = 0, + _ = #qosProfileBssgp{peak_bit_rate_msb = 0, peak_bit_rate_lsb = 80, t_a_precedence = 49}, [Cpdu|_] = Args, diff --git a/lib/compiler/test/core_fold_SUITE.erl b/lib/compiler/test/core_fold_SUITE.erl index ce1aea5de0..bca8afb1b0 100644 --- a/lib/compiler/test/core_fold_SUITE.erl +++ b/lib/compiler/test/core_fold_SUITE.erl @@ -242,6 +242,8 @@ do_guard_try_catch(K, V) -> false end. +-record(cover_opt_guard_try, {list=[]}). + coverage(Config) when is_list(Config) -> ?line {'EXIT',{{case_clause,{a,b,c}},_}} = (catch cover_will_match_list_type({a,b,c})), @@ -251,6 +253,9 @@ coverage(Config) when is_list(Config) -> ?line error = cover_will_match_lit_list(), {ok,[a]} = cover_is_safe_bool_expr(a), + ok = cover_opt_guard_try(#cover_opt_guard_try{list=[a]}), + error = cover_opt_guard_try(#cover_opt_guard_try{list=[]}), + %% Make sure that we don't attempt to make literals %% out of pids. (Putting a pid into a #c_literal{} %% would crash later compiler passes.) @@ -304,6 +309,14 @@ cover_is_safe_bool_expr(X) -> false end. +cover_opt_guard_try(Msg) -> + if + length(Msg#cover_opt_guard_try.list) =/= 1 -> + error; + true -> + ok + end. + bsm_an_inlined(<<_:8>>, _) -> ok; bsm_an_inlined(_, _) -> error. diff --git a/lib/compiler/test/trycatch_SUITE.erl b/lib/compiler/test/trycatch_SUITE.erl index b490257a6e..8ab618bb01 100644 --- a/lib/compiler/test/trycatch_SUITE.erl +++ b/lib/compiler/test/trycatch_SUITE.erl @@ -790,7 +790,6 @@ nested_after_1({X1,C1,V1}, nested_horrid(Config) when is_list(Config) -> - _V = {make_ref(),nested_horrid,4.711}, {[true,true],{[true,1.0],1.0}} = nested_horrid_1({true,void,void}, 1.0), ok. diff --git a/lib/compiler/test/warnings_SUITE.erl b/lib/compiler/test/warnings_SUITE.erl index d59c89b264..6663985ad7 100644 --- a/lib/compiler/test/warnings_SUITE.erl +++ b/lib/compiler/test/warnings_SUITE.erl @@ -281,11 +281,12 @@ bad_arith(Config) when is_list(Config) -> {3,sys_core_fold,{eval_failure,badarith}}, {9,sys_core_fold,nomatch_guard}, {9,sys_core_fold,{eval_failure,badarith}}, + {9,sys_core_fold,{no_effect,{erlang,is_integer,1}}}, {10,sys_core_fold,nomatch_guard}, {10,sys_core_fold,{eval_failure,badarith}}, {15,sys_core_fold,{eval_failure,badarith}} ] }}], - ?line [] = run(Config, Ts), + [] = run(Config, Ts), ok. bool_cases(Config) when is_list(Config) -> -- cgit v1.2.3 From 4babd738633953a09ce3314ed89d0933063b4ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 23 Jan 2015 13:12:09 +0100 Subject: Teach case_opt/3 to avoid unnecessary building Given this code: f(S) -> F0 = F1 = {S,S}, [F0,F1]. case_opt/3 would "optimize" it like this: f(S) -> F1 = {S,S}, F0 = {S,S}, [F0,F1]. Similarly, this code: g({a,_,_}=T) -> {b, [_,_] = [T,none], x}. would be rewritten to: g({a,Tmp1,Tmp2}=T) -> Tmp3 = {a,Tmp1,Tmp2}, {b, [Tmp3,none], x}. where the tuple is rebuilt instead of using the T variable. Rewrite case_opt/3 to be more careful while optimizing. --- lib/compiler/src/sys_core_fold.erl | 144 +++++++++++++++++++++++----------- lib/compiler/test/core_fold_SUITE.erl | 31 +++++++- lib/compiler/test/test_lib.erl | 4 +- 3 files changed, 129 insertions(+), 50 deletions(-) diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl index 72509947d6..c29b5cde41 100644 --- a/lib/compiler/src/sys_core_fold.erl +++ b/lib/compiler/src/sys_core_fold.erl @@ -2025,12 +2025,31 @@ case_opt_args([], Cs, _Sub, _LitExpr, Acc) -> %% Try to expand one argument to several arguments (if tuple/list) %% or to remove a literal argument. %% -case_opt_arg(E0, Sub, Cs0, LitExpr) -> - E = maybe_replace_var(E0, Sub), - case cerl:is_data(E) of +case_opt_arg(E0, Sub, Cs, LitExpr) -> + case cerl:is_c_var(E0) of + false -> + case_opt_arg_1(E0, Cs, LitExpr); + true -> + case case_will_var_match(Cs) of + true -> + %% All clauses will match a variable in the + %% current position. Don't expand this variable + %% (that can only make the code worse). + {error,Cs}; + false -> + %% If possible, expand this variable to a previously + %% matched term. + E = case_expand_var(E0, Sub), + case_opt_arg_1(E, Cs, LitExpr) + end + end. + +case_opt_arg_1(E0, Cs0, LitExpr) -> + case cerl:is_data(E0) of false -> {error,Cs0}; true -> + E = case_opt_compiler_generated(E0), Cs = case_opt_nomatch(E, Cs0, LitExpr), case cerl:data_type(E) of {atomic,_} -> @@ -2040,18 +2059,42 @@ case_opt_arg(E0, Sub, Cs0, LitExpr) -> end end. -%% maybe_replace_var(Expr0, Sub) -> Expr +%% case_will_var_match([Clause]) -> true | false. +%% Return if all clauses will match a variable in the +%% current position. +%% +case_will_var_match(Cs) -> + all(fun({[P|_],_,_,_}) -> + case cerl_clauses:match(P, any) of + {true,_} -> true; + _ -> false + end + end, Cs). + + +%% case_opt_compiler_generated(Core) -> Core' +%% Mark Core expressions as compiler generated to ensure that +%% no warnings are generated if they turn out to be unused. +%% To pretty-printed Core Erlang easier to read, don't mark +%% constructs that can't cause warnings to be emitted. +%% +case_opt_compiler_generated(Core) -> + F = fun(C) -> + case cerl:type(C) of + alias -> C; + var -> C; + _ -> cerl:set_ann(C, [compiler_generated]) + end + end, + cerl_trees:map(F, Core). + + +%% case_expand_var(Expr0, Sub) -> Expr %% If Expr0 is a variable that has been previously matched and %% is known to be a tuple, return the tuple instead. Otherwise %% return Expr0 unchanged. %% -maybe_replace_var(E, Sub) -> - case cerl:is_c_var(E) of - false -> E; - true -> maybe_replace_var_1(E, Sub) - end. - -maybe_replace_var_1(E, #sub{t=Tdb}) -> +case_expand_var(E, #sub{t=Tdb}) -> case orddict:find(cerl:var_name(E), Tdb) of {ok,T0} -> case cerl:is_c_tuple(T0) of @@ -2068,9 +2111,8 @@ maybe_replace_var_1(E, #sub{t=Tdb}) -> %% operator will fail when used in map %% construction (only the '=>' operator is allowed %% when constructing a map from scratch). - ToData = fun coerce_to_data/1, try - cerl_trees:map(ToData, T0) + cerl_trees:map(fun coerce_to_data/1, T0) catch throw:impossible -> %% Something unsuitable was found (map or @@ -2124,8 +2166,9 @@ case_opt_nomatch(_, [], _) -> []. %% will match, and we can remove the corresponding pattern from %% each clause. %% -%% The only complication is if the literal is a binary. Binary -%% pattern matching is tricky, so we will give up in that case. +%% The only complication is if the literal is a binary or map. +%% In general, it is difficult to know whether a binary or +%% map pattern will match, so we give up in that case. case_opt_lit(Lit, Cs0) -> try case_opt_lit_1(Lit, Cs0) of @@ -2152,6 +2195,10 @@ case_opt_lit_1(E, [{[P|Ps],C,PsAcc,Bs0}|Cs]) -> case_opt_lit_1(_, []) -> []. %% case_opt_data(Expr, Clauses0, LitExpr) -> {ok,Exprs,Clauses} +%% The case expression is a non-atomic data constructor (cons +%% or tuple). We can know at compile time whether each clause +%% will match, and we can delay the building of the data to +%% the clauses where it is actually needed. case_opt_data(E, Cs0) -> Es = cerl:data_es(E), @@ -2161,45 +2208,48 @@ case_opt_data(E, Cs0) -> {ok,Es,Cs} catch throw:impossible -> + %% The pattern contained a binary or map. {error,Cs0} end. -case_opt_data_1([{[P|Ps0],C,PsAcc,Bs0}|Cs], Es, TypeSig) -> - {ok,Ps1,Bs1} = case_data_pat(P, TypeSig), - [{Ps1++Ps0,C,PsAcc,Bs1++Bs0}| - case_opt_data_1(Cs, Es, TypeSig)]; +case_opt_data_1([{[P0|Ps0],C,PsAcc,Bs0}|Cs], Es, TypeSig) -> + P = case_opt_compiler_generated(P0), + BindTo = #c_var{name=dummy}, + {Ps1,[{BindTo,_}|Bs1]} = case_data_pat_alias(P, BindTo, TypeSig, []), + [{Ps1++Ps0,C,PsAcc,Bs1++Bs0}|case_opt_data_1(Cs, Es, TypeSig)]; case_opt_data_1([], _, _) -> []. -%% case_data_pat(Pattern, Type, Arity) -> {ok,[Pattern],[{AliasVar,Pat}]} | error. - -case_data_pat(P, TypeSig) -> - case cerl:is_data(P) of - false -> - case_data_pat_var(P, TypeSig); - true -> - {ok,cerl:data_es(P),[]} - end. - -%% case_data_pat_var(Pattern, {DataType,ArityType}) -> -%% {ok,[Pattern],[{AliasVar,Pat}]} - -case_data_pat_var(P, {Type,Arity}=TypeSig) -> - %% If the entire case statement is evaluated in an effect - %% context (e.g. "case {A,B} of ... end, ok"), there will - %% be a warning that a term is constructed but never used. - %% To avoid that warning, we must annotate the data - %% constructor as compiler generated. - Ann = [compiler_generated|cerl:get_ann(P)], +case_data_pat_alias(P, BindTo0, TypeSig, Bs0) -> case cerl:type(P) of - var -> - Vars = make_vars(cerl:get_ann(P), Arity), - {ok,Vars,[{P,cerl:ann_make_data(Ann, Type, Vars)}]}; alias -> - V = cerl:alias_var(P), - Apat = cerl:alias_pat(P), - {ok,Ps,Bs} = case_data_pat(Apat, TypeSig), - {ok,Ps,[{V,cerl:ann_make_data(Ann, Type, - pat_to_expr_list(Ps))}|Bs]} + %% Recursively handle the pattern and bind to + %% the alias variable. + BindTo = cerl:alias_var(P), + Apat0 = cerl:alias_pat(P), + Ann = [compiler_generated], + Apat = cerl:set_ann(Apat0, Ann), + {Ps,Bs} = case_data_pat_alias(Apat, BindTo, TypeSig, Bs0), + {Ps,[{BindTo0,BindTo}|Bs]}; + var -> + %% Here we will need to actually build the data and bind + %% it to the variable. + {Type,Arity} = TypeSig, + Vars = make_vars([], Arity), + Ann = [compiler_generated], + Data = cerl:ann_make_data(Ann, Type, Vars), + Bs = [{BindTo0,P},{P,Data}|Bs0], + {Vars,Bs}; + _ -> + %% Since case_opt_nomatch/3 has removed all clauses that + %% cannot match, we KNOW that this clause must match and + %% that the pattern must be a data constructor. + %% Here we must build the data and bind it to the variable. + {Type,_} = TypeSig, + DataEs = cerl:data_es(P), + Vars = pat_to_expr_list(DataEs), + Ann = [compiler_generated], + Data = cerl:ann_make_data(Ann, Type, Vars), + {DataEs,[{BindTo0,Data}]} end. %% pat_to_expr(Pattern) -> Expression. diff --git a/lib/compiler/test/core_fold_SUITE.erl b/lib/compiler/test/core_fold_SUITE.erl index bca8afb1b0..b45874433c 100644 --- a/lib/compiler/test/core_fold_SUITE.erl +++ b/lib/compiler/test/core_fold_SUITE.erl @@ -23,7 +23,8 @@ t_element/1,setelement/1,t_length/1,append/1,t_apply/1,bifs/1, eq/1,nested_call_in_case/1,guard_try_catch/1,coverage/1, unused_multiple_values_error/1,unused_multiple_values/1, - multiple_aliases/1,redundant_boolean_clauses/1,mixed_matching_clauses/1]). + multiple_aliases/1,redundant_boolean_clauses/1, + mixed_matching_clauses/1,unnecessary_building/1]). -export([foo/0,foo/1,foo/2,foo/3]). @@ -40,7 +41,8 @@ groups() -> [t_element,setelement,t_length,append,t_apply,bifs, eq,nested_call_in_case,guard_try_catch,coverage, unused_multiple_values_error,unused_multiple_values, - multiple_aliases,redundant_boolean_clauses,mixed_matching_clauses]}]. + multiple_aliases,redundant_boolean_clauses, + mixed_matching_clauses,unnecessary_building]}]. init_per_suite(Config) -> @@ -403,4 +405,29 @@ mixed_matching_clauses(Config) when is_list(Config) -> end, ok. +unnecessary_building(Config) when is_list(Config) -> + Term1 = do_unnecessary_building_1(test_lib:id(a)), + [{a,a},{a,a}] = Term1, + 7 = erts_debug:size(Term1), + + %% The Input term should not be rebuilt (thus, it should + %% only be counted once in the size of the combined term). + Input = test_lib:id({a,b,c}), + Term2 = test_lib:id(do_unnecessary_building_2(Input)), + {b,[{a,b,c},none],x} = Term2, + 4+4+4+2 = erts_debug:size([Term2|Input]), + + ok. + +do_unnecessary_building_1(S) -> + %% The tuple must only be built once. + F0 = F1 = {S,S}, + [F0,F1]. + +do_unnecessary_building_2({a,_,_}=T) -> + %% The T term should not be rebuilt. + {b, + [_,_] = [T,none], + x}. + id(I) -> I. diff --git a/lib/compiler/test/test_lib.erl b/lib/compiler/test/test_lib.erl index e06e42276a..e8f469c5b4 100644 --- a/lib/compiler/test/test_lib.erl +++ b/lib/compiler/test/test_lib.erl @@ -20,9 +20,11 @@ -include("test_server.hrl"). -compile({no_auto_import,[binary_part/2]}). --export([recompile/1,parallel/0,uniq/0,opt_opts/1,get_data_dir/1, +-export([id/1,recompile/1,parallel/0,uniq/0,opt_opts/1,get_data_dir/1, smoke_disasm/1,p_run/2,binary_part/2]). +id(I) -> I. + recompile(Mod) when is_atom(Mod) -> case whereis(cover_server) of undefined -> ok; -- cgit v1.2.3