From ff432e262e65243cbc983fcb002527f8fae8c78b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 9 Apr 2010 15:50:17 +0200 Subject: EEP-23: Allow variables in fun M:F/A Currently, the external fun syntax "fun M:F/A" only supports literals. That is, "fun lists:reverse/1" is allowed but not "fun M:F/A". In many real-life situations, some or all of M, F, A are not known until run-time, and one is forced to either use the undocumented erlang:make_fun/3 BIF or to use a "tuple fun" (which is deprecated). EEP-23 suggests that the parser (erl_parse) should immediately transform "fun M:F/A" to "erlang:make_fun(M, F, A)". We have not followed that approach in this implementation, because we want the abstract code to mirror the source code as closely as possible, and we also consider erlang:make_fun/3 to be an implementation detail that we might want to remove in the future. Instead, we will change the abstract format for "fun M:F/A" (in a way that is not backwards compatible), and while we are at it, we will move the translation from "fun M:F/A" to "erlang:make_fun(M, F, A)" from sys_pre_expand down to the v3_core pass. We will also update the debugger and xref to use the new format. We did consider making the abstract format backward compatible if no variables were used in the fun, but decided against it. Keeping it backward compatible would mean that there would be different abstract formats for the no-variable and variable case, and tools would have to handle both formats, probably forever. Reference: http://www.erlang.org/eeps/eep-0023.html --- lib/tools/src/xref_reader.erl | 25 ++++--- lib/tools/test/xref_SUITE.erl | 87 +++++++++++++++++++++++- lib/tools/test/xref_SUITE_data/fun_mfa_r14.beam | Bin 0 -> 1116 bytes lib/tools/test/xref_SUITE_data/fun_mfa_r14.erl | 18 +++++ 4 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 lib/tools/test/xref_SUITE_data/fun_mfa_r14.beam create mode 100644 lib/tools/test/xref_SUITE_data/fun_mfa_r14.erl (limited to 'lib/tools') diff --git a/lib/tools/src/xref_reader.erl b/lib/tools/src/xref_reader.erl index d22f0df164..92f0c45c7b 100644 --- a/lib/tools/src/xref_reader.erl +++ b/lib/tools/src/xref_reader.erl @@ -158,15 +158,20 @@ expr({'try',_Line,Es,Scs,Ccs,As}, S) -> S2 = clauses(Scs, S1), S3 = clauses(Ccs, S2), expr(As, S3); -expr({call, Line, - {remote, _, {atom,_,erlang}, {atom,_,make_fun}}, - [{atom,_,Mod}, {atom,_,Fun}, {integer,_,Arity}]}, S) -> - %% Added in R10B-6. M:F/A. - expr({'fun', Line, {function, Mod, Fun, Arity}}, S); -expr({'fun', Line, {function, Mod, Name, Arity}}, S) -> - %% Added in R10B-6. M:F/A. +expr({'fun', Line, {function, {atom,_,Mod}, + {atom,_,Name}, + {integer,_,Arity}}}, S) -> + %% New format in R15. M:F/A (literals). As = lists:duplicate(Arity, {atom, Line, foo}), external_call(Mod, Name, As, Line, false, S); +expr({'fun', Line, {function, Mod, Name, {integer,_,Arity}}}, S) -> + %% New format in R15. M:F/A (one or more variables). + As = lists:duplicate(Arity, {atom, Line, foo}), + external_call(erlang, apply, [Mod, Name, list2term(As)], Line, true, S); +expr({'fun', Line, {function, Mod, Name, _Arity}}, S) -> + %% New format in R15. M:F/A (one or more variables). + As = {var, Line, '_'}, + external_call(erlang, apply, [Mod, Name, As], Line, true, S); expr({'fun', Line, {function, Name, Arity}, _Extra}, S) -> %% Added in R8. handle_call(local, S#xrefr.module, Name, Arity, Line, S); @@ -286,10 +291,10 @@ check_funarg(W, ArgsList, Line, S) -> expr(ArgsList, S1). funarg({'fun', _, _Clauses, _Extra}, _S) -> true; -funarg({var, _, Var}, S) -> member(Var, S#xrefr.funvars); -funarg({call,_,{remote,_,{atom,_,erlang},{atom,_,make_fun}},_MFA}, _S) -> - %% R10B-6. M:F/A. +funarg({'fun', _, {function,_,_,_}}, _S) -> + %% New abstract format for fun M:F/A in R15. true; +funarg({var, _, Var}, S) -> member(Var, S#xrefr.funvars); funarg(_, _S) -> false. fun_args(apply2, [FunArg, Args]) -> {FunArg, Args}; diff --git a/lib/tools/test/xref_SUITE.erl b/lib/tools/test/xref_SUITE.erl index 2f83ab4995..e0876381ca 100644 --- a/lib/tools/test/xref_SUITE.erl +++ b/lib/tools/test/xref_SUITE.erl @@ -46,7 +46,8 @@ -export([ add/1, default/1, info/1, lib/1, read/1, read2/1, remove/1, replace/1, update/1, deprecated/1, trycatch/1, - abstract_modules/1, fun_mfa/1, qlc/1]). + abstract_modules/1, fun_mfa/1, fun_mfa_r14/1, + fun_mfa_vars/1, qlc/1]). -export([ analyze/1, basic/1, md/1, q/1, variables/1, unused_locals/1]). @@ -82,7 +83,7 @@ groups() -> {files, [], [add, default, info, lib, read, read2, remove, replace, update, deprecated, trycatch, abstract_modules, fun_mfa, - qlc]}, + fun_mfa_r14, fun_mfa_vars, qlc]}, {analyses, [], [analyze, basic, md, q, variables, unused_locals]}, {misc, [], [format_error, otp_7423, otp_7831]}]. @@ -1771,6 +1772,88 @@ fun_mfa(Conf) when is_list(Conf) -> ?line ok = file:delete(Beam), ok. +%% Same as the previous test case, except that we use a BEAM file +%% that was compiled by an R14 compiler to test backward compatibility. +fun_mfa_r14(Conf) when is_list(Conf) -> + Dir = ?config(data_dir, Conf), + MFile = fname(Dir, "fun_mfa_r14"), + + A = fun_mfa_r14, + {ok, _} = xref:start(s), + {ok, A} = xref:add_module(s, MFile, {warnings,false}), + {ok, [{{{A,t,0},{'$M_EXPR','$F_EXPR',0}},[7]}, + {{{A,t,0},{A,t,0}},[6]}, + {{{A,t1,0},{'$M_EXPR','$F_EXPR',0}},[11]}, + {{{A,t1,0},{A,t,0}},[10]}, + {{{A,t2,0},{A,t,0}},[14]}, + {{{A,t3,0},{A,t3,0}},[17]}]} = + xref:q(s, "(Lin) E"), + + ok = check_state(s), + xref:stop(s), + + ok. + +%% fun M:F/A with varibles. +fun_mfa_vars(Conf) when is_list(Conf) -> + Dir = ?copydir, + File = fname(Dir, "fun_mfa_vars.erl"), + MFile = fname(Dir, "fun_mfa_vars"), + Beam = fname(Dir, "fun_mfa_vars.beam"), + Test = <<"-module(fun_mfa_vars). + + -export([t/1, t1/1, t2/3]). + + t(Mod) -> + F = fun Mod:bar/2, + (F)(a, b). + + t1(Name) -> + F = fun ?MODULE:Name/1, + (F)(a). + + t2(Mod, Name, Arity) -> + F = fun Mod:Name/Arity, + (F)(a). + + t3(Arity) -> + F = fun ?MODULE:t/Arity, + (F)(1, 2, 3). + + t4(Mod, Name) -> + F = fun Mod:Name/3, + (F)(a, b, c). + + t5(Mod, Arity) -> + F = fun Mod:t/Arity, + (F)(). + ">>, + + ok = file:write_file(File, Test), + A = fun_mfa_vars, + {ok, A} = compile:file(File, [report,debug_info,{outdir,Dir}]), + {ok, _} = xref:start(s), + {ok, A} = xref:add_module(s, MFile, {warnings,false}), + {ok, [{{{A,t,1},{'$M_EXPR','$F_EXPR',2}},[7]}, + {{{A,t,1},{'$M_EXPR',bar,2}},[6]}, + {{{A,t1,1},{'$M_EXPR','$F_EXPR',1}},[11]}, + {{{A,t1,1},{A,'$F_EXPR',1}},[10]}, + {{{A,t2,3},{'$M_EXPR','$F_EXPR',-1}},[14]}, + {{{A,t2,3},{'$M_EXPR','$F_EXPR',1}},[15]}, + {{{A,t3,1},{'$M_EXPR','$F_EXPR',3}},[19]}, + {{{A,t3,1},{fun_mfa_vars,t,-1}},[18]}, + {{{A,t4,2},{'$M_EXPR','$F_EXPR',3}},[22,23]}, + {{{A,t5,2},{'$M_EXPR','$F_EXPR',0}},[27]}, + {{{A,t5,2},{'$M_EXPR',t,-1}},[26]}]} = + xref:q(s, "(Lin) E"), + + ok = check_state(s), + xref:stop(s), + + ok = file:delete(File), + ok = file:delete(Beam), + ok. + qlc(suite) -> []; qlc(doc) -> ["OTP-5195: A bug fix when using qlc:q/1,2."]; qlc(Conf) when is_list(Conf) -> diff --git a/lib/tools/test/xref_SUITE_data/fun_mfa_r14.beam b/lib/tools/test/xref_SUITE_data/fun_mfa_r14.beam new file mode 100644 index 0000000000..4645525690 Binary files /dev/null and b/lib/tools/test/xref_SUITE_data/fun_mfa_r14.beam differ diff --git a/lib/tools/test/xref_SUITE_data/fun_mfa_r14.erl b/lib/tools/test/xref_SUITE_data/fun_mfa_r14.erl new file mode 100644 index 0000000000..293bd83a8b --- /dev/null +++ b/lib/tools/test/xref_SUITE_data/fun_mfa_r14.erl @@ -0,0 +1,18 @@ +-module(fun_mfa_r14). + +-export([t/0, t1/0, t2/0, t3/0]). + +t() -> + F = fun ?MODULE:t/0, + (F)(). + +t1() -> + F = fun t/0, + (F)(). + +t2() -> + fun ?MODULE:t/0(). + +t3() -> + fun t3/0(). + -- cgit v1.2.3 From af5edfe20b24116fa892a37a746f4053fde6039b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 29 Sep 2011 08:13:24 +0200 Subject: beam_asm: Strenghten the calculation of Uniq for funs Funs are identified by a triple, , where Module is the module name, Uniq is a 27 bit hash value of some intermediate representation of the code for the fun, and index is a small integer. When a fun is loaded, the triple for the fun will be compared to previously loaded funs. If all elements in the triple in the newly loaded fun are the same, the newly loaded fun will replace the previous fun. The idea is that if Uniq are the same, the code for the fun is also the same. The problem is that Uniq is only based on the intermediate representation of the fun itself. If the fun calls local functions in the same module, Uniq may remain the same even if the behavior of the fun has been changed. See http://erlang.org/pipermail/erlang-bugs/2007-June/000368.htlm for an example. As a long-term plan to fix this problem, the NewIndex and NewUniq fields was added to each fun in the R8 release (where NewUniq is the MD5 of the BEAM code for the module). Unfortunately, it turns out that the compiler does not assign unique value to NewIndex (if it isn't tested, it doesn't work), so we cannot use the triple as identification. It would be possible to use , but that seems ugly. Therefore, fix the problem by making Uniq more unique by taking 27 bits from the MD5 for the BEAM code. That only requires a change to the compiler. Also update a test case for cover, which now fails because of the stronger Uniq calculation. (The comment in test case about why the Pid2 process survived is not correct.) --- lib/tools/test/cover_SUITE.erl | 31 ++++++++----------------- lib/tools/test/cover_SUITE_data/otp_6115/f1.erl | 11 +++++---- 2 files changed, 16 insertions(+), 26 deletions(-) (limited to 'lib/tools') diff --git a/lib/tools/test/cover_SUITE.erl b/lib/tools/test/cover_SUITE.erl index fe7f92de78..881a3c2997 100644 --- a/lib/tools/test/cover_SUITE.erl +++ b/lib/tools/test/cover_SUITE.erl @@ -583,21 +583,14 @@ otp_6115_1(Config) -> %% called -- running cover compiled code when there is no cover %% server and thus no ets tables to bump counters in, makes no %% sense. - ?line Pid1 = f1:start_fail(), - - %% If f1 is cover compiled, a process P is started with a - %% reference to the fun created in start_ok/0, and - %% cover:stop() is called, then P should survive. - %% This is because (the fun held by) P always references the current - %% version of the module, and is thus not affected by the cover - %% compiled version being unloaded. - ?line Pid2 = f1:start_ok(), + Pid1 = f1:start_a(), + Pid2 = f1:start_b(), %% Now stop cover ?line cover:stop(), - %% Ensure that f1 is loaded (and not cover compiled), that Pid1 - %% is dead and Pid2 is alive, but with no reference to old code + %% Ensure that f1 is loaded (and not cover compiled), and that + %% both Pid1 and Pid2 are dead. case code:which(f1) of Beam when is_list(Beam) -> ok; @@ -608,19 +601,15 @@ otp_6115_1(Config) -> undefined -> ok; _PI1 -> - RefToOldP = erlang:check_process_code(Pid1, f1), - ?line ?t:fail({"Pid1 still alive", RefToOldP}) + RefToOldP1 = erlang:check_process_code(Pid1, f1), + ?t:fail({"Pid1 still alive", RefToOldP1}) end, case process_info(Pid2) of - PI2 when is_list(PI2) -> - case erlang:check_process_code(Pid2, f2) of - false -> - ok; - true -> - ?line ?t:fail("Pid2 has ref to old code") - end; undefined -> - ?line ?t:fail("Pid2 has died") + ok; + _PI2 -> + RefToOldP2 = erlang:check_process_code(Pid1, f2), + ?t:fail({"Pid2 still alive", RefToOldP2}) end, ?line file:set_cwd(CWD), diff --git a/lib/tools/test/cover_SUITE_data/otp_6115/f1.erl b/lib/tools/test/cover_SUITE_data/otp_6115/f1.erl index b659e5d818..5399b33f19 100644 --- a/lib/tools/test/cover_SUITE_data/otp_6115/f1.erl +++ b/lib/tools/test/cover_SUITE_data/otp_6115/f1.erl @@ -1,12 +1,13 @@ -module(f1). --export([start_fail/0, start_ok/0]). +-export([start_a/0, start_b/0]). -start_fail() -> +start_a() -> f2:start(fun() -> - io:format("this does not work\n",[]) + ok end). -start_ok() -> +start_b() -> f2:start(fun fun1/0). + fun1() -> - io:format("this works\n",[]). + ok. -- cgit v1.2.3