diff --git a/lib/elixir/lib/kernel/lexical_tracker.ex b/lib/elixir/lib/kernel/lexical_tracker.ex index b27cf19351..0b7ed7c050 100644 --- a/lib/elixir/lib/kernel/lexical_tracker.ex +++ b/lib/elixir/lib/kernel/lexical_tracker.ex @@ -30,8 +30,8 @@ defmodule Kernel.LexicalTracker do end @doc false - def add_require(pid, module) when is_atom(module) do - :gen_server.cast(pid, {:add_require, module}) + def add_export(pid, module) when is_atom(module) do + :gen_server.cast(pid, {:add_export, module}) end @doc false @@ -88,23 +88,24 @@ defmodule Kernel.LexicalTracker do @doc false def collect_unused_imports(pid) do - unused(pid, :import) + unused(pid, :unused_imports) end @doc false def collect_unused_aliases(pid) do - unused(pid, :alias) + unused(pid, :unused_aliases) end defp unused(pid, tag) do - :gen_server.call(pid, {:unused, tag}, @timeout) + :gen_server.call(pid, tag, @timeout) end # Callbacks def init(:ok) do state = %{ - directives: %{}, + aliases: %{}, + imports: %{}, references: %{}, exports: %{}, cache: %{}, @@ -116,13 +117,12 @@ defmodule Kernel.LexicalTracker do end @doc false - def handle_call({:unused, tag}, _from, state) do - directives = - for {{^tag, module_or_mfa}, marker} <- state.directives, is_integer(marker) do - {module_or_mfa, marker} - end + def handle_call(:unused_aliases, _from, state) do + {:reply, Enum.sort(state.aliases), state} + end - {:reply, Enum.sort(directives), state} + def handle_call(:unused_imports, _from, state) do + {:reply, Enum.sort(state.imports), state} end def handle_call(:references, _from, state) do @@ -148,12 +148,26 @@ defmodule Kernel.LexicalTracker do end def handle_cast({:import_dispatch, module, {function, arity}, mode}, state) do - state = add_import_dispatch(state, module, function, arity, mode) - {:noreply, state} + %{imports: imports, references: references} = state + + imports = + case imports do + %{^module => modules_and_fas} -> + modules_and_fas + |> Map.delete(module) + |> Map.delete({function, arity}) + |> then(&Map.put(imports, module, &1)) + + %{} -> + imports + end + + references = add_reference(references, module, mode) + {:noreply, %{state | imports: imports, references: references}} end - def handle_cast({:alias_dispatch, module}, state) do - {:noreply, %{state | directives: add_dispatch(state.directives, module, :alias)}} + def handle_cast({:alias_dispatch, module}, %{aliases: aliases} = state) do + {:noreply, %{state | aliases: Map.delete(aliases, module)}} end def handle_cast({:set_file, file}, state) do @@ -168,28 +182,25 @@ defmodule Kernel.LexicalTracker do {:noreply, update_in(state.compile_env, &:ordsets.add_element({app, path, return}, &1))} end - def handle_cast({:add_require, module}, state) do + def handle_cast({:add_export, module}, state) do {:noreply, put_in(state.exports[module], true)} end def handle_cast({:add_import, module, fas, line, warn}, state) do - to_remove = for {{:import, {^module, _, _}} = key, _} <- state.directives, do: key - - directives = - state.directives - |> Map.drop(to_remove) - |> add_directive(module, line, warn, :import) - - directives = - Enum.reduce(fas, directives, fn {function, arity}, directives -> - add_directive(directives, {module, function, arity}, line, warn, :import) - end) - - {:noreply, %{state | directives: directives}} + if warn do + imports = for module_or_fa <- [module | fas], do: {module_or_fa, line}, into: %{} + {:noreply, put_in(state.imports[module], imports)} + else + {:noreply, state} + end end def handle_cast({:add_alias, module, line, warn}, state) do - {:noreply, %{state | directives: add_directive(state.directives, module, line, warn, :alias)}} + if warn do + {:noreply, put_in(state.aliases[module], line)} + else + {:noreply, state} + end end @doc false @@ -221,31 +232,9 @@ defmodule Kernel.LexicalTracker do do: Map.put(references, module, :compile) defp add_reference(references, module, :runtime) when is_atom(module) do - case Map.fetch(references, module) do - {:ok, _} -> references - :error -> Map.put(references, module, :runtime) + case references do + %{^module => _} -> references + %{} -> Map.put(references, module, :runtime) end end - - defp add_import_dispatch(state, module, function, arity, mode) do - directives = - state.directives - |> add_dispatch(module, :import) - |> add_dispatch({module, function, arity}, :import) - - references = add_reference(state.references, module, mode) - %{state | directives: directives, references: references} - end - - # In the map we keep imports and aliases. - # If the value is a line, it was imported/aliased and has a pending warning - # If the value is true, it was imported/aliased and used - defp add_directive(directives, module_or_mfa, line, warn, tag) do - marker = if warn, do: line, else: true - Map.put(directives, {tag, module_or_mfa}, marker) - end - - defp add_dispatch(directives, module_or_mfa, tag) do - Map.put(directives, {tag, module_or_mfa}, true) - end end diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl index d36f764b78..d23377047e 100644 --- a/lib/elixir/src/elixir_def.erl +++ b/lib/elixir/src/elixir_def.erl @@ -1,6 +1,6 @@ % Holds the logic responsible for function definitions (def(p) and defmacro(p)). -module(elixir_def). --export([setup/1, reset_last/1, local_for/4, +-export([setup/1, reset_last/1, local_for/5, external_for/5, take_definition/2, store_definition/5, store_definition/9, fetch_definitions/2, format_error/1]). -include("elixir.hrl"). @@ -16,18 +16,32 @@ reset_last({DataSet, _DataBag}) -> reset_last(Module) when is_atom(Module) -> reset_last(elixir_module:data_tables(Module)). -local_for(Module, Name, Arity, Kinds) -> +%% Finds the local definition for the current module. +local_for(Meta, Name, Arity, Kinds, E) -> + External = fun({Mod, Fun}, Args) -> + invoke_external([{from_local_macro, {Name, Arity}} | Meta], Mod, Fun, Args, E) + end, + fun_for(Meta, ?key(E, module), Name, Arity, Kinds, {value, External}). + +%% Finds the local definition for an external module. +external_for(Meta, Module, Name, Arity, Kinds) -> + fun_for(Meta, Module, Name, Arity, Kinds, none). + +fun_for(Meta, Module, Name, Arity, Kinds, External) -> Tuple = {Name, Arity}, try {Set, Bag} = elixir_module:data_tables(Module), {ets:lookup(Set, {def, Tuple}), ets:lookup(Bag, {clauses, Tuple})} of - {[{_, Kind, Meta, _, _, _}], Clauses} -> + {[{_, Kind, LocalMeta, _, _, _}], ClausesPairs} -> case (Kinds == all) orelse (lists:member(Kind, Kinds)) of - true -> elixir_erl:definition_to_anonymous(Module, Kind, Meta, - [Clause || {_, Clause} <- Clauses]); - false -> false + true -> + Local = {value, fun(Fun, Args) -> invoke_local(Meta, Module, Fun, Args, External) end}, + Clauses = [Clause || {_, Clause} <- ClausesPairs], + elixir_erl:definition_to_anonymous(Kind, LocalMeta, Clauses, Local, External); + false -> + false end; {[], _} -> false @@ -35,6 +49,21 @@ local_for(Module, Name, Arity, Kinds) -> _:_ -> false end. +invoke_local(Meta, Module, ErlName, Args, External) -> + {Name, Arity} = elixir_utils:erl_fa_to_elixir_fa(ErlName, length(Args)), + + case fun_for(Meta, Module, Name, Arity, all, External) of + false -> + {current_stacktrace, [_ | T]} = erlang:process_info(self(), current_stacktrace), + erlang:raise(error, undef, [{Module, Name, Arity, []} | T]); + Fun -> + apply(Fun, Args) + end. + +invoke_external(Meta, Mod, Name, Args, E) -> + is_map(E) andalso elixir_env:trace({require, Meta, Mod, []}, E), + apply(Mod, Name, Args). + %% Take a definition out of the table take_definition(Module, {Name, Arity} = Tuple) -> diff --git a/lib/elixir/src/elixir_dispatch.erl b/lib/elixir/src/elixir_dispatch.erl index a51b1eb614..65325f0a25 100644 --- a/lib/elixir/src/elixir_dispatch.erl +++ b/lib/elixir/src/elixir_dispatch.erl @@ -71,7 +71,7 @@ import_function(Meta, Name, Arity, E) -> Function = ?key(E, function), case (Function /= nil) andalso (Function /= Tuple) andalso - elixir_def:local_for(?key(E, module), Name, Arity, [defmacro, defmacrop]) of + elixir_def:local_for(Meta, Name, Arity, [defmacro, defmacrop], E) of false -> elixir_env:trace({local_function, Meta, Name, Arity}, E), elixir_locals:record_local(Tuple, ?key(E, module), ?key(E, function), Meta, false), @@ -142,7 +142,7 @@ expand_import(Meta, {Name, Arity} = Tuple, Args, S, E, Extra, External) -> _ -> AllowLocals = External orelse ((Function /= nil) andalso (Function /= Tuple)), Local = AllowLocals andalso - elixir_def:local_for(Module, Name, Arity, [defmacro, defmacrop]), + elixir_def:local_for(Meta, Name, Arity, [defmacro, defmacrop], E), case Dispatch of %% There is a local and an import. This is a conflict unless diff --git a/lib/elixir/src/elixir_erl.erl b/lib/elixir/src/elixir_erl.erl index 0f8757b0cb..55dfcb3008 100644 --- a/lib/elixir/src/elixir_erl.erl +++ b/lib/elixir/src/elixir_erl.erl @@ -1,6 +1,7 @@ %% Compiler backend to Erlang. + -module(elixir_erl). --export([elixir_to_erl/1, elixir_to_erl/2, definition_to_anonymous/4, compile/1, consolidate/3, +-export([elixir_to_erl/1, elixir_to_erl/2, definition_to_anonymous/5, compile/1, consolidate/3, get_ann/1, debug_info/4, scope/2, format_error/1]). -include("elixir.hrl"). -define(typespecs, 'Elixir.Kernel.Typespec'). @@ -47,24 +48,12 @@ get_ann([], Gen, Line) -> erl_anno:set_generated(Gen, erl_anno:new(Line)). %% Converts an Elixir definition to an anonymous function. -definition_to_anonymous(Module, Kind, Meta, Clauses) -> +definition_to_anonymous(Kind, Meta, Clauses, LocalHandler, ExternalHandler) -> ErlClauses = [translate_clause(Kind, 0, Clause, true) || Clause <- Clauses], Fun = {'fun', ?ann(Meta), {clauses, ErlClauses}}, - LocalHandler = fun(LocalName, LocalArgs) -> invoke_local(Module, LocalName, LocalArgs) end, - {value, Result, _Binding} = erl_eval:expr(Fun, [], {value, LocalHandler}), + {value, Result, _Binding} = erl_eval:expr(Fun, [], LocalHandler, ExternalHandler), Result. -invoke_local(Module, ErlName, Args) -> - {Name, Arity} = elixir_utils:erl_fa_to_elixir_fa(ErlName, length(Args)), - - case elixir_def:local_for(Module, Name, Arity, all) of - false -> - {current_stacktrace, [_ | T]} = erlang:process_info(self(), current_stacktrace), - erlang:raise(error, undef, [{Module, Name, Arity, []} | T]); - Fun -> - apply(Fun, Args) - end. - %% Converts Elixir quoted literals to Erlang AST. elixir_to_erl(Tree) -> elixir_to_erl(Tree, erl_anno:new(0)). diff --git a/lib/elixir/src/elixir_lexical.erl b/lib/elixir/src/elixir_lexical.erl index c655333728..2f6de52de4 100644 --- a/lib/elixir/src/elixir_lexical.erl +++ b/lib/elixir/src/elixir_lexical.erl @@ -47,10 +47,10 @@ trace({alias_expansion, _Meta, Lookup, _Result}, #{lexical_tracker := Pid}) -> ?tracker:alias_dispatch(Pid, Lookup), ok; trace({require, _Meta, Module, _Opts}, #{lexical_tracker := Pid}) -> - ?tracker:add_require(Pid, Module), + ?tracker:add_export(Pid, Module), ok; trace({struct_expansion, _Meta, Module, _Keys}, #{lexical_tracker := Pid}) -> - ?tracker:add_require(Pid, Module), + ?tracker:add_export(Pid, Module), ok; trace({alias_reference, _Meta, Module}, #{lexical_tracker := Pid} = E) -> ?tracker:remote_dispatch(Pid, Module, mode(E)), @@ -98,21 +98,20 @@ with_file(File, #{lexical_tracker := Pid} = E, Callback) -> %% ERROR HANDLING warn_unused_imports(Pid, E) -> - {ModuleImports, MFAImports} = - lists:partition(fun({M, _}) -> is_atom(M) end, ?tracker:collect_unused_imports(Pid)), - - Modules = [M || {M, _L} <- ModuleImports], - MFAImportsFiltered = [T || {{M, _, _}, _} = T <- MFAImports, not lists:member(M, Modules)], - - [begin - elixir_errors:form_warn([{line, L}], ?key(E, file), ?MODULE, {unused_import, M}) - end || {M, L} <- ModuleImports ++ MFAImportsFiltered], + [elixir_errors:form_warn([{line, Line}], ?key(E, file), ?MODULE, {unused_import, ModOrMFA}) + || {Module, Imports} <- ?tracker:collect_unused_imports(Pid), + {ModOrMFA, Line} <- unused_imports_for_module(Module, Imports)], ok. +unused_imports_for_module(Module, Imports) -> + case Imports of + #{Module := Line} -> [{Module, Line}]; + #{} -> [{{Module, Fun, Arity}, Line} || {{Fun, Arity}, Line} <- maps:to_list(Imports)] + end. + warn_unused_aliases(Pid, E) -> - [begin - elixir_errors:form_warn([{line, L}], ?key(E, file), ?MODULE, {unused_alias, M}) - end || {M, L} <- ?tracker:collect_unused_aliases(Pid)], + [elixir_errors:form_warn([{line, Line}], ?key(E, file), ?MODULE, {unused_alias, Module}) + || {Module, Line} <- ?tracker:collect_unused_aliases(Pid)], ok. format_error({unused_alias, Module}) -> diff --git a/lib/elixir/src/elixir_map.erl b/lib/elixir/src/elixir_map.erl index 4a84696278..500a61d0f3 100644 --- a/lib/elixir/src/elixir_map.erl +++ b/lib/elixir/src/elixir_map.erl @@ -141,13 +141,14 @@ wrapped_load_struct(Meta, Name, Args, Keys, E) -> InContext = lists:member(Name, [?key(E, module) | ?key(E, context_modules)]), Arity = length(Args), - Local = InContext orelse (not(ensure_loaded(Name)) andalso wait_for_struct(Name)), + External = InContext orelse (not(ensure_loaded(Name)) andalso wait_for_struct(Name)), try - case Local andalso elixir_def:local_for(Name, '__struct__', Arity, [def]) of + case External andalso elixir_def:external_for(Meta, Name, '__struct__', Arity, [def]) of false -> apply(Name, '__struct__', Args); - LocalFun -> + + ExternalFun -> %% There is an inherent race condition when using local_for. %% By the time we got to execute the function, the ETS table %% with temporary definitions for the given module may no longer @@ -157,7 +158,7 @@ wrapped_load_struct(Meta, Name, Args, Keys, E) -> %% the table has not been deleted (unless compilation of that %% module failed which should then cause this call to fail too). try - apply(LocalFun, Args) + apply(ExternalFun, Args) catch error:undef -> apply(Name, '__struct__', Args) end diff --git a/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs b/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs index c69a841a6e..61deac2f2c 100644 --- a/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs +++ b/lib/elixir/test/elixir/kernel/lexical_tracker_test.exs @@ -22,7 +22,7 @@ defmodule Kernel.LexicalTrackerTest do end test "can add requires", config do - D.add_require(config[:pid], URI) + D.add_export(config[:pid], URI) assert D.references(config[:pid]) == {[], [URI], [], []} D.remote_dispatch(config[:pid], URI, :runtime) @@ -33,7 +33,7 @@ defmodule Kernel.LexicalTrackerTest do end test "can add module imports", config do - D.add_require(config[:pid], String) + D.add_export(config[:pid], String) D.add_import(config[:pid], String, [], 1, true) D.import_dispatch(config[:pid], String, {:upcase, 1}, :runtime) @@ -44,7 +44,7 @@ defmodule Kernel.LexicalTrackerTest do end test "can add module with {function, arity} imports", config do - D.add_require(config[:pid], String) + D.add_export(config[:pid], String) D.add_import(config[:pid], String, [upcase: 1], 1, true) D.import_dispatch(config[:pid], String, {:upcase, 1}, :compile) @@ -59,32 +59,32 @@ defmodule Kernel.LexicalTrackerTest do test "unused module imports", config do D.add_import(config[:pid], String, [], 1, true) - assert D.collect_unused_imports(config[:pid]) == [{String, 1}] + assert D.collect_unused_imports(config[:pid]) == [{String, %{String => 1}}] end test "used module imports are not unused", config do D.add_import(config[:pid], String, [], 1, true) D.import_dispatch(config[:pid], String, {:upcase, 1}, :compile) - assert D.collect_unused_imports(config[:pid]) == [] + assert D.collect_unused_imports(config[:pid]) == [{String, %{}}] end test "unused {module, function, arity} imports", config do D.add_import(config[:pid], String, [upcase: 1], 1, true) - assert D.collect_unused_imports(config[:pid]) == [{String, 1}, {{String, :upcase, 1}, 1}] + assert D.collect_unused_imports(config[:pid]) == [{String, %{String => 1, {:upcase, 1} => 1}}] end test "used {module, function, arity} imports are not unused", config do D.add_import(config[:pid], String, [upcase: 1], 1, true) D.add_import(config[:pid], String, [downcase: 1], 1, true) D.import_dispatch(config[:pid], String, {:upcase, 1}, :compile) - assert D.collect_unused_imports(config[:pid]) == [{{String, :downcase, 1}, 1}] + assert D.collect_unused_imports(config[:pid]) == [{String, %{{:downcase, 1} => 1}}] end test "overwriting {module, function, arity} import with module import", config do D.add_import(config[:pid], String, [upcase: 1], 1, true) D.add_import(config[:pid], String, [], 1, true) D.import_dispatch(config[:pid], String, {:downcase, 1}, :compile) - assert D.collect_unused_imports(config[:pid]) == [] + assert D.collect_unused_imports(config[:pid]) == [{String, %{}}] end test "imports with no warn are not unused", config do @@ -282,5 +282,27 @@ defmodule Kernel.LexicalTrackerTest do refute Kernel.LexicalTrackerTest.Defmodule in exports refute Kernel.LexicalTrackerTest.Defmodule in runtime end + + # TODO: This should not be an export dependency but all requires are exports at the moment + test "defmacro adds an export dependency" do + {{compile, exports, runtime, _}, _binding} = + Code.eval_string(""" + defmodule Kernel.LexicalTrackerTest.Defmacro do + defmacro uri() do + Macro.escape(URI.parse("/hello")) + end + + def fun() do + uri() + end + + Kernel.LexicalTracker.references(__ENV__.lexical_tracker) + end |> elem(3) + """) + + refute URI in compile + assert URI in exports + assert URI in runtime + end end end diff --git a/lib/elixir/test/elixir/kernel/quote_test.exs b/lib/elixir/test/elixir/kernel/quote_test.exs index c81aec22ed..c0b81d55ba 100644 --- a/lib/elixir/test/elixir/kernel/quote_test.exs +++ b/lib/elixir/test/elixir/kernel/quote_test.exs @@ -601,7 +601,7 @@ defmodule Kernel.QuoteTest.ImportsHygieneTest do test "lazy expand imports no conflicts" do import Kernel, except: [length: 1] - import String, only: [length: 1] + import String, only: [length: 1], warn: false assert get_list_length() == 5 assert get_list_length_with_partial() == 5