Fix overridden BIFs

The filters in a list comprehension can be guard expressions or
an ordinary expressions.

If a guard expression is used as a filter, an exception will basically
mean the same as 'false':

  t() ->
    L = [{some_tag,42},an_atom],
    [X || X <- L, element(1, X) =:= some_tag]
    %% Returns [{some_tag,42}]

On the other hand, if an ordinary expression is used as a filter, there
will be an exception:

  my_element(N, T) -> element(N, T).

  t() ->
    L = [{some_tag,42},an_atom],
    [X || X <- L, my_element(1, X) =:= some_tag]
    %% Causes a 'badarg' exception when element(1, an_atom) is evaluated

It has been allowed for several releases to override a BIF with
a local function. Thus, if we define a function called element/2,
it will be called instead of the BIF element/2 within the module.
We must use the "erlang:" prefix to call the BIF.

Therefore, the following code is expected to work the same way as in
our second example above:

-compile({no_auto_import,[element/2]}).

element(N, T) ->
    erlang:element(N, T).

t() ->
    L = [{some_tag,42},an_atom],
    [X || X <- L, element(1, X) =:= some_tag].
    %% Causes a 'badarg' exception when element(1, an_atom) is evaluated

But the compiler refuses to compile the code with the following
diagnostic:

  call to local/imported function element/2 is illegal in guard

4 files changed, 148 insertions, 67 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 5ec9b9053a..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]).
 
@@ -1769,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};
@@ -2004,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
@@ -2086,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) ->
@@ -3197,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,
@@ -3205,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.
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/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) ->