From 1a23202000716bbc430f1212c06d7ef622acdd3e Mon Sep 17 00:00:00 2001 From: Stavros Aronis Date: Wed, 26 Mar 2014 18:06:09 +0100 Subject: Dialyzer now plays nicely with funs that come as "external" arguments Two steps are needed to make this work: 1) Avoid generating the additional "apply_constraint" in dialyzer_typesig by reporting every function argument as a potential external function (patch on dialyzer_dep). This will produce correct success typings for all functions in the test case, but dataflow would miss the key warnings that help identify the bugs. 2) Patch dialyzer_dataflow so that it uses the "handle just external" path as a fallback whenever there are any external calls. As a result, if we have info about some paths, then: a) use the old "handle known apply" code to mark these functions as used and b) ignore the generalized result and use the one found by typesig for the return value of the apply itself. --- lib/dialyzer/src/dialyzer_dataflow.erl | 51 +++++++------ lib/dialyzer/src/dialyzer_dep.erl | 6 +- .../small_SUITE_data/results/funs_from_outside | 7 ++ .../small_SUITE_data/src/funs_from_outside.erl | 83 ++++++++++++++++++++++ 4 files changed, 123 insertions(+), 24 deletions(-) create mode 100644 lib/dialyzer/test/small_SUITE_data/results/funs_from_outside create mode 100644 lib/dialyzer/test/small_SUITE_data/src/funs_from_outside.erl (limited to 'lib/dialyzer') diff --git a/lib/dialyzer/src/dialyzer_dataflow.erl b/lib/dialyzer/src/dialyzer_dataflow.erl index 692684cd99..e0873b17f8 100644 --- a/lib/dialyzer/src/dialyzer_dataflow.erl +++ b/lib/dialyzer/src/dialyzer_dataflow.erl @@ -363,20 +363,24 @@ traverse_list([], Map, State, Acc) -> handle_apply(Tree, Map, State) -> Args = cerl:apply_args(Tree), Op = cerl:apply_op(Tree), - {State1, Map1, ArgTypes} = traverse_list(Args, Map, State), - {State2, Map2, OpType} = traverse(Op, Map1, State1), + {State0, Map1, ArgTypes} = traverse_list(Args, Map, State), + {State1, Map2, OpType} = traverse(Op, Map1, State0), case any_none(ArgTypes) of true -> - {State2, Map2, t_none()}; + {State1, Map2, t_none()}; false -> - {CallSitesKnown, FunList} = - case state__lookup_call_site(Tree, State2) of - error -> {false, []}; - {ok, [external]} -> {false, []}; - {ok, List} -> {true, List} + FunList = + case state__lookup_call_site(Tree, State) of + error -> [external]; %% so that we go directly in the fallback + {ok, List} -> List end, - case CallSitesKnown of - false -> + FunInfoList = [{local, state__fun_info(Fun, State)} || Fun <- FunList], + case + handle_apply_or_call(FunInfoList, Args, ArgTypes, Map2, Tree, State1) + of + {had_external, State2} -> + %% Fallback: use whatever info we collected from traversing the op + %% instead of the result that has been generalized to t_any(). Arity = length(Args), OpType1 = t_inf(OpType, t_fun(Arity, t_any())), case t_is_none(OpType1) of @@ -408,25 +412,23 @@ handle_apply(Tree, Map, State) -> {State2, enter_type(Op, OpType1, Map3), Range} end end; - true -> - FunInfoList = [{local, state__fun_info(Fun, State)} - || Fun <- FunList], - handle_apply_or_call(FunInfoList, Args, ArgTypes, Map2, Tree, State1) + Normal -> Normal end end. handle_apply_or_call(FunInfoList, Args, ArgTypes, Map, Tree, State) -> None = t_none(), handle_apply_or_call(FunInfoList, Args, ArgTypes, Map, Tree, State, - [None || _ <- ArgTypes], None). + [None || _ <- ArgTypes], None, false). handle_apply_or_call([{local, external}|Left], Args, ArgTypes, Map, Tree, State, - _AccArgTypes, _AccRet) -> + _AccArgTypes, _AccRet, _HadExternal) -> handle_apply_or_call(Left, Args, ArgTypes, Map, Tree, State, - ArgTypes, t_any()); + ArgTypes, t_any(), true); handle_apply_or_call([{TypeOfApply, {Fun, Sig, Contr, LocalRet}}|Left], Args, ArgTypes, Map, Tree, - #state{opaques = Opaques} = State, AccArgTypes, AccRet) -> + #state{opaques = Opaques} = State, + AccArgTypes, AccRet, HadExternal) -> Any = t_any(), AnyArgs = [Any || _ <- Args], GenSig = {AnyArgs, fun(_) -> t_any() end}, @@ -573,11 +575,16 @@ handle_apply_or_call([{TypeOfApply, {Fun, Sig, Contr, LocalRet}}|Left], NewAccRet = t_sup(AccRet, TotalRet), ?debug("NewAccRet: ~s\n", [t_to_string(NewAccRet)]), handle_apply_or_call(Left, Args, ArgTypes, Map, Tree, - State3, NewAccArgTypes, NewAccRet); + State3, NewAccArgTypes, NewAccRet, HadExternal); handle_apply_or_call([], Args, _ArgTypes, Map, _Tree, State, - AccArgTypes, AccRet) -> - NewMap = enter_type_lists(Args, AccArgTypes, Map), - {State, NewMap, AccRet}. + AccArgTypes, AccRet, HadExternal) -> + case HadExternal of + false -> + NewMap = enter_type_lists(Args, AccArgTypes, Map), + {State, NewMap, AccRet}; + true -> + {had_external, State} + end. apply_fail_reason(FailedSig, FailedBif, FailedContract) -> if diff --git a/lib/dialyzer/src/dialyzer_dep.erl b/lib/dialyzer/src/dialyzer_dep.erl index e3ece144c9..572e60278d 100644 --- a/lib/dialyzer/src/dialyzer_dep.erl +++ b/lib/dialyzer/src/dialyzer_dep.erl @@ -124,8 +124,10 @@ traverse(Tree, Out, State, CurrentFun) -> TmpState = state__add_deps(Label, O1, State), state__add_deps(CurrentFun, O2,TmpState) end, - {BodyFuns, State2} = traverse(Body, Out, State1, - cerl_trees:get_label(Tree)), + Vars = cerl:fun_vars(Tree), + Out1 = bind_single(Vars, output(set__singleton(external)), Out), + {BodyFuns, State2} = + traverse(Body, Out1, State1, cerl_trees:get_label(Tree)), {output(set__singleton(Label)), state__add_esc(BodyFuns, State2)}; 'let' -> Vars = cerl:let_vars(Tree), diff --git a/lib/dialyzer/test/small_SUITE_data/results/funs_from_outside b/lib/dialyzer/test/small_SUITE_data/results/funs_from_outside new file mode 100644 index 0000000000..3e597ef1bc --- /dev/null +++ b/lib/dialyzer/test/small_SUITE_data/results/funs_from_outside @@ -0,0 +1,7 @@ + +funs_from_outside.erl:18: The pattern 'error' can never match the type {'ok','nothing' | 'something'} +funs_from_outside.erl:32: Function run2/2 has no local return +funs_from_outside.erl:35: Function testb/3 has no local return +funs_from_outside.erl:41: The pattern 'error' can never match the type {'ok','nothing' | 'something'} +funs_from_outside.erl:78: Function test2/1 has no local return +funs_from_outside.erl:83: The pattern 'error' can never match the type 'ok' diff --git a/lib/dialyzer/test/small_SUITE_data/src/funs_from_outside.erl b/lib/dialyzer/test/small_SUITE_data/src/funs_from_outside.erl new file mode 100644 index 0000000000..f4cbf31160 --- /dev/null +++ b/lib/dialyzer/test/small_SUITE_data/src/funs_from_outside.erl @@ -0,0 +1,83 @@ +-module(funs_from_outside). + +-export([run1/2, run2/2, run3/2]). +-export([test1/1, test2/1]). + +%%------------------------------------------------------------------------------ + +run1(X, Y) -> + testa(fun do_something/1, X, Y). + +testa(Fun, X, Y) -> + F = case even(X) of + true -> Fun; + false -> fun do_nothing/1 + end, + case F(Y) of + {ok, _} -> ok; + error -> error + end. + +do_nothing(_) -> {ok, nothing}. + +do_something(_) -> {ok, something}. + +even(X) -> + X rem 2 =:= 0. + +%%------------------------------------------------------------------------------ + +%% Duplicating code since we are monovariant... + +run2(X, Y) -> + testb(fun do_something/1, X, Y). + +testb(Fun, X, Y) -> + F = case even(X) of + true -> Fun; + false -> fun do_nothing/1 + end, + case F(Y) of + error -> error + end. + +%%------------------------------------------------------------------------------ + +%% Duplicating code since we are monovariant... + +run3(X, Y) -> + testc(fun do_something_2/1, X, Y). + +testc(Fun, X, Y) -> + F = case even(X) of + true -> Fun; + false -> fun do_nothing/1 + end, + case F(Y) of + {ok, _} -> ok; + %% This pattern can match. + error -> error + end. + +do_something_2(foo) -> {ok, something}; +do_something_2(_) -> error. + +%%------------------------------------------------------------------------------ + +test1(Fun) -> + F = case get(test1) of + test1_t -> Fun; + test1_f -> fun fok/0 + end, + error = F(). + +fok() -> ok. + +%%------------------------------------------------------------------------------ + +test2(Fun) -> + F = case get(test1) of + test1_t -> fun fok/0; + test1_f -> fun fok/0 + end, + error = F(). -- cgit v1.2.3