diff options
Diffstat (limited to 'lib/stdlib')
-rw-r--r-- | lib/stdlib/src/erl_expand_records.erl | 11 | ||||
-rw-r--r-- | lib/stdlib/src/erl_lint.erl | 165 | ||||
-rw-r--r-- | lib/stdlib/src/qlc_pt.erl | 46 | ||||
-rw-r--r-- | lib/stdlib/test/erl_lint_SUITE.erl | 10 | ||||
-rw-r--r-- | lib/stdlib/test/qlc_SUITE.erl | 21 |
5 files changed, 178 insertions, 75 deletions
diff --git a/lib/stdlib/src/erl_expand_records.erl b/lib/stdlib/src/erl_expand_records.erl index ab47f05320..2280464bff 100644 --- a/lib/stdlib/src/erl_expand_records.erl +++ b/lib/stdlib/src/erl_expand_records.erl @@ -507,9 +507,16 @@ lc_tq(Line, [{b_generate,Lg,P0,G0} | Qs0], St0) -> {P1,St2} = pattern(P0, St1), {Qs1,St3} = lc_tq(Line, Qs0, St2), {[{b_generate,Lg,P1,G1} | Qs1],St3}; -lc_tq(Line, [F0 | Qs0], St0) -> +lc_tq(Line, [F0 | Qs0], #exprec{calltype=Calltype}=St0) -> %% Allow record/2 and expand out as guard test. - case erl_lint:is_guard_test(F0) of + IsOverriden = fun(FA) -> + case Calltype of + #{FA := local} -> true; + #{FA := {imported,_}} -> true; + _ -> false + end + end, + case erl_lint:is_guard_test(F0, [], IsOverriden) of true -> {F1,St1} = guard_test(F0, St0), {Qs1,St2} = lc_tq(Line, Qs0, St1), diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl index e9332ce069..49b65069b7 100644 --- a/lib/stdlib/src/erl_lint.erl +++ b/lib/stdlib/src/erl_lint.erl @@ -27,7 +27,7 @@ -export([module/1,module/2,module/3,format_error/1]). -export([exprs/2,exprs_opt/3,used_vars/2]). % Used from erl_eval.erl. --export([is_pattern_expr/1,is_guard_test/1,is_guard_test/2]). +-export([is_pattern_expr/1,is_guard_test/1,is_guard_test/2,is_guard_test/3]). -export([is_guard_expr/1]). -export([bool_option/4,value_option/3,value_option/7]). @@ -238,7 +238,11 @@ format_error({removed_type, MNA, ReplacementMNA, Rel}) -> io_lib:format("the type ~s was removed in ~s; use ~s instead", [format_mna(MNA), Rel, format_mna(ReplacementMNA)]); format_error({obsolete_guard, {F, A}}) -> - io_lib:format("~p/~p obsolete", [F, A]); + io_lib:format("~p/~p obsolete (use is_~p/~p)", [F, A, F, A]); +format_error({obsolete_guard_overridden,Test}) -> + io_lib:format("obsolete ~s/1 (meaning is_~s/1) is illegal when " + "there is a local/imported function named is_~p/1 ", + [Test,Test,Test]); format_error({too_many_arguments,Arity}) -> io_lib:format("too many arguments (~w) - " "maximum allowed is ~w", [Arity,?MAX_ARGUMENTS]); @@ -1765,7 +1769,8 @@ bit_size({atom,_Line,all}, _Vt, St, _Check) -> {all,[],St}; bit_size(Size, Vt, St, Check) -> %% Try to safely evaluate Size if constant to get size, %% otherwise just treat it as an expression. - case is_gexpr(Size, St#lint.records) of + Info = is_guard_test2_info(St), + case is_gexpr(Size, Info) of true -> case erl_eval:partial_eval(Size) of {integer,_ILn,I} -> {I,[],St}; @@ -2000,77 +2005,104 @@ gexpr_list(Es, Vt, St) -> %% is_guard_test(Expression) -> boolean(). %% Test if a general expression is a guard test. +%% +%% Note: Only use this function in contexts where there can be +%% no definition of a local function that may override a guard BIF +%% (for example, in the shell). -spec is_guard_test(Expr) -> boolean() when Expr :: erl_parse:abstract_expr(). is_guard_test(E) -> - is_guard_test2(E, dict:new()). + is_guard_test2(E, {dict:new(),fun(_) -> false end}). %% is_guard_test(Expression, Forms) -> boolean(). is_guard_test(Expression, Forms) -> + is_guard_test(Expression, Forms, fun(_) -> false end). + + +%% is_guard_test(Expression, Forms, IsOverridden) -> boolean(). +%% Test if a general expression is a guard test. +%% +%% IsOverridden({Name,Arity}) should return 'true' if Name/Arity is +%% a local or imported function in the module. If the abstract code has +%% passed through erl_expand_records, any call without an explicit +%% module is to a local function, so IsOverridden can be defined as: +%% +%% fun(_) -> true end +%% +-spec is_guard_test(Expr, Forms, IsOverridden) -> boolean() when + Expr :: erl_parse:abstract_expr(), + Forms :: [erl_parse:abstract_form() | erl_parse:form_info()], + IsOverridden :: fun((fa()) -> boolean()). + +is_guard_test(Expression, Forms, IsOverridden) -> RecordAttributes = [A || A = {attribute, _, record, _D} <- Forms], St0 = foldl(fun(Attr0, St1) -> Attr = set_file(Attr0, "none"), attribute_state(Attr, St1) end, start(), RecordAttributes), - is_guard_test2(set_file(Expression, "nofile"), St0#lint.records). + is_guard_test2(set_file(Expression, "nofile"), + {St0#lint.records,IsOverridden}). %% is_guard_test2(Expression, RecordDefs :: dict:dict()) -> boolean(). -is_guard_test2({call,Line,{atom,Lr,record},[E,A]}, RDs) -> - is_gexpr({call,Line,{atom,Lr,is_record},[E,A]}, RDs); -is_guard_test2({call,_Line,{atom,_La,Test},As}=Call, RDs) -> - case erl_internal:type_test(Test, length(As)) of - true -> is_gexpr_list(As, RDs); - false -> is_gexpr(Call, RDs) - end; -is_guard_test2(G, RDs) -> +is_guard_test2({call,Line,{atom,Lr,record},[E,A]}, Info) -> + is_gexpr({call,Line,{atom,Lr,is_record},[E,A]}, Info); +is_guard_test2({call,_Line,{atom,_La,Test},As}=Call, {_,IsOverridden}=Info) -> + A = length(As), + not IsOverridden({Test,A}) andalso + case erl_internal:type_test(Test, A) of + true -> is_gexpr_list(As, Info); + false -> is_gexpr(Call, Info) + end; +is_guard_test2(G, Info) -> %%Everything else is a guard expression. - is_gexpr(G, RDs). + is_gexpr(G, Info). %% is_guard_expr(Expression) -> boolean(). %% Test if an expression is a guard expression. is_guard_expr(E) -> is_gexpr(E, []). -is_gexpr({var,_L,_V}, _RDs) -> true; -is_gexpr({char,_L,_C}, _RDs) -> true; -is_gexpr({integer,_L,_I}, _RDs) -> true; -is_gexpr({float,_L,_F}, _RDs) -> true; -is_gexpr({atom,_L,_A}, _RDs) -> true; -is_gexpr({string,_L,_S}, _RDs) -> true; -is_gexpr({nil,_L}, _RDs) -> true; -is_gexpr({cons,_L,H,T}, RDs) -> is_gexpr_list([H,T], RDs); -is_gexpr({tuple,_L,Es}, RDs) -> is_gexpr_list(Es, RDs); -%%is_gexpr({struct,_L,_Tag,Es}, RDs) -> -%% is_gexpr_list(Es, RDs); -is_gexpr({record_index,_L,_Name,Field}, RDs) -> - is_gexpr(Field, RDs); -is_gexpr({record_field,_L,Rec,_Name,Field}, RDs) -> - is_gexpr_list([Rec,Field], RDs); -is_gexpr({record,L,Name,Inits}, RDs) -> - is_gexpr_fields(Inits, L, Name, RDs); -is_gexpr({bin,_L,Fs}, RDs) -> +is_gexpr({var,_L,_V}, _Info) -> true; +is_gexpr({char,_L,_C}, _Info) -> true; +is_gexpr({integer,_L,_I}, _Info) -> true; +is_gexpr({float,_L,_F}, _Info) -> true; +is_gexpr({atom,_L,_A}, _Info) -> true; +is_gexpr({string,_L,_S}, _Info) -> true; +is_gexpr({nil,_L}, _Info) -> true; +is_gexpr({cons,_L,H,T}, Info) -> is_gexpr_list([H,T], Info); +is_gexpr({tuple,_L,Es}, Info) -> is_gexpr_list(Es, Info); +%%is_gexpr({struct,_L,_Tag,Es}, Info) -> +%% is_gexpr_list(Es, Info); +is_gexpr({record_index,_L,_Name,Field}, Info) -> + is_gexpr(Field, Info); +is_gexpr({record_field,_L,Rec,_Name,Field}, Info) -> + is_gexpr_list([Rec,Field], Info); +is_gexpr({record,L,Name,Inits}, Info) -> + is_gexpr_fields(Inits, L, Name, Info); +is_gexpr({bin,_L,Fs}, Info) -> all(fun ({bin_element,_Line,E,Sz,_Ts}) -> - is_gexpr(E, RDs) and (Sz =:= default orelse is_gexpr(Sz, RDs)) + is_gexpr(E, Info) and (Sz =:= default orelse is_gexpr(Sz, Info)) end, Fs); -is_gexpr({call,_L,{atom,_Lf,F},As}, RDs) -> +is_gexpr({call,_L,{atom,_Lf,F},As}, {_,IsOverridden}=Info) -> A = length(As), - erl_internal:guard_bif(F, A) andalso is_gexpr_list(As, RDs); -is_gexpr({call,_L,{remote,_Lr,{atom,_Lm,erlang},{atom,_Lf,F}},As}, RDs) -> + not IsOverridden({F,A}) andalso erl_internal:guard_bif(F, A) + andalso is_gexpr_list(As, Info); +is_gexpr({call,_L,{remote,_Lr,{atom,_Lm,erlang},{atom,_Lf,F}},As}, Info) -> A = length(As), (erl_internal:guard_bif(F, A) orelse is_gexpr_op(F, A)) - andalso is_gexpr_list(As, RDs); -is_gexpr({call,L,{tuple,Lt,[{atom,Lm,erlang},{atom,Lf,F}]},As}, RDs) -> - is_gexpr({call,L,{remote,Lt,{atom,Lm,erlang},{atom,Lf,F}},As}, RDs); -is_gexpr({op,_L,Op,A}, RDs) -> - is_gexpr_op(Op, 1) andalso is_gexpr(A, RDs); -is_gexpr({op,_L,'andalso',A1,A2}, RDs) -> - is_gexpr_list([A1,A2], RDs); -is_gexpr({op,_L,'orelse',A1,A2}, RDs) -> - is_gexpr_list([A1,A2], RDs); -is_gexpr({op,_L,Op,A1,A2}, RDs) -> - is_gexpr_op(Op, 2) andalso is_gexpr_list([A1,A2], RDs); -is_gexpr(_Other, _RDs) -> false. + andalso is_gexpr_list(As, Info); +is_gexpr({call,L,{tuple,Lt,[{atom,Lm,erlang},{atom,Lf,F}]},As}, Info) -> + is_gexpr({call,L,{remote,Lt,{atom,Lm,erlang},{atom,Lf,F}},As}, Info); +is_gexpr({op,_L,Op,A}, Info) -> + is_gexpr_op(Op, 1) andalso is_gexpr(A, Info); +is_gexpr({op,_L,'andalso',A1,A2}, Info) -> + is_gexpr_list([A1,A2], Info); +is_gexpr({op,_L,'orelse',A1,A2}, Info) -> + is_gexpr_list([A1,A2], Info); +is_gexpr({op,_L,Op,A1,A2}, Info) -> + is_gexpr_op(Op, 2) andalso is_gexpr_list([A1,A2], Info); +is_gexpr(_Other, _Info) -> false. is_gexpr_op(Op, A) -> try erl_internal:op_type(Op, A) of @@ -2082,14 +2114,14 @@ is_gexpr_op(Op, A) -> catch _:_ -> false end. -is_gexpr_list(Es, RDs) -> all(fun (E) -> is_gexpr(E, RDs) end, Es). +is_gexpr_list(Es, Info) -> all(fun (E) -> is_gexpr(E, Info) end, Es). -is_gexpr_fields(Fs, L, Name, RDs) -> +is_gexpr_fields(Fs, L, Name, {RDs,_}=Info) -> IFs = case dict:find(Name, RDs) of {ok,{_Line,Fields}} -> Fs ++ init_fields(Fs, L, Fields); error -> Fs end, - all(fun ({record_field,_Lf,_Name,V}) -> is_gexpr(V, RDs); + all(fun ({record_field,_Lf,_Name,V}) -> is_gexpr(V, Info); (_Other) -> false end, IFs). %% exprs(Sequence, VarTable, State) -> @@ -3193,7 +3225,8 @@ lc_quals([{b_generate,_Line,P,E} | Qs], Vt0, Uvt0, St0) -> {Vt,Uvt,St} = handle_generator(P,E,Vt0,Uvt0,St1), lc_quals(Qs, Vt, Uvt, St); lc_quals([F|Qs], Vt, Uvt, St0) -> - {Fvt,St1} = case is_guard_test2(F, St0#lint.records) of + Info = is_guard_test2_info(St0), + {Fvt,St1} = case is_guard_test2(F, Info) of true -> guard_test(F, Vt, St0); false -> expr(F, Vt, St0) end, @@ -3201,6 +3234,12 @@ lc_quals([F|Qs], Vt, Uvt, St0) -> lc_quals([], Vt, Uvt, St) -> {Vt, Uvt, St}. +is_guard_test2_info(#lint{records=RDs,locals=Locals,imports=Imports}) -> + {RDs,fun(FA) -> + is_local_function(Locals, FA) orelse + is_imported_function(Imports, FA) + end}. + handle_generator(P,E,Vt,Uvt,St0) -> {Evt,St1} = expr(E, Vt, St0), %% Forget variables local to E immediately. @@ -3618,16 +3657,26 @@ obsolete_guard({call,Line,{atom,Lr,F},As}, St0) -> false -> deprecated_function(Line, erlang, F, As, St0); true -> - case is_warn_enabled(obsolete_guard, St0) of - true -> - add_warning(Lr,{obsolete_guard, {F, Arity}}, St0); - false -> - St0 - end + St = case is_warn_enabled(obsolete_guard, St0) of + true -> + add_warning(Lr, {obsolete_guard, {F, Arity}}, St0); + false -> + St0 + end, + test_overriden_by_local(Lr, F, Arity, St) end; obsolete_guard(_G, St) -> St. +test_overriden_by_local(Line, OldTest, Arity, St) -> + ModernTest = list_to_atom("is_"++atom_to_list(OldTest)), + case is_local_function(St#lint.locals, {ModernTest, Arity}) of + true -> + add_error(Line, {obsolete_guard_overridden,OldTest}, St); + false -> + St + end. + %% keyword_warning(Line, Atom, State) -> State. %% Add warning for atoms that will be reserved keywords in the future. %% (Currently, no such keywords to warn for.) diff --git a/lib/stdlib/src/qlc_pt.erl b/lib/stdlib/src/qlc_pt.erl index 869b003668..28221ea75f 100644 --- a/lib/stdlib/src/qlc_pt.erl +++ b/lib/stdlib/src/qlc_pt.erl @@ -41,6 +41,7 @@ }). -record(state, {imp, + overridden, maxargs, records, xwarnings = [], @@ -184,7 +185,9 @@ initiate(Forms0, Imported) -> exclude_integers_from_unique_line_numbers(Forms0, NodeInfo), ?DEBUG("node info0 ~p~n", [lists:sort(ets:tab2list(NodeInfo))]), + IsOverridden = set_up_overridden(Forms0), State0 = #state{imp = Imported, + overridden = IsOverridden, maxargs = ?EVAL_MAX_NUM_OF_ARGS, records = record_attributes(Forms0), node_info = NodeInfo}, @@ -1519,36 +1522,35 @@ filter_info(FilterData, AllIVs, Dependencies, State) -> %% to be placed after further generators (the docs states otherwise, but %% this seems to be common practice). filter_list(FilterData, Dependencies, State) -> - RDs = State#state.records, - sel_gf(FilterData, 1, Dependencies, RDs, [], []). + sel_gf(FilterData, 1, Dependencies, State, [], []). sel_gf([], _N, _Deps, _RDs, _Gens, _Gens1) -> []; -sel_gf([{#qid{no = N}=Id,{fil,F}}=Fil | FData], N, Deps, RDs, Gens, Gens1) -> - case erl_lint:is_guard_test(F, RDs) of +sel_gf([{#qid{no = N}=Id,{fil,F}}=Fil | FData], N, Deps, State, Gens, Gens1) -> + case is_guard_test(F, State) of true -> {Id,GIds} = lists:keyfind(Id, 1, Deps), case length(GIds) =< 1 of true -> case generators_in_scope(GIds, Gens1) of true -> - [Fil|sel_gf(FData, N+1, Deps, RDs, Gens, Gens1)]; + [Fil|sel_gf(FData, N+1, Deps, State, Gens, Gens1)]; false -> - sel_gf(FData, N + 1, Deps, RDs, [], []) + sel_gf(FData, N + 1, Deps, State, [], []) end; false -> case generators_in_scope(GIds, Gens) of true -> - [Fil | sel_gf(FData, N + 1, Deps, RDs, Gens, [])]; + [Fil | sel_gf(FData, N + 1, Deps, State, Gens, [])]; false -> - sel_gf(FData, N + 1, Deps, RDs, [], []) + sel_gf(FData, N + 1, Deps, State, [], []) end end; false -> - sel_gf(FData, N + 1, Deps, RDs, [], []) + sel_gf(FData, N + 1, Deps, State, [], []) end; -sel_gf(FData, N, Deps, RDs, Gens, Gens1) -> - sel_gf(FData, N + 1, Deps, RDs, [N | Gens], [N | Gens1]). +sel_gf(FData, N, Deps, State, Gens, Gens1) -> + sel_gf(FData, N + 1, Deps, State, [N | Gens], [N | Gens1]). generators_in_scope(GenIds, GenNumbers) -> lists:all(fun(#qid{no=N}) -> lists:member(N, GenNumbers) end, GenIds). @@ -2483,7 +2485,7 @@ filter(E, L, QIVs, S, RL, Fun, Go, GoI, IVs, State) -> %% This is the "guard semantics" used in ordinary list %% comprehension: if a filter looks like a guard test, it returns %% 'false' rather than fails. - Body = case erl_lint:is_guard_test(E, State#state.records) of + Body = case is_guard_test(E, State) of true -> CT = {clause,L,[],[[E]],[{call,L,?V(Fun),NAsT}]}, CF = {clause,L,[],[[?A(true)]],[{call,L,?V(Fun),NAsF}]}, @@ -2887,6 +2889,26 @@ family_list(L) -> family(L) -> sofs:relation_to_family(sofs:relation(L)). +is_guard_test(E, #state{records = RDs, overridden = IsOverridden}) -> + erl_lint:is_guard_test(E, RDs, IsOverridden). + +%% In code that has been run through erl_expand_records, a guard +%% test will never contain calls without an explicit module +%% prefix. Unfortunately, this module runs *some* of the code +%% through erl_expand_records, but not all of it. +%% +%% Therefore, we must set up our own list of local and imported functions +%% that will override a BIF with the same name. + +set_up_overridden(Forms) -> + Locals = [{Name,Arity} || {function,_,Name,Arity,_} <- Forms], + Imports0 = [Fs || {attribute,_,import,Fs} <- Forms], + Imports1 = lists:flatten(Imports0), + Imports2 = [Fs || {_,Fs} <- Imports1], + Imports = lists:flatten(Imports2), + Overridden = gb_sets:from_list(Imports ++ Locals), + fun(FA) -> gb_sets:is_element(FA, Overridden) end. + -ifdef(debug). display_forms(Forms) -> io:format("Forms ***~n"), diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl index d916eb3eef..4ee3950882 100644 --- a/lib/stdlib/test/erl_lint_SUITE.erl +++ b/lib/stdlib/test/erl_lint_SUITE.erl @@ -1554,7 +1554,15 @@ guard(Config) when is_list(Config) -> [], {errors,[{1,erl_lint,illegal_guard_expr}, {2,erl_lint,illegal_guard_expr}], - []}} + []}}, + {guard10, + <<"is_port(_) -> false. + t(P) when port(P) -> ok. + ">>, + [], + {error, + [{2,erl_lint,{obsolete_guard_overridden,port}}], + [{2,erl_lint,{obsolete_guard,{port,1}}}]}} ], [] = run(Config, Ts1), ok. diff --git a/lib/stdlib/test/qlc_SUITE.erl b/lib/stdlib/test/qlc_SUITE.erl index 2bd940020c..8c7d5a5fcf 100644 --- a/lib/stdlib/test/qlc_SUITE.erl +++ b/lib/stdlib/test/qlc_SUITE.erl @@ -58,7 +58,7 @@ -export([ badarg/1, nested_qlc/1, unused_var/1, lc/1, fun_clauses/1, filter_var/1, single/1, exported_var/1, generator_vars/1, - nomatch/1, errors/1, pattern/1, + nomatch/1, errors/1, pattern/1, overridden_bif/1, eval/1, cursor/1, fold/1, eval_unique/1, eval_cache/1, append/1, evaluator/1, string_to_handle/1, table/1, process_dies/1, @@ -126,7 +126,7 @@ groups() -> [{parse_transform, [], [badarg, nested_qlc, unused_var, lc, fun_clauses, filter_var, single, exported_var, generator_vars, - nomatch, errors, pattern]}, + nomatch, errors, pattern, overridden_bif]}, {evaluation, [], [eval, cursor, fold, eval_unique, eval_cache, append, evaluator, string_to_handle, table, process_dies, sort, @@ -468,6 +468,23 @@ pattern(Config) when is_list(Config) -> -record(k, {t,v}).\n">>, Ts), ok. +%% Override a guard BIF with an imported or local function. +overridden_bif(Config) -> + Ts = [ + <<"[2] = qlc:e(qlc:q([P || P <- [1,2,3], port(P)])), + [10] = qlc:e(qlc:q([P || P <- [0,9,10,11,12], + (is_reference(P) andalso P > 5)])), + Empty = gb_sets:empty(), Single = gb_sets:singleton(42), + GbSets = [Empty,Single], + [Single] = qlc:e(qlc:q([S || S <- GbSets, size(S) =/= 0])) + ">> + ], + run(Config, "-import(gb_sets, [size/1]). + -compile({no_auto_import, [size/1, is_reference/1]}). + port(N) -> N rem 2 =:= 0. + is_reference(N) -> N rem 10 =:= 0.\n", Ts), + ok. + %% eval/2 eval(Config) when is_list(Config) -> |