Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:Ledest:erlang:23
erlang
2170-Eliminate-confusing-case_clause-exception....
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 2170-Eliminate-confusing-case_clause-exception.patch of Package erlang
From fe244ad80c1b8621fc499f94cac758b90d063831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org> Date: Tue, 11 Jan 2022 08:53:09 +0100 Subject: [PATCH] Eliminate confusing `case_clause` exception Consider this module: -module(bug). -export([foo/0]). foo() -> fun(a) -> ok end(b). The call to the fun will always fail, which will be noted by the compiler: 1> c(bug). bug.erl:5:5: Warning: no clause will ever match % 5| fun(a) -> ok end(b). % | ^ {ok,bug} What is unexpected is that the exception that is raised is not a `function_clause` exception: 2> bug:foo(). ** exception error: no case clause matching {b} in function bug:foo/0 (bug.erl, line 5) This confusing `case_clause` exception started to appear in OTP 24 because of 72675baaa9fd30 (#4545) that inlines funs that are immediately used. Before OTP 24, when the fun was not inlined, the exception would be: 2> bug:foo(). ** exception error: no function clause matching bug:'-foo/0-fun-0-'(b) (bug.erl, line 5) The reason that `function_clause` exceptions are rewritten to `case_clause` exceptions in inlined code is to avoid the even more confusing and misleading exception: 2> bug:foo(). ** exception error: no function clause matching bug:foo(b) (bug.erl, line 5) This is confusing because it seems that `foo/0` was called with one argument. To reduce the confusion, this commmit ensures that `function_clause` exceptions in inlined code remains `function_clause` exceptions, but with a generated name that makes it clear that the `function_clause` exception occurred in a fun: 2> bug:foo(). ** exception error: no function clause matching bug:'-foo/0-inlined-0-'(b) (bug.erl, line 5) Fixes #5513 --- lib/compiler/src/beam_kernel_to_ssa.erl | 13 ++- lib/compiler/src/beam_ssa_pre_codegen.erl | 128 ++++++++++++++++------ lib/compiler/src/beam_validator.erl | 13 ++- lib/compiler/src/v3_core.erl | 2 +- lib/compiler/src/v3_kernel.erl | 49 ++++++--- lib/compiler/test/beam_except_SUITE.erl | 8 ++ lib/compiler/test/bs_match_SUITE.erl | 7 +- lib/compiler/test/guard_SUITE.erl | 3 +- lib/compiler/test/inline_SUITE.erl | 2 +- 9 files changed, 165 insertions(+), 60 deletions(-) diff --git a/lib/compiler/src/beam_kernel_to_ssa.erl b/lib/compiler/src/beam_kernel_to_ssa.erl index c2e22f4b6d..7722078206 100644 --- a/lib/compiler/src/beam_kernel_to_ssa.erl +++ b/lib/compiler/src/beam_kernel_to_ssa.erl @@ -25,7 +25,7 @@ -export([module/2]). -import(lists, [all/2,append/1,flatmap/2,foldl/3, - keysort/2,mapfoldl/3,map/2,member/2, + keyfind/3,keysort/2,mapfoldl/3,map/2,member/2, reverse/1,reverse/2,sort/1]). -include("v3_kernel.hrl"). @@ -672,7 +672,7 @@ call_cg(Func, As, [#k_var{name=R}|MoreRs]=Rs, Le, St0) -> %% Ordinary function call in a function body. Args = ssa_args([Func|As], St0), {Ret,St1} = new_ssa_var(R, St0), - Call = #b_set{anno=line_anno(Le),op=call,dst=Ret,args=Args}, + Call = #b_set{anno=call_anno(Le),op=call,dst=Ret,args=Args}, %% If this is a call to erlang:error(), MoreRs could be a %% nonempty list of variables that each need a value. @@ -685,9 +685,16 @@ call_cg(Func, As, [#k_var{name=R}|MoreRs]=Rs, Le, St0) -> enter_cg(Func, As0, Le, St0) -> As = ssa_args([Func|As0], St0), {Ret,St} = new_ssa_var('@ssa_ret', St0), - Call = #b_set{anno=line_anno(Le),op=call,dst=Ret,args=As}, + Call = #b_set{anno=call_anno(Le),op=call,dst=Ret,args=As}, {[Call,#b_ret{arg=Ret}],St}. +call_anno(Le) -> + Anno = line_anno(Le), + case keyfind(inlined, 1, Le) of + false -> Anno; + {inlined,NameArity} -> Anno#{inlined => NameArity} + end. + %% bif_cg(#k_bif{}, Le,State) -> {[Ainstr],State}. %% Generate code for a guard BIF or primop. diff --git a/lib/compiler/src/beam_ssa_pre_codegen.erl b/lib/compiler/src/beam_ssa_pre_codegen.erl index 3567fb4210..f5ad81f760 100644 --- a/lib/compiler/src/beam_ssa_pre_codegen.erl +++ b/lib/compiler/src/beam_ssa_pre_codegen.erl @@ -72,7 +72,8 @@ -import(lists, [all/2,any/2,append/1,duplicate/2, foldl/3,last/1,member/2,partition/2, - reverse/1,reverse/2,sort/1,splitwith/2,zip/2]). + reverse/1,reverse/2,seq/2,sort/1, + splitwith/2,usort/1,zip/2]). -spec module(beam_ssa:b_module(), [compile:option()]) -> {'ok',beam_ssa:b_module()}. @@ -80,7 +81,8 @@ module(#b_module{body=Fs0}=Module, Opts) -> UseBSM3 = not proplists:get_bool(no_bsm3, Opts), Ps = passes(Opts), - Fs = functions(Fs0, Ps, UseBSM3), + Fs1 = functions(Fs0, Ps, UseBSM3), + Fs = fc_stubs(Fs1, Module), {ok,Module#b_module{body=Fs}}. functions([F|Fs], Ps, UseBSM3) -> @@ -835,19 +837,21 @@ match_fail_instrs_1([{L,#b_blk{is=Is0}=Blk}|Bs], Arity, Blocks0) -> match_fail_instrs_1([], _Arity, Blocks) -> Blocks. match_fail_instrs_blk([#b_set{op=put_tuple,dst=Dst, - args=[#b_literal{val=Tag},Val]}, + args=[#b_literal{val=Tag}|Values]}, #b_set{op=call, args=[#b_remote{mod=#b_literal{val=erlang}, name=#b_literal{val=error}}, Dst]}=Call|Is], _Arity, Acc) -> - match_fail_instr(Call, Tag, Val, Is, Acc); + match_fail_instr(Call, Tag, Values, Is, Acc); match_fail_instrs_blk([#b_set{op=call, args=[#b_remote{mod=#b_literal{val=erlang}, name=#b_literal{val=error}}, - #b_literal{val={Tag,Val}}]}=Call|Is], - _Arity, Acc) -> - match_fail_instr(Call, Tag, #b_literal{val=Val}, Is, Acc); + #b_literal{val=Tuple}]}=Call|Is], + _Arity, Acc) when tuple_size(Tuple) >= 1 -> + [Tag|Values0] = tuple_to_list(Tuple), + Values = [#b_literal{val=V} || V <- Values0], + match_fail_instr(Call, Tag, Values, Is, Acc); match_fail_instrs_blk([#b_set{op=call, args=[#b_remote{mod=#b_literal{val=erlang}, name=#b_literal{val=error}}, @@ -860,34 +864,29 @@ match_fail_instrs_blk([#b_set{op=call,anno=Anno, name=#b_literal{val=error}}, #b_literal{val=function_clause}, Stk]}=Call], - {Arity,Location}, Acc) -> - case match_fail_stk(Stk, Acc, [], []) of - {[_|_]=Vars,Is} when length(Vars) =:= Arity -> - case maps:get(location, Anno, none) of - Location -> - I = Call#b_set{op=match_fail, - args=[#b_literal{val=function_clause}|Vars]}, - Is ++ [I]; - _ -> - %% erlang:error/2 has a different location than the - %% func_info instruction at the beginning of the function - %% (probably because of inlining). Keep the original call. - reverse(Acc, [Call]) - end; - _ -> - %% Either the stacktrace could not be picked apart (for example, - %% if the call to erlang:error/2 was handwritten) or the number - %% of arguments in the stacktrace was different from the arity - %% of the host function (because it is the implementation of a - %% fun). Keep the original call. - reverse(Acc, [Call]) - end; + Arity, Acc) -> + match_fail_fc(Anno, Call, Stk, Arity, Acc); match_fail_instrs_blk([I|Is], Arity, Acc) -> match_fail_instrs_blk(Is, Arity, [I|Acc]); match_fail_instrs_blk(_, _, _) -> none. -match_fail_instr(Call, Tag, Val, Is, Acc) -> +match_fail_instr(Call, function_clause, Values, Is, Acc) -> + case beam_ssa:get_anno(inlined, Call, none) of + none -> + %% If there is no `inlined` annotation, it implies that + %% the call to erlang:error/1 was handwritten. + none; + {Name,Arity} -> + %% A `function_clause` in inlined code. Convert it to + %% a call to a stub function that will raise a proper + %% `function_clause` exception. (The stub function will + %% be created later by fc_stubs/2.) + Target = #b_local{name=#b_literal{val=Name},arity=Arity}, + I = Call#b_set{args=[Target|Values]}, + reverse(Acc, [I|Is]) + end; +match_fail_instr(Call, Tag, [Val], Is, Acc) -> Op = case Tag of badmatch -> Tag; case_clause -> case_end; @@ -900,19 +899,84 @@ match_fail_instr(Call, Tag, Val, Is, Acc) -> _ -> I = Call#b_set{op=match_fail,args=[#b_literal{val=Op},Val]}, reverse(Acc, [I|Is]) + end; +match_fail_instr(_, _, _, _, _) -> none. + +match_fail_fc(Anno, Call, Stk, {Arity,Location}, Acc) -> + case match_fail_stk(Stk, Acc, [], []) of + {[_|_]=Vars,Is} when length(Vars) =:= Arity -> + case maps:get(location, Anno, none) of + Location -> + I = Call#b_set{op=match_fail, + args=[#b_literal{val=function_clause}|Vars]}, + Is ++ [I]; + _ -> + %% erlang:error/2 has a different location than + %% the func_info instruction at the beginning of + %% the function (probably because of + %% inlining). Keep the original call. + reverse(Acc, [Call]) + end; + _ -> + %% Either the stacktrace could not be picked apart (for + %% example, if the call to erlang:error/2 was handwritten) + %% or the number of arguments in the stacktrace was + %% different from the arity of the host function (because + %% it is the implementation of a fun). Keep the original + %% call. + reverse(Acc, [Call]) end. match_fail_stk(#b_var{}=V, [#b_set{op=put_list,dst=V,args=[H,T]}|Is], IAcc, VAcc) -> match_fail_stk(T, Is, IAcc, [H|VAcc]); match_fail_stk(#b_literal{val=[H|T]}, Is, IAcc, VAcc) -> match_fail_stk(#b_literal{val=T}, Is, IAcc, [#b_literal{val=H}|VAcc]); -match_fail_stk(#b_literal{val=[]}, [], IAcc, VAcc) -> - {reverse(VAcc),IAcc}; +match_fail_stk(#b_literal{val=[]}, Is, IAcc, VAcc) -> + {reverse(VAcc),reverse(Is, IAcc)}; match_fail_stk(T, [#b_set{op=Op}=I|Is], IAcc, VAcc) when Op =:= bs_get_tail; Op =:= bs_set_position -> match_fail_stk(T, Is, [I|IAcc], VAcc); match_fail_stk(_, _, _, _) -> none. +%% Create stubs for `function_clause` exceptions generated by +%% inlined code. +fc_stubs(Fs, #b_module{name=Mod}) -> + Stubs0 = usort(find_fc_calls(Fs, [])), + Stubs = [begin + Seq = seq(0, Arity-1), + Args = [#b_var{name=V} || V <- Seq], + XRegs = [{x,V} || V <- Seq], + Ret = #b_var{name='@ssa_ret'}, + Regs = maps:from_list([{Ret,{x,0}}|zip(Args, XRegs)]), + Anno = #{func_info => {Mod,Name,Arity}, + location => Location, + parameter_info => #{}, + registers => Regs}, + Fc = #b_set{op=match_fail,dst=Ret, + args=[#b_literal{val=function_clause}|Args]}, + Blk = #b_blk{is=[Fc],last=#b_ret{arg=Ret}}, + #b_function{anno=Anno,args=Args, + bs=#{0 => Blk}, + cnt=1} + end || {{Name,Arity},Location} <- Stubs0], + Fs ++ Stubs. + +find_fc_calls([#b_function{bs=Blocks}|Fs], Acc0) -> + F = fun(#b_set{anno=Anno,op=call}, A) -> + case Anno of + #{inlined := FA} -> + [{FA,maps:get(location, Anno, [])}|A]; + #{} -> + A + end; + (_, A) -> + A + end, + Acc = beam_ssa:fold_instrs(F, maps:keys(Blocks), Acc0, Blocks), + find_fc_calls(Fs, Acc); +find_fc_calls([], Acc) -> Acc. + + %%% %%% Fix tuples. %%% diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl index 125c01c6a6..adbddc7059 100644 --- a/lib/compiler/src/beam_validator.erl +++ b/lib/compiler/src/beam_validator.erl @@ -245,7 +245,8 @@ build_function_table([{function,Name,Arity,Entry,Code0}|Fs], Acc) -> case Code of [{label,Entry} | Is] -> Info = #{ arity => Arity, - parameter_info => find_parameter_info(Is, #{}) }, + parameter_info => find_parameter_info(Is, #{}), + always_fails => always_fails(Is) }, build_function_table(Fs, Acc#{ Entry => Info }); _ -> %% Something is seriously wrong. Ignore it for now. @@ -262,6 +263,9 @@ find_parameter_info([{'%', _} | Is], Acc) -> find_parameter_info(_, Acc) -> Acc. +always_fails([{jump,_}|_]) -> true; +always_fails(_) -> false. + validate_1(Is, MFA0, Entry, Level, Ft) -> {Offset, MFA, Header, Body} = extract_header(Is, MFA0, Entry, 1, []), @@ -3078,6 +3082,13 @@ will_call_succeed({extfunc,M,F,A}, Vst) -> beam_call_types:will_succeed(M, F, get_call_args(A, Vst)); will_call_succeed(bs_init_writable, _Vst) -> yes; +will_call_succeed({f,Lbl}, #vst{ft=Ft}) -> + case Ft of + #{Lbl := #{always_fails := true}} -> + no; + #{} -> + maybe + end; will_call_succeed(_Call, _Vst) -> maybe. diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl index 6bf87e131c..8e89c88318 100644 --- a/lib/compiler/src/v3_core.erl +++ b/lib/compiler/src/v3_core.erl @@ -264,7 +264,7 @@ body(Cs0, Name, Arity, St0) -> Args = reverse(Args0), %Nicer order {Cs1,St2} = clauses(Cs0, St1), {Ps,St3} = new_vars(Arity, St2), %Need new variables here - Fc = function_clause(Ps, Anno), + Fc = function_clause(Ps, FunAnno), {#ifun{anno=#a{anno=FunAnno},id=[],vars=Args,clauses=Cs1,fc=Fc},St3}. %% clause(Clause, State) -> {Cclause,State}. diff --git a/lib/compiler/src/v3_kernel.erl b/lib/compiler/src/v3_kernel.erl index aa3fc033eb..aa8a906ca7 100644 --- a/lib/compiler/src/v3_kernel.erl +++ b/lib/compiler/src/v3_kernel.erl @@ -405,36 +405,51 @@ letrec_goto([{#c_var{name={Label,0}},Cfail}], Cb, Sub0, %% erlang:error/2. translate_match_fail(Arg, Sub, Anno, St0) -> - Cargs = case {cerl:data_type(Arg),cerl:data_es(Arg)} of - {tuple,[#c_literal{val=function_clause}|As]} -> - translate_fc_args(As, Sub, St0); - {_,_} -> - [Arg] - end, - {Kargs,Ap,St} = atomic_list(Cargs, Sub, St0), + {Cargs,ExtraAnno,St1} = + case {cerl:data_type(Arg),cerl:data_es(Arg)} of + {tuple,[#c_literal{val=function_clause}|As]} -> + translate_fc_args(As, Sub, Anno, St0); + {_,_} -> + {[Arg],[],St0} + end, + {Kargs,Ap,St} = atomic_list(Cargs, Sub, St1), Ar = length(Cargs), - Call = #k_call{anno=Anno, + Call = #k_call{anno=ExtraAnno++Anno, op=#k_remote{mod=#k_literal{val=erlang}, name=#k_literal{val=error}, arity=Ar},args=Kargs}, {Call,Ap,St}. -translate_fc_args(As, Sub, #kern{fargs=Fargs}) -> +translate_fc_args(As, Sub, Anno, #kern{fargs=Fargs}=St0) -> case same_args(As, Fargs, Sub) of true -> %% The arguments for the `function_clause` exception are %% the arguments for the current function in the correct %% order. - [#c_literal{val=function_clause},cerl:make_list(As)]; + {[#c_literal{val=function_clause},cerl:make_list(As)], + [], + St0}; false -> %% The arguments in the `function_clause` exception don't - %% match the arguments for the current function because - %% of inlining. Keeping the `function_clause` - %% exception reason would be confusing. Rewrite it to - %% a `case_clause` exception with the arguments in a - %% tuple. - [cerl:c_tuple([#c_literal{val=case_clause}, - cerl:c_tuple(As)])] + %% match the arguments for the current function because of + %% inlining. + Args = [cerl:c_tuple([#c_literal{val=function_clause}|As])], + case keyfind(function, 1, Anno) of + false -> + %% This is probably a fun that has been inlined + %% by sys_core_fold. + {Name,St1} = new_fun_name("inlined", St0), + {Args, + [{inlined,{Name,length(As)}}], + St1}; + {_,{Name0,Arity}} -> + %% This is function that has been inlined. + Name1 = ["-inlined-",Name0,"/",Arity,"-"], + Name = list_to_atom(lists:concat(Name1)), + {Args, + [{inlined,{Name,Arity}}], + St0} + end end. same_args([#c_var{name=Cv}|Vs], [#k_var{name=Kv}|As], Sub) -> diff --git a/lib/compiler/test/beam_except_SUITE.erl b/lib/compiler/test/beam_except_SUITE.erl index 95e5989081..2bc805be71 100644 --- a/lib/compiler/test/beam_except_SUITE.erl +++ b/lib/compiler/test/beam_except_SUITE.erl @@ -135,14 +135,18 @@ coverage(_) -> {'EXIT',{function_clause,[{?MODULE,foobar,[[fail],1,2], [{file,"fake.erl"},{line,16}]}|_]}} = (catch foobar([fail], 1, 2)), + {'EXIT',{function_clause,[{?MODULE,fake_function_clause1,[{a,b},42.0],_}|_]}} = (catch fake_function_clause1({a,b})), {'EXIT',{function_clause,[{?MODULE,fake_function_clause2,[42|bad_tl],_}|_]}} = (catch fake_function_clause2(42, bad_tl)), + {'EXIT',{function_clause,[{?MODULE,fake_function_clause3,[x,y],_}|_]}} = (catch fake_function_clause3(42, id([x,y]))), + {'EXIT',{{function_clause,a,b,c}, _}} = catch fake_function_clause4(), + {'EXIT',{{badmatch,0.0},_}} = (catch coverage_1(id(42))), {'EXIT',{badarith,_}} = (catch coverage_1(id(a))), @@ -153,9 +157,13 @@ coverage_1(X) -> true = 0 / X. fake_function_clause1(A) -> error(function_clause, [A,42.0]). + fake_function_clause2(A, Tl) -> error(function_clause, [A|Tl]). + fake_function_clause3(_, Stk) -> error(function_clause, Stk). +fake_function_clause4() -> error({function_clause,a,b,c}). + binary_construction_allocation(_Config) -> ok = do_binary_construction_allocation("PUT"), ok. diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl index f7542be568..96ae549b28 100644 --- a/lib/compiler/test/bs_match_SUITE.erl +++ b/lib/compiler/test/bs_match_SUITE.erl @@ -1455,13 +1455,14 @@ haystack_2(Haystack) -> fc({'EXIT',{function_clause,_}}) -> ok; fc({'EXIT',{{case_clause,_},_}}) when ?MODULE =:= bs_match_inline_SUITE -> ok. -fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,Name,Args,_}|_]}}) -> ok; +fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,Name,Args,_}|_]}}) -> + ok; fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,Name,Arity,_}|_]}}) when length(Args) =:= Arity -> true = test_server:is_native(?MODULE); -fc(_, Args, {'EXIT',{{case_clause,ActualArgs},_}}) +fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,_,Args,_}|_]}}) when ?MODULE =:= bs_match_inline_SUITE -> - Args = tuple_to_list(ActualArgs). + ok. %% Cover the clause handling bs_context to binary in %% beam_block:initialized_regs/2. diff --git a/lib/compiler/test/guard_SUITE.erl b/lib/compiler/test/guard_SUITE.erl index 6699d82262..6a42453654 100644 --- a/lib/compiler/test/guard_SUITE.erl +++ b/lib/compiler/test/guard_SUITE.erl @@ -3070,5 +3070,4 @@ check(F, Result) -> ct:fail(check_failed) end. -fc({'EXIT',{function_clause,_}}) -> ok; -fc({'EXIT',{{case_clause,_},_}}) when ?MODULE =:= guard_inline_SUITE -> ok. +fc({'EXIT',{function_clause,_}}) -> ok. diff --git a/lib/compiler/test/inline_SUITE.erl b/lib/compiler/test/inline_SUITE.erl index 7482f53ed4..b2778bebcd 100644 --- a/lib/compiler/test/inline_SUITE.erl +++ b/lib/compiler/test/inline_SUITE.erl @@ -332,7 +332,7 @@ badarg(Reply, _A) -> Reply. otp_7223(Config) when is_list(Config) -> - {'EXIT', {{case_clause,{1}},_}} = (catch otp_7223_1(1)), + {'EXIT', {function_clause, [{?MODULE,_,[1],_}|_]}} = (catch otp_7223_1(1)), ok. -compile({inline,[{otp_7223_1,1}]}). -- 2.31.1
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
