From 792e60421f0aa17fec60a37fd6e5e73789fe8560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 10:58:23 +0100 Subject: [PATCH 01/22] Prepare for local inference --- lib/elixir/lib/module/parallel_checker.ex | 119 +++++++++++++--------- lib/elixir/lib/module/types.ex | 1 - lib/elixir/src/elixir_module.erl | 30 +++--- 3 files changed, 85 insertions(+), 65 deletions(-) diff --git a/lib/elixir/lib/module/parallel_checker.ex b/lib/elixir/lib/module/parallel_checker.ex index ef24485d02c..a64ab47123a 100644 --- a/lib/elixir/lib/module/parallel_checker.ex +++ b/lib/elixir/lib/module/parallel_checker.ex @@ -5,7 +5,6 @@ defmodule Module.ParallelChecker do @type cache() :: {pid(), :ets.tid()} @type warning() :: term() - @type kind() :: :def | :defmacro @type mode() :: :elixir | :erlang @doc """ @@ -48,7 +47,25 @@ defmodule Module.ParallelChecker do @doc """ Spawns a process that runs the parallel checker. """ - def spawn({pid, checker}, module, info, log?) do + def spawn(pid_checker, module_map, log?, infer_types?, env) do + %{module: module, definitions: definitions, file: file} = module_map + + module_map = + if infer_types? do + %{module_map | signatures: Module.Types.infer(module, file, definitions, env)} + else + module_map + end + + with {pid, checker} <- pid_checker do + ets = :gen_server.call(checker, :ets, :infinity) + inner_spawn(pid, checker, module, cache_from_module_map(ets, module_map), log?) + end + + module_map + end + + defp inner_spawn(pid, checker, module, info, log?) do ref = make_ref() spawned = @@ -59,17 +76,22 @@ defmodule Module.ParallelChecker do {^ref, :cache, ets} -> Process.link(pid) - module_map = - if is_map(info) do - info - else - case File.read(info) do - {:ok, binary} -> maybe_module_map(binary, module) - {:error, _} -> nil - end + module_tuple = + cond do + is_tuple(info) -> + info + + is_binary(info) -> + with {:ok, binary} <- File.read(info), + {:ok, {_, [debug_info: chunk]}} <- :beam_lib.chunks(binary, [:debug_info]), + {:debug_info_v1, backend, data} = chunk, + {:ok, module_map} <- backend.debug_info(:elixir_v1, module, data, []) do + cache_from_module_map(ets, module_map) + else + _ -> nil + end end - module_map && cache_from_module_map(ets, module_map) send(checker, {ref, :cached}) receive do @@ -78,8 +100,8 @@ defmodule Module.ParallelChecker do :erlang.put(:elixir_compiler_info, {pid, self()}) warnings = - if module_map do - check_module(module_map, {checker, ets}, log?) + if module_tuple do + check_module(module_tuple, {checker, ets}, log?) else [] end @@ -147,7 +169,7 @@ defmodule Module.ParallelChecker do log? = not match?({_, false}, value) for {module, file} <- runtime_files do - spawn({self(), checker}, module, file, log?) + inner_spawn(self(), checker, module, file, log?) end count = :gen_server.call(checker, :start, :infinity) @@ -212,14 +234,43 @@ defmodule Module.ParallelChecker do ## Module checking - defp check_module(module_map, cache, log?) do + defp check_module(module_tuple, cache, log?) do + {module, file, line, definitions, no_warn_undefined, behaviours, impls, after_verify} = + module_tuple + + behaviour_warnings = + Module.Behaviour.check_behaviours_and_impls( + module, + file, + line, + behaviours, + impls, + definitions + ) + + diagnostics = + module + |> Module.Types.warnings(file, definitions, no_warn_undefined, cache) + |> Kernel.++(behaviour_warnings) + |> group_warnings() + |> emit_warnings(log?) + + Enum.each(after_verify, fn {verify_mod, verify_fun} -> + apply(verify_mod, verify_fun, [module]) + end) + + diagnostics + end + + defp module_map_to_module_tuple(module_map) do %{ module: module, file: file, compile_opts: compile_opts, definitions: definitions, attributes: attributes, - impls: impls + impls: impls, + after_verify: after_verify } = module_map # TODO: Match on anno directly in Elixir v1.22+ @@ -236,28 +287,7 @@ defmodule Module.ParallelChecker do |> extract_no_warn_undefined() |> merge_compiler_no_warn_undefined() - behaviour_warnings = - Module.Behaviour.check_behaviours_and_impls( - module, - file, - line, - behaviours, - impls, - definitions - ) - - diagnostics = - module - |> Module.Types.warnings(file, definitions, no_warn_undefined, cache) - |> Kernel.++(behaviour_warnings) - |> group_warnings() - |> emit_warnings(log?) - - module_map - |> Map.get(:after_verify, []) - |> Enum.each(fn {verify_mod, verify_fun} -> apply(verify_mod, verify_fun, [module]) end) - - diagnostics + {module, file, line, definitions, no_warn_undefined, behaviours, impls, after_verify} end defp extract_no_warn_undefined(compile_opts) do @@ -400,18 +430,6 @@ defmodule Module.ParallelChecker do _ -> %{} end - defp maybe_module_map(binary, module) when is_binary(binary) do - # If a module was compiled without debug_info, - # then there is no module_map for further verification. - with {:ok, {_, [debug_info: chunk]}} <- :beam_lib.chunks(binary, [:debug_info]), - {:debug_info_v1, backend, data} = chunk, - {:ok, module_map} <- backend.debug_info(:elixir_v1, module, data, []) do - module_map - else - _ -> nil - end - end - defp cache_from_module_map(ets, map) do exports = [{:__info__, 1}] ++ @@ -419,6 +437,7 @@ defmodule Module.ParallelChecker do for({function, :def, _meta, _clauses} <- map.definitions, do: function) cache_info(ets, map.module, exports, Map.new(map.deprecated), map.signatures, :elixir) + module_map_to_module_tuple(map) end defp cache_info(ets, module, exports, deprecated, sigs, mode) do diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 5257a784c0e..7a717783b6c 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -29,7 +29,6 @@ defmodule Module.Types do end end) - # TODO: Reuse context from patterns and guards {{fun, arity}, {:infer, Enum.reverse(pair_types)}} end end diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index 95d737d5d59..ade9e5071bd 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -173,12 +173,7 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> false -> ok end, - Signatures = case elixir_config:get(infer_signatures) of - true -> 'Elixir.Module.Types':infer(Module, File, AllDefinitions, CallbackE); - false -> #{} - end, - - ModuleMap = #{ + ModuleMapWithoutSignatures = #{ struct => get_struct(DataSet), module => Module, anno => Anno, @@ -192,12 +187,12 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> deprecated => get_deprecated(DataBag), defines_behaviour => defines_behaviour(DataBag), impls => Impls, - signatures => Signatures + signatures => #{} }, + ModuleMap = spawn_parallel_checker(CheckerInfo, ModuleMapWithoutSignatures, CallbackE), Binary = elixir_erl:compile(ModuleMap), Autoload = proplists:get_value(autoload, CompileOpts, true), - spawn_parallel_checker(CheckerInfo, Module, ModuleMap), {Binary, PersistedAttributes, Autoload} end), @@ -207,7 +202,7 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> elixir_env:trace({on_module, Binary, none}, ModuleE), warn_unused_attributes(DataSet, DataBag, PersistedAttributes, E), make_module_available(Module, Binary), - (CheckerInfo == undefined) andalso + (CheckerInfo == nil) andalso [VerifyMod:VerifyFun(Module) || {VerifyMod, VerifyFun} <- bag_lookup_element(DataBag, {accumulate, after_verify}, 2)], {module, Module, Binary, Result} @@ -534,19 +529,26 @@ beam_location(ModuleAsCharlist) -> checker_info() -> case get(elixir_checker_info) of - undefined -> undefined; + undefined -> nil; _ -> 'Elixir.Module.ParallelChecker':get() end. -spawn_parallel_checker(undefined, _Module, _ModuleMap) -> - ok; -spawn_parallel_checker(CheckerInfo, Module, ModuleMap) -> +spawn_parallel_checker(CheckerInfo, ModuleMap, E) -> Log = case erlang:get(elixir_code_diagnostics) of {_, false} -> false; _ -> true end, - 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, Module, ModuleMap, Log). + + Infer = elixir_config:get(infer_signatures), + + if + %% We need this clause for bootstrap reasons + CheckerInfo /= nil; Infer -> + 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, ModuleMap, Log, Infer, E); + true -> + ModuleMap + end. make_module_available(Module, Binary) -> case get(elixir_module_binaries) of From 7862ac484c7da6aeabb18629df2568873ebf76d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 13:04:16 +0100 Subject: [PATCH 02/22] Initial recursion for local calls --- lib/elixir/lib/module/types.ex | 168 +++- lib/elixir/lib/module/types/apply.ex | 830 +++++++++++++++++ lib/elixir/lib/module/types/expr.ex | 21 +- lib/elixir/lib/module/types/of.ex | 837 +----------------- lib/elixir/lib/module/types/pattern.ex | 2 +- lib/elixir/src/elixir_compiler.erl | 1 + .../test/elixir/module/types/type_helper.exs | 2 +- 7 files changed, 977 insertions(+), 884 deletions(-) create mode 100644 lib/elixir/lib/module/types/apply.ex diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 7a717783b6c..9efe13c31eb 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -3,33 +3,117 @@ defmodule Module.Types do alias Module.Types.{Descr, Expr, Pattern} + # TODO: Inference of tail recursion + # TODO: Checking of unused private functions/clauses + # These functions are not inferred because they are added/managed by the compiler @no_infer [__protocol__: 1, behaviour_info: 1] @doc false def infer(module, file, defs, env) do - context = context() - - for {{fun, arity}, :def, _meta, clauses} <- defs, - {fun, arity} not in @no_infer, - into: %{} do - stack = stack(:infer, file, module, {fun, arity}, :all, env) - expected = List.duplicate(Descr.dynamic(), arity) - - pair_types = - Enum.reduce(clauses, [], fn {meta, args, guards, body}, inferred -> - try do - {args, context} = - Pattern.of_head(args, guards, expected, :default, meta, stack, context) - - {return, _context} = Expr.of_expr(body, stack, context) - add_inferred(inferred, args, return, []) - rescue - e -> internal_error!(e, __STACKTRACE__, :def, meta, module, fun, args, guards, body) - end - end) - - {{fun, arity}, {:infer, Enum.reverse(pair_types)}} + finder = &List.keyfind(defs, &1, 0) + handler = &infer_signature_for(&1, &2, module, file, finder, env) + context = context({handler, %{}}) + + {types, _context} = + for {fun_arity, kind, _meta, _clauses} = def <- defs, + kind == :def and fun_arity not in @no_infer, + reduce: {[], context} do + {types, context} -> + {_kind, inferred, context} = + infer_signature_for(fun_arity, context, module, file, fn _ -> def end, env) + + {[{fun_arity, inferred} | types], context} + end + + Map.new(types) + end + + defp infer_signature_for(fun_arity, context, module, file, finder, env) do + case context.local_handler do + {_, %{^fun_arity => {kind, inferred}}} -> + {kind, inferred, context} + + {_, _} -> + {{fun, arity}, kind, _meta, clauses} = finder.(fun_arity) + expected = List.duplicate(Descr.dynamic(), arity) + + stack = stack(:infer, file, module, fun_arity, :all, env) + context = update_local_state(context, &Map.put(&1, fun_arity, {kind, :none})) + + {pair_types, context} = + Enum.reduce(clauses, {[], context}, fn + {meta, args, guards, body}, {inferred, context} -> + context = context(context.local_handler) + + try do + {args_types, context} = + Pattern.of_head(args, guards, expected, :default, meta, stack, context) + + {return_type, context} = Expr.of_expr(body, stack, context) + {add_inferred(inferred, args_types, return_type, []), context} + rescue + e -> + internal_error!(e, __STACKTRACE__, kind, meta, module, fun, args, guards, body) + end + end) + + inferred = {:infer, Enum.reverse(pair_types)} + {kind, inferred, update_local_state(context, &Map.put(&1, fun_arity, {kind, inferred}))} + end + end + + @doc false + def warnings(module, file, defs, no_warn_undefined, cache) do + finder = &List.keyfind(defs, &1, 0) + handler = &warnings_for(&1, &2, module, file, finder, no_warn_undefined, cache) + context = context({handler, %{}}) + + context = + Enum.reduce(defs, context, fn {fun_arity, _kind, _meta, _clauses} = def, context -> + finder = fn _ -> def end + + {_kind, _inferred, context} = + warnings_for(fun_arity, context, module, file, finder, no_warn_undefined, cache) + + context + end) + + context.warnings + end + + defp warnings_for(fun_arity, context, module, file, finder, no_warn_undefined, cache) do + case context.local_handler do + {_, %{^fun_arity => {kind, inferred}}} -> + {kind, inferred, context} + + {_, _} -> + {{fun, arity}, kind, meta, clauses} = finder.(fun_arity) + expected = List.duplicate(Descr.dynamic(), arity) + + file = with_file_meta(meta, file) + stack = stack(:dynamic, file, module, fun_arity, no_warn_undefined, cache) + context = update_local_state(context, &Map.put(&1, fun_arity, {kind, :none})) + + {pair_types, context} = + Enum.reduce(clauses, {[], context}, fn + {meta, args, guards, body}, {inferred, context} -> + context = fresh_context(context) + + try do + {args_types, context} = + Pattern.of_head(args, guards, expected, :default, meta, stack, context) + + {return_type, context} = Expr.of_expr(body, stack, context) + {add_inferred(inferred, args_types, return_type, []), context} + rescue + e -> + internal_error!(e, __STACKTRACE__, kind, meta, module, fun, args, guards, body) + end + end) + + inferred = {:infer, Enum.reverse(pair_types)} + {kind, inferred, update_local_state(context, &Map.put(&1, fun_arity, {kind, inferred}))} end end @@ -44,30 +128,6 @@ defmodule Module.Types do defp add_inferred([], args, return, acc), do: [{args, return} | Enum.reverse(acc)] - @doc false - def warnings(module, file, defs, no_warn_undefined, cache) do - context = context() - - Enum.flat_map(defs, fn {{fun, arity}, kind, meta, clauses} -> - file = with_file_meta(meta, file) - stack = stack(:dynamic, file, module, {fun, arity}, no_warn_undefined, cache) - expected = List.duplicate(Descr.dynamic(), arity) - - Enum.flat_map(clauses, fn {meta, args, guards, body} -> - try do - {_types, context} = - Pattern.of_head(args, guards, expected, :default, meta, stack, context) - - {_type, context} = Expr.of_expr(body, stack, context) - context.warnings - rescue - e -> - internal_error!(e, __STACKTRACE__, kind, meta, module, fun, args, guards, body) - end - end) - end) - end - defp with_file_meta(meta, file) do case Keyword.fetch(meta, :file) do {:ok, {meta_file, _}} -> meta_file @@ -145,16 +205,26 @@ defmodule Module.Types do end @doc false - def context() do + def context(local_handler, warnings \\ []) do %{ # A list of all warnings found so far - warnings: [], + warnings: warnings, # All vars and their types vars: %{}, # Variables and arguments from patterns pattern_info: nil, # If type checking has found an error/failure - failed: false + failed: false, + # Local handler + local_handler: local_handler } end + + defp fresh_context(%{local_handler: local_handler, warnings: warnings}) do + context(local_handler, warnings) + end + + defp update_local_state(%{local_handler: {handler, state}} = context, fun) do + %{context | local_handler: {handler, fun.(state)}} + end end diff --git a/lib/elixir/lib/module/types/apply.ex b/lib/elixir/lib/module/types/apply.ex new file mode 100644 index 00000000000..9296dbec896 --- /dev/null +++ b/lib/elixir/lib/module/types/apply.ex @@ -0,0 +1,830 @@ +defmodule Module.Types.Apply do + # Typing functionality shared between Expr and Pattern. + # Generic AST and Enum helpers go to Module.Types.Helpers. + @moduledoc false + + alias Module.ParallelChecker + import Module.Types.{Helpers, Descr} + + ## Signatures + + # Define strong arrows found in the standard library. + # A strong arrow means that, if a type outside of its + # domain is given, an error is raised. We are also + # ensuring that domains for the same function have + # no overlaps. + + # Remote for callback info functions + + kw = fn kw -> + kw + |> Enum.map(fn {key, type} when is_atom(key) -> + tuple([atom([key]), type]) + end) + |> Enum.reduce(&union/2) + |> list() + end + + fas = list(tuple([atom(), integer()])) + + shared_info = [ + attributes: list(tuple([atom(), list(term())])), + compile: kw.(version: list(integer()), source: list(integer()), options: list(term())), + exports: fas, + md5: binary(), + module: atom() + ] + + infos = + %{ + behaviour_info: [ + callbacks: fas, + optional_callbacks: fas + ], + module_info: [functions: fas, nifs: fas] ++ shared_info, + __info__: + [ + deprecated: list(tuple([tuple([atom(), integer()]), binary()])), + exports_md5: binary(), + functions: fas, + macros: fas, + struct: + list(closed_map(default: term(), field: atom(), required: boolean())) + |> union(atom([nil])) + ] ++ shared_info, + __protocol__: [ + module: atom(), + functions: fas, + consolidated?: boolean(), + impls: union(atom([:not_consolidated]), tuple([atom([:consolidated]), list(atom())])) + ] + } + + for {name, clauses} <- infos do + domain = atom(Keyword.keys(clauses)) + clauses = Enum.map(clauses, fn {key, return} -> {[atom([key])], return} end) + + defp signature(unquote(name), 1) do + {:strong, [unquote(Macro.escape(domain))], unquote(Macro.escape(clauses))} + end + end + + defp signature(:module_info, 0) do + {:strong, nil, [{[], unquote(Macro.escape(kw.(infos.module_info)))}]} + end + + defp signature(_, _), do: :none + + # Remote for compiler functions + + mfargs = [atom(), atom(), list(term())] + + send_destination = + pid() + |> union(reference()) + |> union(port()) + |> union(atom()) + |> union(tuple([atom(), atom()])) + + basic_arith_2_args_clauses = [ + {[integer(), integer()], integer()}, + {[integer(), float()], float()}, + {[float(), integer()], float()}, + {[float(), float()], float()} + ] + + is_clauses = [{[term()], boolean()}] + + args_or_arity = union(list(term()), integer()) + args_or_none = union(list(term()), atom([:none])) + extra_info = kw.(file: list(integer()), line: integer(), error_info: open_map()) + + raise_stacktrace = + list( + tuple([atom(), atom(), args_or_arity, extra_info]) + |> union(tuple([atom(), atom(), args_or_arity])) + |> union(tuple([fun(), args_or_arity, extra_info])) + |> union(tuple([fun(), args_or_arity])) + ) + + and_signature = + for left <- [true, false], right <- [true, false] do + {[atom([left]), atom([right])], atom([left and right])} + end + + or_signature = + for left <- [true, false], right <- [true, false] do + {[atom([left]), atom([right])], atom([left or right])} + end + + for {mod, fun, clauses} <- [ + # :binary + {:binary, :copy, [{[binary(), integer()], binary()}]}, + + # :erlang + {:erlang, :+, [{[integer()], integer()}, {[float()], float()}]}, + {:erlang, :+, basic_arith_2_args_clauses}, + {:erlang, :-, [{[integer()], integer()}, {[float()], float()}]}, + {:erlang, :-, basic_arith_2_args_clauses}, + {:erlang, :*, basic_arith_2_args_clauses}, + {:erlang, :/, [{[union(integer(), float()), union(integer(), float())], float()}]}, + {:erlang, :"/=", [{[term(), term()], boolean()}]}, + {:erlang, :"=/=", [{[term(), term()], boolean()}]}, + {:erlang, :<, [{[term(), term()], boolean()}]}, + {:erlang, :"=<", [{[term(), term()], boolean()}]}, + {:erlang, :==, [{[term(), term()], boolean()}]}, + {:erlang, :"=:=", [{[term(), term()], boolean()}]}, + {:erlang, :>, [{[term(), term()], boolean()}]}, + {:erlang, :>=, [{[term(), term()], boolean()}]}, + {:erlang, :abs, [{[integer()], integer()}, {[float()], float()}]}, + {:erlang, :and, and_signature}, + {:erlang, :atom_to_binary, [{[atom()], binary()}]}, + {:erlang, :atom_to_list, [{[atom()], list(integer())}]}, + {:erlang, :band, [{[integer(), integer()], integer()}]}, + {:erlang, :binary_part, [{[binary(), integer(), integer()], binary()}]}, + {:erlang, :binary_to_atom, [{[binary()], atom()}]}, + {:erlang, :binary_to_existing_atom, [{[binary()], atom()}]}, + {:erlang, :binary_to_integer, [{[binary()], integer()}]}, + {:erlang, :binary_to_integer, [{[binary(), integer()], integer()}]}, + {:erlang, :binary_to_float, [{[binary()], float()}]}, + {:erlang, :bit_size, [{[binary()], integer()}]}, + {:erlang, :bnot, [{[integer()], integer()}]}, + {:erlang, :bor, [{[integer(), integer()], integer()}]}, + {:erlang, :bsl, [{[integer(), integer()], integer()}]}, + {:erlang, :bsr, [{[integer(), integer()], integer()}]}, + {:erlang, :bxor, [{[integer(), integer()], integer()}]}, + {:erlang, :byte_size, [{[binary()], integer()}]}, + {:erlang, :ceil, [{[union(integer(), float())], integer()}]}, + {:erlang, :div, [{[integer(), integer()], integer()}]}, + {:erlang, :error, [{[term()], none()}]}, + {:erlang, :error, [{[term(), args_or_none], none()}]}, + {:erlang, :error, [{[term(), args_or_none, kw.(error_info: open_map())], none()}]}, + {:erlang, :floor, [{[union(integer(), float())], integer()}]}, + {:erlang, :function_exported, [{[atom(), atom(), integer()], boolean()}]}, + {:erlang, :integer_to_binary, [{[integer()], binary()}]}, + {:erlang, :integer_to_binary, [{[integer(), integer()], binary()}]}, + {:erlang, :integer_to_list, [{[integer()], non_empty_list(integer())}]}, + {:erlang, :integer_to_list, [{[integer(), integer()], non_empty_list(integer())}]}, + {:erlang, :is_atom, is_clauses}, + {:erlang, :is_binary, is_clauses}, + {:erlang, :is_bitstring, is_clauses}, + {:erlang, :is_boolean, is_clauses}, + {:erlang, :is_float, is_clauses}, + {:erlang, :is_function, is_clauses}, + {:erlang, :is_function, [{[term(), integer()], boolean()}]}, + {:erlang, :is_integer, is_clauses}, + {:erlang, :is_list, is_clauses}, + {:erlang, :is_map, is_clauses}, + {:erlang, :is_map_key, [{[term(), open_map()], boolean()}]}, + {:erlang, :is_number, is_clauses}, + {:erlang, :is_pid, is_clauses}, + {:erlang, :is_port, is_clauses}, + {:erlang, :is_reference, is_clauses}, + {:erlang, :is_tuple, is_clauses}, + {:erlang, :length, [{[list(term())], integer()}]}, + {:erlang, :list_to_atom, [{[list(integer())], atom()}]}, + {:erlang, :list_to_existing_atom, [{[list(integer())], atom()}]}, + {:erlang, :list_to_float, [{[non_empty_list(integer())], float()}]}, + {:erlang, :list_to_integer, [{[non_empty_list(integer())], integer()}]}, + {:erlang, :list_to_integer, [{[non_empty_list(integer()), integer()], integer()}]}, + {:erlang, :list_to_tuple, [{[list(term())], dynamic(open_tuple([]))}]}, + {:erlang, :make_ref, [{[], reference()}]}, + {:erlang, :map_size, [{[open_map()], integer()}]}, + {:erlang, :node, [{[], atom()}]}, + {:erlang, :node, [{[pid() |> union(reference()) |> union(port())], atom()}]}, + {:erlang, :not, [{[atom([false])], atom([true])}, {[atom([true])], atom([false])}]}, + {:erlang, :or, or_signature}, + {:erlang, :raise, [{[atom([:error, :exit, :throw]), term(), raise_stacktrace], none()}]}, + {:erlang, :rem, [{[integer(), integer()], integer()}]}, + {:erlang, :round, [{[union(integer(), float())], integer()}]}, + {:erlang, :self, [{[], pid()}]}, + {:erlang, :spawn, [{[fun()], pid()}]}, + {:erlang, :spawn, [{mfargs, pid()}]}, + {:erlang, :spawn_link, [{[fun()], pid()}]}, + {:erlang, :spawn_link, [{mfargs, pid()}]}, + {:erlang, :spawn_monitor, [{[fun()], tuple([reference(), pid()])}]}, + {:erlang, :spawn_monitor, [{mfargs, tuple([reference(), pid()])}]}, + {:erlang, :tuple_size, [{[open_tuple([])], integer()}]}, + {:erlang, :trunc, [{[union(integer(), float())], integer()}]}, + + # TODO: Replace term()/dynamic() by parametric types + {:erlang, :++, [{[list(term()), term()], dynamic(list(term(), term()))}]}, + {:erlang, :--, [{[list(term()), list(term())], dynamic(list(term()))}]}, + {:erlang, :andalso, [{[boolean(), term()], dynamic()}]}, + {:erlang, :delete_element, [{[integer(), open_tuple([])], dynamic(open_tuple([]))}]}, + {:erlang, :hd, [{[non_empty_list(term(), term())], dynamic()}]}, + {:erlang, :element, [{[integer(), open_tuple([])], dynamic()}]}, + {:erlang, :insert_element, + [{[integer(), open_tuple([]), term()], dynamic(open_tuple([]))}]}, + {:erlang, :max, [{[term(), term()], dynamic()}]}, + {:erlang, :min, [{[term(), term()], dynamic()}]}, + {:erlang, :orelse, [{[boolean(), term()], dynamic()}]}, + {:erlang, :send, [{[send_destination, term()], dynamic()}]}, + {:erlang, :setelement, [{[integer(), open_tuple([]), term()], dynamic(open_tuple([]))}]}, + {:erlang, :tl, [{[non_empty_list(term(), term())], dynamic()}]}, + {:erlang, :tuple_to_list, [{[open_tuple([])], dynamic(list(term()))}]} + ] do + [arity] = Enum.map(clauses, fn {args, _return} -> length(args) end) |> Enum.uniq() + + true = + Code.ensure_loaded?(mod) and + (function_exported?(mod, fun, arity) or fun in [:orelse, :andalso]) + + domain_clauses = + case clauses do + [_] -> + {:strong, nil, clauses} + + _ -> + domain = + clauses + |> Enum.map(fn {args, _} -> args end) + |> Enum.zip_with(fn types -> Enum.reduce(types, &union/2) end) + + {:strong, domain, clauses} + end + + defp signature(unquote(mod), unquote(fun), unquote(arity)), + do: unquote(Macro.escape(domain_clauses)) + end + + defp signature(_mod, _fun, _arity), do: :none + + @doc """ + Applies a function in unknown modules. + + Used only by info functions. + """ + def remote(name, args_types, expr, stack, context) do + arity = length(args_types) + + case signature(name, arity) do + :none -> {dynamic(), context} + info -> apply_remote(info, args_types, expr, stack, context) + end + end + + @doc """ + Applies a function in a given module. + """ + def remote(:erlang, :element, [_, tuple], {_, meta, [index, _]} = expr, stack, context) + when is_integer(index) do + case tuple_fetch(tuple, index - 1) do + {_optional?, value_type} -> + {value_type, context} + + :badtuple -> + {error_type(), badremote_error(expr, [integer(), tuple], stack, context)} + + reason -> + {error_type(), error({reason, expr, tuple, index - 1, context}, meta, stack, context)} + end + end + + def remote( + :erlang, + :insert_element, + [_, tuple, value], + {_, meta, [index, _, _]} = expr, + stack, + context + ) + when is_integer(index) do + case tuple_insert_at(tuple, index - 1, value) do + value_type when is_descr(value_type) -> + {value_type, context} + + :badtuple -> + {error_type(), badremote_error(expr, [integer(), tuple, value], stack, context)} + + reason -> + {error_type(), error({reason, expr, tuple, index - 2, context}, meta, stack, context)} + end + end + + def remote(:erlang, :delete_element, [_, tuple], {_, meta, [index, _]} = expr, stack, context) + when is_integer(index) do + case tuple_delete_at(tuple, index - 1) do + value_type when is_descr(value_type) -> + {value_type, context} + + :badtuple -> + {error_type(), badremote_error(expr, [integer(), tuple], stack, context)} + + reason -> + {error_type(), error({reason, expr, tuple, index - 1, context}, meta, stack, context)} + end + end + + def remote(:erlang, :make_tuple, [_, elem], {_, _meta, [size, _]}, _stack, context) + when is_integer(size) and size >= 0 do + {tuple(List.duplicate(elem, size)), context} + end + + def remote(:erlang, :hd, [list], expr, stack, context) do + case list_hd(list) do + {_, value_type} -> + {value_type, context} + + :badnonemptylist -> + {error_type(), badremote_error(expr, [list], stack, context)} + end + end + + def remote(:erlang, :tl, [list], expr, stack, context) do + case list_tl(list) do + {_, value_type} -> + {value_type, context} + + :badnonemptylist -> + {error_type(), badremote_error(expr, [list], stack, context)} + end + end + + def remote(:erlang, name, [left, right] = args_types, expr, stack, context) + when name in [:>=, :"=<", :>, :<, :min, :max] do + context = + cond do + stack.mode == :infer -> + context + + match?({false, _}, map_fetch(left, :__struct__)) or + match?({false, _}, map_fetch(right, :__struct__)) -> + warning = {:struct_comparison, expr, context} + warn(__MODULE__, warning, elem(expr, 1), stack, context) + + number_type?(left) and number_type?(right) -> + context + + disjoint?(left, right) -> + warning = {:mismatched_comparison, expr, context} + warn(__MODULE__, warning, elem(expr, 1), stack, context) + + true -> + context + end + + if name in [:min, :max] do + {union(left, right), context} + else + {return(boolean(), args_types, stack), context} + end + end + + def remote(:erlang, name, [left, right] = args_types, expr, stack, context) + when name in [:==, :"/=", :"=:=", :"=/="] do + context = + cond do + stack.mode == :infer -> + context + + name in [:==, :"/="] and number_type?(left) and number_type?(right) -> + context + + disjoint?(left, right) -> + warning = {:mismatched_comparison, expr, context} + warn(__MODULE__, warning, elem(expr, 1), stack, context) + + true -> + context + end + + {return(boolean(), args_types, stack), context} + end + + def remote(mod, name, args_types, expr, stack, context) do + arity = length(args_types) + + case :elixir_rewrite.inline(mod, name, arity) do + {mod, name} -> + remote(mod, name, args_types, expr, stack, context) + + false -> + {info, context} = signature(mod, name, arity, elem(expr, 1), stack, context) + apply_remote(info, args_types, expr, stack, context) + end + end + + defp apply_remote(info, args_types, expr, stack, context) do + case apply_types(info, args_types, stack) do + {:ok, type} -> + {type, context} + + {:error, domain, clauses} -> + error = {:badremote, expr, args_types, domain, clauses, context} + {error_type(), error(error, elem(expr, 1), stack, context)} + end + end + + @doc """ + Gets a mfa signature. + + It returns either a tuple with the remote information and the context. + The remote information may be one of: + + * `:none` - no typing information found. + + * `{:infer, clauses}` - clauses from inferences. You must check all + all clauses and return the union between them. They are dynamic + and they can only be converted into arrows by computing the union + of all arguments. + + * `{:strong, domain or nil, clauses}` - clauses from signatures. So far + these are strong arrows with non-overlapping domains + + """ + def signature(module, fun, arity, meta, stack, context) when is_atom(module) do + if Keyword.get(meta, :runtime_module, false) do + {:none, context} + else + case signature(module, fun, arity) do + :none -> export(module, fun, arity, meta, stack, context) + clauses -> {clauses, context} + end + end + end + + defp export(_module, :module_info, arity, _meta, _stack, context) when arity in [0, 1] do + {signature(:module_info, arity), context} + end + + defp export(_module, _fun, _arity, _meta, %{cache: %Macro.Env{}}, context) do + {:none, context} + end + + defp export(module, fun, arity, meta, stack, context) do + case ParallelChecker.fetch_export(stack.cache, module, fun, arity) do + {:ok, mode, reason, info} -> + info = if info == :none, do: signature(fun, arity), else: info + {info, check_deprecated(mode, module, fun, arity, reason, meta, stack, context)} + + {:error, type} -> + context = + if warn_undefined?(module, fun, arity, stack) do + warn(__MODULE__, {:undefined, type, module, fun, arity}, meta, stack, context) + else + context + end + + {:none, context} + end + end + + defp check_deprecated(:elixir, module, fun, arity, reason, meta, stack, context) do + if reason do + warn(__MODULE__, {:deprecated, module, fun, arity, reason}, meta, stack, context) + else + context + end + end + + defp check_deprecated(:erlang, module, fun, arity, _reason, meta, stack, context) do + case :otp_internal.obsolete(module, fun, arity) do + {:deprecated, string} when is_list(string) -> + reason = string |> List.to_string() |> :string.titlecase() + warn(__MODULE__, {:deprecated, module, fun, arity, reason}, meta, stack, context) + + {:deprecated, string, removal} when is_list(string) and is_list(removal) -> + reason = string |> List.to_string() |> :string.titlecase() + reason = "It will be removed in #{removal}. #{reason}" + warn(__MODULE__, {:deprecated, module, fun, arity, reason}, meta, stack, context) + + _ -> + context + end + end + + defp warn_undefined?(_, _, _, %{no_warn_undefined: :all}) do + false + end + + defp warn_undefined?(module, fun, arity, stack) do + not Enum.any?(stack.no_warn_undefined, &(&1 == module or &1 == {module, fun, arity})) + end + + ## Local + + def local(_fun, _args_types, _stack, context) do + {dynamic(), context} + end + + ## Application helpers + + defp return(type, args_types, stack) do + cond do + stack.mode == :static -> type + Enum.any?(args_types, &gradual?/1) -> dynamic(type) + true -> type + end + end + + defp apply_types(:none, _args_types, _stack) do + {:ok, dynamic()} + end + + defp apply_types({:strong, nil, [{expected, return}] = clauses}, args_types, stack) do + # Optimize single clauses as the domain is the single clause args. + case zip_compatible?(args_types, expected) do + true -> {:ok, return(return, args_types, stack)} + false -> {:error, expected, clauses} + end + end + + defp apply_types({:strong, domain, clauses}, args_types, stack) do + # If the type is only gradual, the compatibility check is the same + # as a non disjoint check. So we skip checking compatibility twice. + with true <- zip_compatible_or_only_gradual?(args_types, domain), + [_ | _] = returns <- + for({expected, return} <- clauses, zip_not_disjoint?(args_types, expected), do: return) do + {:ok, returns |> Enum.reduce(&union/2) |> return(args_types, stack)} + else + _ -> {:error, domain, clauses} + end + end + + defp apply_types({:infer, clauses}, args_types, _stack) do + case for({expected, return} <- clauses, zip_not_disjoint?(args_types, expected), do: return) do + [] -> + domain = + clauses + |> Enum.map(fn {args, _} -> args end) + |> Enum.zip_with(fn types -> Enum.reduce(types, &union/2) end) + + {:error, domain, clauses} + + returns -> + {:ok, returns |> Enum.reduce(&union/2) |> dynamic()} + end + end + + defp zip_compatible_or_only_gradual?([actual | actuals], [expected | expecteds]) do + (only_gradual?(actual) or compatible?(actual, expected)) and + zip_compatible_or_only_gradual?(actuals, expecteds) + end + + defp zip_compatible_or_only_gradual?([], []), do: true + + defp zip_compatible?([actual | actuals], [expected | expecteds]) do + compatible?(actual, expected) and zip_compatible?(actuals, expecteds) + end + + defp zip_compatible?([], []), do: true + + defp zip_not_disjoint?([actual | actuals], [expected | expecteds]) do + not disjoint?(actual, expected) and zip_not_disjoint?(actuals, expecteds) + end + + defp zip_not_disjoint?([], []), do: true + + ## Error handling + + defp error(warning, meta, stack, context) do + error(__MODULE__, warning, meta, stack, context) + end + + defp badremote_error({{:., _, [mod, fun]}, meta, _} = expr, args_types, stack, context) do + {_type, domain, [{args, _} | _] = clauses} = signature(mod, fun, length(args_types)) + error({:badremote, expr, args_types, domain || args, clauses, context}, meta, stack, context) + end + + ## Diagnosstics + + def format_diagnostic({:badindex, expr, type, index, context}) do + traces = collect_traces(expr, context) + + %{ + details: %{typing_traces: traces}, + message: + IO.iodata_to_binary([ + """ + expected a tuple with at least #{pluralize(index + 1, "element", "elements")} in #{format_mfa(expr)}: + + #{expr_to_string(expr) |> indent(4)} + + the given type does not have the given index: + + #{to_quoted_string(type) |> indent(4)} + """, + format_traces(traces) + ]) + } + end + + def format_diagnostic({:badremote, expr, args_types, domain, clauses, context}) do + traces = collect_traces(expr, context) + {{:., _, [mod, fun]}, _, args} = expr + {mod, fun, args, converter} = :elixir_rewrite.erl_to_ex(mod, fun, args) + + explanation = + if i = Enum.find_index(args_types, &empty?/1) do + """ + the #{integer_to_ordinal(i + 1)} argument is empty (often represented as none()), \ + most likely because it is the result of an expression that always fails, such as \ + a `raise` or a previous invalid call. This causes any function called with this \ + value to fail + """ + else + """ + but expected one of: + #{clauses_args_to_quoted_string(clauses, converter)} + """ + end + + %{ + details: %{typing_traces: traces}, + message: + IO.iodata_to_binary([ + """ + incompatible types given to #{Exception.format_mfa(mod, fun, length(args))}: + + #{expr_to_string(expr) |> indent(4)} + + given types: + + #{args_to_quoted_string(args_types, domain, converter) |> indent(4)} + + """, + explanation, + format_traces(traces) + ]) + } + end + + def format_diagnostic({:mismatched_comparison, expr, context}) do + traces = collect_traces(expr, context) + + %{ + details: %{typing_traces: traces}, + message: + IO.iodata_to_binary([ + """ + comparison between distinct types found: + + #{expr_to_string(expr) |> indent(4)} + """, + format_traces(traces), + """ + + While Elixir can compare across all types, you are comparing \ + across types which are always disjoint, and the result is either \ + always true or always false + """ + ]) + } + end + + def format_diagnostic({:struct_comparison, expr, context}) do + traces = collect_traces(expr, context) + + %{ + details: %{typing_traces: traces}, + message: + IO.iodata_to_binary([ + """ + comparison with structs found: + + #{expr_to_string(expr) |> indent(4)} + """, + format_traces(traces), + """ + + Comparison operators (>, <, >=, <=, min, and max) perform structural \ + and not semantic comparison. Comparing with a struct won't give meaningful \ + results. Structs that can be compared typically define a compare/2 function \ + within their modules that can be used for semantic comparison. + """ + ]) + } + end + + def format_diagnostic({:undefined, :module, module, fun, arity}) do + top = + if fun == :__struct__ and arity == 0 do + "struct #{inspect(module)}" + else + Exception.format_mfa(module, fun, arity) + end + + %{ + message: + IO.iodata_to_binary([ + top, + " is undefined (module ", + inspect(module), + " is not available or is yet to be defined)", + UndefinedFunctionError.hint_for_missing_module(module, fun, arity) + ]), + group: true + } + end + + def format_diagnostic({:undefined, :function, module, :__struct__, 0}) do + %{ + message: + "struct #{inspect(module)} is undefined (there is such module but it does not define a struct)", + group: true + } + end + + def format_diagnostic({:undefined, :function, module, fun, arity}) do + %{ + message: + IO.iodata_to_binary([ + Exception.format_mfa(module, fun, arity), + " is undefined or private", + UndefinedFunctionError.hint_for_loaded_module(module, fun, arity) + ]), + group: true + } + end + + def format_diagnostic({:deprecated, module, fun, arity, reason}) do + %{ + message: + IO.iodata_to_binary([ + Exception.format_mfa(module, fun, arity), + " is deprecated. ", + reason + ]), + group: true + } + end + + defp pluralize(1, singular, _), do: "1 #{singular}" + defp pluralize(i, _, plural), do: "#{i} #{plural}" + + defp format_mfa({{:., _, [mod, fun]}, _, args}) do + {mod, fun, args, _} = :elixir_rewrite.erl_to_ex(mod, fun, args) + Exception.format_mfa(mod, fun, length(args)) + end + + ## Algebra helpers + + alias Inspect.Algebra, as: IA + + defp clauses_args_to_quoted_string([{args, _return}], converter) do + "\n " <> (clause_args_to_quoted_string(args, converter) |> indent(4)) + end + + defp clauses_args_to_quoted_string(clauses, converter) do + clauses + |> Enum.with_index(fn {args, _return}, index -> + """ + + ##{index + 1} + #{clause_args_to_quoted_string(args, converter)}\ + """ + |> indent(4) + end) + |> Enum.join("\n") + end + + defp clause_args_to_quoted_string(args, converter) do + docs = Enum.map(args, &(&1 |> to_quoted() |> Code.Formatter.to_algebra())) + args_docs_to_quoted_string(converter.(docs)) + end + + defp args_to_quoted_string(args_types, domain, converter) do + ansi? = IO.ANSI.enabled?() + + docs = + Enum.zip_with(args_types, domain, fn actual, expected -> + doc = actual |> to_quoted() |> Code.Formatter.to_algebra() + + cond do + compatible?(actual, expected) -> doc + ansi? -> IA.concat(IA.color(doc, IO.ANSI.red()), IA.color(IA.empty(), IO.ANSI.reset())) + true -> IA.concat(["-", doc, "-"]) + end + end) + + args_docs_to_quoted_string(converter.(docs)) + end + + defp args_docs_to_quoted_string(docs) do + doc = IA.fold(docs, fn doc, acc -> IA.glue(IA.concat(doc, ","), acc) end) + + wrapped_docs = + case docs do + [_] -> IA.concat("(", IA.concat(doc, ")")) + _ -> IA.group(IA.glue(IA.nest(IA.glue("(", "", doc), 2), "", ")")) + end + + wrapped_docs + |> IA.format(98) + |> IO.iodata_to_binary() + |> case do + "(\n" <> _ = multiple_lines -> multiple_lines + single_line -> binary_slice(single_line, 1..-2//1) + end + end + + defp integer_to_ordinal(i) do + case rem(i, 10) do + 1 when rem(i, 100) != 11 -> "#{i}st" + 2 when rem(i, 100) != 12 -> "#{i}nd" + 3 when rem(i, 100) != 13 -> "#{i}rd" + _ -> "#{i}th" + end + end +end diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex index 88887c4d88f..ca415c2c279 100644 --- a/lib/elixir/lib/module/types/expr.ex +++ b/lib/elixir/lib/module/types/expr.ex @@ -1,7 +1,7 @@ defmodule Module.Types.Expr do @moduledoc false - alias Module.Types.{Of, Pattern} + alias Module.Types.{Apply, Of, Pattern} import Module.Types.{Helpers, Descr} 14 = length(Macro.Env.__info__(:struct)) @@ -391,7 +391,7 @@ defmodule Module.Types.Expr do {mods, context} = Of.modules(remote_type, name, arity, expr, meta, stack, context) context = - Enum.reduce(mods, context, &(Of.remote(&1, name, arity, meta, stack, &2) |> elem(1))) + Enum.reduce(mods, context, &(Apply.signature(&1, name, arity, meta, stack, &2) |> elem(1))) {fun(), context} end @@ -402,11 +402,11 @@ defmodule Module.Types.Expr do {fun(), context} end - # TODO: local_call(arg) + # Local calls def of_expr({fun, _meta, args}, stack, context) when is_atom(fun) and is_list(args) do - {_arg_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) - {dynamic(), context} + {args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) + Apply.local(fun, args_types, stack, context) end # var @@ -427,9 +427,8 @@ defmodule Module.Types.Expr do {info, context} = Of.struct_info(exception, meta, stack, context) {Of.struct_type(exception, info, args), context} else - # If the exception cannot be found or is invalid, - # we call Of.remote/5 to emit a warning. - {_, context} = Of.remote(exception, :__struct__, 0, meta, stack, context) + # If the exception cannot be found or is invalid, fetch the signature to emit warnings. + {_, context} = Apply.signature(exception, :__struct__, 0, meta, stack, context) {error_type(), context} end end) @@ -521,17 +520,17 @@ defmodule Module.Types.Expr do ## General helpers defp apply_many([], function, args_types, expr, stack, context) do - Of.apply(function, args_types, expr, stack, context) + Apply.remote(function, args_types, expr, stack, context) end defp apply_many([mod], function, args_types, expr, stack, context) do - Of.apply(mod, function, args_types, expr, stack, context) + Apply.remote(mod, function, args_types, expr, stack, context) end defp apply_many(mods, function, args_types, expr, stack, context) do {returns, context} = Enum.map_reduce(mods, context, fn mod, context -> - Of.apply(mod, function, args_types, expr, stack, context) + Apply.remote(mod, function, args_types, expr, stack, context) end) {Enum.reduce(returns, &union/2), context} diff --git a/lib/elixir/lib/module/types/of.ex b/lib/elixir/lib/module/types/of.ex index 03a7c12ca72..efd2631baed 100644 --- a/lib/elixir/lib/module/types/of.ex +++ b/lib/elixir/lib/module/types/of.ex @@ -2,8 +2,6 @@ defmodule Module.Types.Of do # Typing functionality shared between Expr and Pattern. # Generic AST and Enum helpers go to Module.Types.Helpers. @moduledoc false - - alias Module.ParallelChecker import Module.Types.{Helpers, Descr} @prefix quote(do: ...) @@ -215,7 +213,8 @@ defmodule Module.Types.Of do end _ -> - {_, context} = export(struct, :__struct__, 0, meta, stack, context) + # Fetch the signature to validate for warnings. + {_, context} = Module.Types.Apply.signature(struct, :__struct__, 0, meta, stack, context) info = struct.__info__(:struct) || @@ -333,7 +332,7 @@ defmodule Module.Types.Of do ## Modules @doc """ - Returns the modules. + Returns modules in a type. The call information is used on report reporting. """ @@ -348,568 +347,6 @@ defmodule Module.Types.Of do end end - ## Remotes - - # Define strong arrows found in the standard library. - # A strong arrow means that, if a type outside of its - # domain is given, an error is raised. We are also - # ensuring that domains for the same function have - # no overlaps. - - # Remote for callback info functions - - kw = fn kw -> - kw - |> Enum.map(fn {key, type} when is_atom(key) -> - tuple([atom([key]), type]) - end) - |> Enum.reduce(&union/2) - |> list() - end - - fas = list(tuple([atom(), integer()])) - - shared_info = [ - attributes: list(tuple([atom(), list(term())])), - compile: kw.(version: list(integer()), source: list(integer()), options: list(term())), - exports: fas, - md5: binary(), - module: atom() - ] - - infos = - %{ - behaviour_info: [ - callbacks: fas, - optional_callbacks: fas - ], - module_info: [functions: fas, nifs: fas] ++ shared_info, - __info__: - [ - deprecated: list(tuple([tuple([atom(), integer()]), binary()])), - exports_md5: binary(), - functions: fas, - macros: fas, - struct: - list(closed_map(default: term(), field: atom(), required: boolean())) - |> union(atom([nil])) - ] ++ shared_info, - __protocol__: [ - module: atom(), - functions: fas, - consolidated?: boolean(), - impls: union(atom([:not_consolidated]), tuple([atom([:consolidated]), list(atom())])) - ] - } - - for {name, clauses} <- infos do - domain = atom(Keyword.keys(clauses)) - clauses = Enum.map(clauses, fn {key, return} -> {[atom([key])], return} end) - - defp remote(unquote(name), 1) do - {:strong, [unquote(Macro.escape(domain))], unquote(Macro.escape(clauses))} - end - end - - defp remote(:module_info, 0) do - {:strong, nil, [{[], unquote(Macro.escape(kw.(infos.module_info)))}]} - end - - defp remote(_, _), do: :none - - # Remote for compiler functions - - mfargs = [atom(), atom(), list(term())] - - send_destination = - pid() - |> union(reference()) - |> union(port()) - |> union(atom()) - |> union(tuple([atom(), atom()])) - - basic_arith_2_args_clauses = [ - {[integer(), integer()], integer()}, - {[integer(), float()], float()}, - {[float(), integer()], float()}, - {[float(), float()], float()} - ] - - is_clauses = [{[term()], boolean()}] - - args_or_arity = union(list(term()), integer()) - args_or_none = union(list(term()), atom([:none])) - extra_info = kw.(file: list(integer()), line: integer(), error_info: open_map()) - - raise_stacktrace = - list( - tuple([atom(), atom(), args_or_arity, extra_info]) - |> union(tuple([atom(), atom(), args_or_arity])) - |> union(tuple([fun(), args_or_arity, extra_info])) - |> union(tuple([fun(), args_or_arity])) - ) - - and_signature = - for left <- [true, false], right <- [true, false] do - {[atom([left]), atom([right])], atom([left and right])} - end - - or_signature = - for left <- [true, false], right <- [true, false] do - {[atom([left]), atom([right])], atom([left or right])} - end - - for {mod, fun, clauses} <- [ - # :binary - {:binary, :copy, [{[binary(), integer()], binary()}]}, - - # :erlang - {:erlang, :+, [{[integer()], integer()}, {[float()], float()}]}, - {:erlang, :+, basic_arith_2_args_clauses}, - {:erlang, :-, [{[integer()], integer()}, {[float()], float()}]}, - {:erlang, :-, basic_arith_2_args_clauses}, - {:erlang, :*, basic_arith_2_args_clauses}, - {:erlang, :/, [{[union(integer(), float()), union(integer(), float())], float()}]}, - {:erlang, :"/=", [{[term(), term()], boolean()}]}, - {:erlang, :"=/=", [{[term(), term()], boolean()}]}, - {:erlang, :<, [{[term(), term()], boolean()}]}, - {:erlang, :"=<", [{[term(), term()], boolean()}]}, - {:erlang, :==, [{[term(), term()], boolean()}]}, - {:erlang, :"=:=", [{[term(), term()], boolean()}]}, - {:erlang, :>, [{[term(), term()], boolean()}]}, - {:erlang, :>=, [{[term(), term()], boolean()}]}, - {:erlang, :abs, [{[integer()], integer()}, {[float()], float()}]}, - {:erlang, :and, and_signature}, - {:erlang, :atom_to_binary, [{[atom()], binary()}]}, - {:erlang, :atom_to_list, [{[atom()], list(integer())}]}, - {:erlang, :band, [{[integer(), integer()], integer()}]}, - {:erlang, :binary_part, [{[binary(), integer(), integer()], binary()}]}, - {:erlang, :binary_to_atom, [{[binary()], atom()}]}, - {:erlang, :binary_to_existing_atom, [{[binary()], atom()}]}, - {:erlang, :binary_to_integer, [{[binary()], integer()}]}, - {:erlang, :binary_to_integer, [{[binary(), integer()], integer()}]}, - {:erlang, :binary_to_float, [{[binary()], float()}]}, - {:erlang, :bit_size, [{[binary()], integer()}]}, - {:erlang, :bnot, [{[integer()], integer()}]}, - {:erlang, :bor, [{[integer(), integer()], integer()}]}, - {:erlang, :bsl, [{[integer(), integer()], integer()}]}, - {:erlang, :bsr, [{[integer(), integer()], integer()}]}, - {:erlang, :bxor, [{[integer(), integer()], integer()}]}, - {:erlang, :byte_size, [{[binary()], integer()}]}, - {:erlang, :ceil, [{[union(integer(), float())], integer()}]}, - {:erlang, :div, [{[integer(), integer()], integer()}]}, - {:erlang, :error, [{[term()], none()}]}, - {:erlang, :error, [{[term(), args_or_none], none()}]}, - {:erlang, :error, [{[term(), args_or_none, kw.(error_info: open_map())], none()}]}, - {:erlang, :floor, [{[union(integer(), float())], integer()}]}, - {:erlang, :function_exported, [{[atom(), atom(), integer()], boolean()}]}, - {:erlang, :integer_to_binary, [{[integer()], binary()}]}, - {:erlang, :integer_to_binary, [{[integer(), integer()], binary()}]}, - {:erlang, :integer_to_list, [{[integer()], non_empty_list(integer())}]}, - {:erlang, :integer_to_list, [{[integer(), integer()], non_empty_list(integer())}]}, - {:erlang, :is_atom, is_clauses}, - {:erlang, :is_binary, is_clauses}, - {:erlang, :is_bitstring, is_clauses}, - {:erlang, :is_boolean, is_clauses}, - {:erlang, :is_float, is_clauses}, - {:erlang, :is_function, is_clauses}, - {:erlang, :is_function, [{[term(), integer()], boolean()}]}, - {:erlang, :is_integer, is_clauses}, - {:erlang, :is_list, is_clauses}, - {:erlang, :is_map, is_clauses}, - {:erlang, :is_map_key, [{[term(), open_map()], boolean()}]}, - {:erlang, :is_number, is_clauses}, - {:erlang, :is_pid, is_clauses}, - {:erlang, :is_port, is_clauses}, - {:erlang, :is_reference, is_clauses}, - {:erlang, :is_tuple, is_clauses}, - {:erlang, :length, [{[list(term())], integer()}]}, - {:erlang, :list_to_atom, [{[list(integer())], atom()}]}, - {:erlang, :list_to_existing_atom, [{[list(integer())], atom()}]}, - {:erlang, :list_to_float, [{[non_empty_list(integer())], float()}]}, - {:erlang, :list_to_integer, [{[non_empty_list(integer())], integer()}]}, - {:erlang, :list_to_integer, [{[non_empty_list(integer()), integer()], integer()}]}, - {:erlang, :list_to_tuple, [{[list(term())], dynamic(open_tuple([]))}]}, - {:erlang, :make_ref, [{[], reference()}]}, - {:erlang, :map_size, [{[open_map()], integer()}]}, - {:erlang, :node, [{[], atom()}]}, - {:erlang, :node, [{[pid() |> union(reference()) |> union(port())], atom()}]}, - {:erlang, :not, [{[atom([false])], atom([true])}, {[atom([true])], atom([false])}]}, - {:erlang, :or, or_signature}, - {:erlang, :raise, [{[atom([:error, :exit, :throw]), term(), raise_stacktrace], none()}]}, - {:erlang, :rem, [{[integer(), integer()], integer()}]}, - {:erlang, :round, [{[union(integer(), float())], integer()}]}, - {:erlang, :self, [{[], pid()}]}, - {:erlang, :spawn, [{[fun()], pid()}]}, - {:erlang, :spawn, [{mfargs, pid()}]}, - {:erlang, :spawn_link, [{[fun()], pid()}]}, - {:erlang, :spawn_link, [{mfargs, pid()}]}, - {:erlang, :spawn_monitor, [{[fun()], tuple([reference(), pid()])}]}, - {:erlang, :spawn_monitor, [{mfargs, tuple([reference(), pid()])}]}, - {:erlang, :tuple_size, [{[open_tuple([])], integer()}]}, - {:erlang, :trunc, [{[union(integer(), float())], integer()}]}, - - # TODO: Replace term()/dynamic() by parametric types - {:erlang, :++, [{[list(term()), term()], dynamic(list(term(), term()))}]}, - {:erlang, :--, [{[list(term()), list(term())], dynamic(list(term()))}]}, - {:erlang, :andalso, [{[boolean(), term()], dynamic()}]}, - {:erlang, :delete_element, [{[integer(), open_tuple([])], dynamic(open_tuple([]))}]}, - {:erlang, :hd, [{[non_empty_list(term(), term())], dynamic()}]}, - {:erlang, :element, [{[integer(), open_tuple([])], dynamic()}]}, - {:erlang, :insert_element, - [{[integer(), open_tuple([]), term()], dynamic(open_tuple([]))}]}, - {:erlang, :max, [{[term(), term()], dynamic()}]}, - {:erlang, :min, [{[term(), term()], dynamic()}]}, - {:erlang, :orelse, [{[boolean(), term()], dynamic()}]}, - {:erlang, :send, [{[send_destination, term()], dynamic()}]}, - {:erlang, :setelement, [{[integer(), open_tuple([]), term()], dynamic(open_tuple([]))}]}, - {:erlang, :tl, [{[non_empty_list(term(), term())], dynamic()}]}, - {:erlang, :tuple_to_list, [{[open_tuple([])], dynamic(list(term()))}]} - ] do - [arity] = Enum.map(clauses, fn {args, _return} -> length(args) end) |> Enum.uniq() - - true = - Code.ensure_loaded?(mod) and - (function_exported?(mod, fun, arity) or fun in [:orelse, :andalso]) - - domain_clauses = - case clauses do - [_] -> - {:strong, nil, clauses} - - _ -> - domain = - clauses - |> Enum.map(fn {args, _} -> args end) - |> Enum.zip_with(fn types -> Enum.reduce(types, &union/2) end) - - {:strong, domain, clauses} - end - - defp remote(unquote(mod), unquote(fun), unquote(arity)), - do: unquote(Macro.escape(domain_clauses)) - end - - defp remote(_mod, _fun, _arity), do: :none - - @doc """ - Checks a module is a valid remote. - - It returns either a tuple with the remote information and the context. - The remote information may be one of: - - * `:none` - no typing information found. - - * `{:infer, clauses}` - clauses from inferences. You must check all - all clauses and return the union between them. They are dynamic - and they can only be converted into arrows by computing the union - of all arguments. - - * `{:strong, domain or nil, clauses}` - clauses from signatures. So far - these are strong arrows with non-overlapping domains - - """ - def remote(module, fun, arity, meta, stack, context) when is_atom(module) do - if Keyword.get(meta, :runtime_module, false) do - {:none, context} - else - case remote(module, fun, arity) do - :none -> export(module, fun, arity, meta, stack, context) - clauses -> {clauses, context} - end - end - end - - @doc """ - Applies a function in unknown modules. - - Used only by info functions. - """ - def apply(name, args_types, expr, stack, context) do - arity = length(args_types) - - case remote(name, arity) do - :none -> {dynamic(), context} - info -> apply_remote(info, args_types, expr, stack, context) - end - end - - @doc """ - Applies a function in a given module. - """ - def apply(:erlang, :element, [_, tuple], {_, meta, [index, _]} = expr, stack, context) - when is_integer(index) do - case tuple_fetch(tuple, index - 1) do - {_optional?, value_type} -> - {value_type, context} - - :badtuple -> - {error_type(), to_badapply_error(expr, [integer(), tuple], stack, context)} - - reason -> - {error_type(), error({reason, expr, tuple, index - 1, context}, meta, stack, context)} - end - end - - def apply( - :erlang, - :insert_element, - [_, tuple, value], - {_, meta, [index, _, _]} = expr, - stack, - context - ) - when is_integer(index) do - case tuple_insert_at(tuple, index - 1, value) do - value_type when is_descr(value_type) -> - {value_type, context} - - :badtuple -> - {error_type(), to_badapply_error(expr, [integer(), tuple, value], stack, context)} - - reason -> - {error_type(), error({reason, expr, tuple, index - 2, context}, meta, stack, context)} - end - end - - def apply(:erlang, :delete_element, [_, tuple], {_, meta, [index, _]} = expr, stack, context) - when is_integer(index) do - case tuple_delete_at(tuple, index - 1) do - value_type when is_descr(value_type) -> - {value_type, context} - - :badtuple -> - {error_type(), to_badapply_error(expr, [integer(), tuple], stack, context)} - - reason -> - {error_type(), error({reason, expr, tuple, index - 1, context}, meta, stack, context)} - end - end - - def apply(:erlang, :make_tuple, [_, elem], {_, _meta, [size, _]}, _stack, context) - when is_integer(size) and size >= 0 do - {tuple(List.duplicate(elem, size)), context} - end - - def apply(:erlang, :hd, [list], expr, stack, context) do - case list_hd(list) do - {_, value_type} -> - {value_type, context} - - :badnonemptylist -> - {error_type(), to_badapply_error(expr, [list], stack, context)} - end - end - - def apply(:erlang, :tl, [list], expr, stack, context) do - case list_tl(list) do - {_, value_type} -> - {value_type, context} - - :badnonemptylist -> - {error_type(), to_badapply_error(expr, [list], stack, context)} - end - end - - def apply(:erlang, name, [left, right] = args_types, expr, stack, context) - when name in [:>=, :"=<", :>, :<, :min, :max] do - context = - cond do - stack.mode == :infer -> - context - - match?({false, _}, map_fetch(left, :__struct__)) or - match?({false, _}, map_fetch(right, :__struct__)) -> - warning = {:struct_comparison, expr, context} - warn(__MODULE__, warning, elem(expr, 1), stack, context) - - number_type?(left) and number_type?(right) -> - context - - disjoint?(left, right) -> - warning = {:mismatched_comparison, expr, context} - warn(__MODULE__, warning, elem(expr, 1), stack, context) - - true -> - context - end - - if name in [:min, :max] do - {union(left, right), context} - else - {remote_return(boolean(), args_types, stack), context} - end - end - - def apply(:erlang, name, [left, right] = args_types, expr, stack, context) - when name in [:==, :"/=", :"=:=", :"=/="] do - context = - cond do - stack.mode == :infer -> - context - - name in [:==, :"/="] and number_type?(left) and number_type?(right) -> - context - - disjoint?(left, right) -> - warning = {:mismatched_comparison, expr, context} - warn(__MODULE__, warning, elem(expr, 1), stack, context) - - true -> - context - end - - {remote_return(boolean(), args_types, stack), context} - end - - def apply(mod, name, args_types, expr, stack, context) do - arity = length(args_types) - - case :elixir_rewrite.inline(mod, name, arity) do - {mod, name} -> - apply(mod, name, args_types, expr, stack, context) - - false -> - {info, context} = remote(mod, name, arity, elem(expr, 1), stack, context) - apply_remote(info, args_types, expr, stack, context) - end - end - - defp remote_return(type, args_types, stack) do - cond do - stack.mode == :static -> type - Enum.any?(args_types, &gradual?/1) -> dynamic(type) - true -> type - end - end - - defp apply_remote(info, args_types, expr, stack, context) do - case apply_remote(info, args_types, stack) do - {:ok, type} -> - {type, context} - - {:error, domain, clauses} -> - error = {:badapply, expr, args_types, domain, clauses, context} - {error_type(), error(error, elem(expr, 1), stack, context)} - end - end - - defp apply_remote(:none, _args_types, _stack) do - {:ok, dynamic()} - end - - defp apply_remote({:strong, nil, [{expected, return}] = clauses}, args_types, stack) do - # Optimize single clauses as the domain is the single clause args. - case zip_compatible?(args_types, expected) do - true -> {:ok, remote_return(return, args_types, stack)} - false -> {:error, expected, clauses} - end - end - - defp apply_remote({:strong, domain, clauses}, args_types, stack) do - # If the type is only gradual, the compatibility check is the same - # as a non disjoint check. So we skip checking compatibility twice. - with true <- zip_compatible_or_only_gradual?(args_types, domain), - [_ | _] = returns <- - for({expected, return} <- clauses, zip_not_disjoint?(args_types, expected), do: return) do - {:ok, returns |> Enum.reduce(&union/2) |> remote_return(args_types, stack)} - else - _ -> {:error, domain, clauses} - end - end - - defp apply_remote({:infer, clauses}, args_types, _stack) do - case for({expected, return} <- clauses, zip_not_disjoint?(args_types, expected), do: return) do - [] -> - domain = - clauses - |> Enum.map(fn {args, _} -> args end) - |> Enum.zip_with(fn types -> Enum.reduce(types, &union/2) end) - - {:error, domain, clauses} - - returns -> - {:ok, returns |> Enum.reduce(&union/2) |> dynamic()} - end - end - - defp zip_compatible_or_only_gradual?([actual | actuals], [expected | expecteds]) do - (only_gradual?(actual) or compatible?(actual, expected)) and - zip_compatible_or_only_gradual?(actuals, expecteds) - end - - defp zip_compatible_or_only_gradual?([], []), do: true - - defp zip_compatible?([actual | actuals], [expected | expecteds]) do - compatible?(actual, expected) and zip_compatible?(actuals, expecteds) - end - - defp zip_compatible?([], []), do: true - - defp zip_not_disjoint?([actual | actuals], [expected | expecteds]) do - not disjoint?(actual, expected) and zip_not_disjoint?(actuals, expecteds) - end - - defp zip_not_disjoint?([], []), do: true - - defp export(_module, :module_info, arity, _meta, _stack, context) when arity in [0, 1] do - {remote(:module_info, arity), context} - end - - defp export(_module, _fun, _arity, _meta, %{cache: %Macro.Env{}}, context) do - {:none, context} - end - - defp export(module, fun, arity, meta, stack, context) do - case ParallelChecker.fetch_export(stack.cache, module, fun, arity) do - {:ok, mode, reason, info} -> - info = if info == :none, do: remote(fun, arity), else: info - {info, check_deprecated(mode, module, fun, arity, reason, meta, stack, context)} - - {:error, type} -> - context = - if warn_undefined?(module, fun, arity, stack) do - warn(__MODULE__, {:undefined, type, module, fun, arity}, meta, stack, context) - else - context - end - - {:none, context} - end - end - - defp check_deprecated(:elixir, module, fun, arity, reason, meta, stack, context) do - if reason do - warn(__MODULE__, {:deprecated, module, fun, arity, reason}, meta, stack, context) - else - context - end - end - - defp check_deprecated(:erlang, module, fun, arity, _reason, meta, stack, context) do - case :otp_internal.obsolete(module, fun, arity) do - {:deprecated, string} when is_list(string) -> - reason = string |> List.to_string() |> :string.titlecase() - warn(__MODULE__, {:deprecated, module, fun, arity, reason}, meta, stack, context) - - {:deprecated, string, removal} when is_list(string) and is_list(removal) -> - reason = string |> List.to_string() |> :string.titlecase() - reason = "It will be removed in #{removal}. #{reason}" - warn(__MODULE__, {:deprecated, module, fun, arity, reason}, meta, stack, context) - - _ -> - context - end - end - - defp warn_undefined?(_, _, _, %{no_warn_undefined: :all}) do - false - end - - defp warn_undefined?(module, fun, arity, stack) do - not Enum.any?(stack.no_warn_undefined, &(&1 == module or &1 == {module, fun, arity})) - end - ## Warning helpers @doc """ @@ -987,7 +424,7 @@ defmodule Module.Types.Of do } end - def format_diagnostic({:badmap, expr, type, key, context}) do + def format_diagnostic({:badmodule, expr, type, fun, arity, hints, context}) do traces = collect_traces(expr, context) %{ @@ -995,7 +432,7 @@ defmodule Module.Types.Of do message: IO.iodata_to_binary([ """ - expected a map or struct when accessing .#{key} in expression: + expected a module (an atom) when invoking #{fun}/#{arity} in expression: #{expr_to_string(expr) |> indent(4)} """, @@ -1006,308 +443,64 @@ defmodule Module.Types.Of do #{to_quoted_string(type) |> indent(4)} """), format_traces(traces), - format_hints([:dot]) + format_hints(hints) ]) } end - def format_diagnostic({:badkey, expr, type, key, context}) do + def format_diagnostic({:badmap, expr, type, key, context}) do traces = collect_traces(expr, context) %{ details: %{typing_traces: traces}, - span: expr |> get_meta() |> :elixir_env.calculate_span(key) |> Keyword.get(:span), message: IO.iodata_to_binary([ """ - unknown key .#{key} in expression: + expected a map or struct when accessing .#{key} in expression: #{expr_to_string(expr) |> indent(4)} """, empty_if(dot_var?(expr), """ - the given type does not have the given key: + but got type: #{to_quoted_string(type) |> indent(4)} """), - format_traces(traces) - ]) - } - end - - def format_diagnostic({:badindex, expr, type, index, context}) do - traces = collect_traces(expr, context) - - %{ - details: %{typing_traces: traces}, - message: - IO.iodata_to_binary([ - """ - expected a tuple with at least #{pluralize(index + 1, "element", "elements")} in #{format_mfa(expr)}: - - #{expr_to_string(expr) |> indent(4)} - - the given type does not have the given index: - - #{to_quoted_string(type) |> indent(4)} - """, - format_traces(traces) + format_traces(traces), + format_hints([:dot]) ]) } end - def format_diagnostic({:badmodule, expr, type, fun, arity, hints, context}) do + def format_diagnostic({:badkey, expr, type, key, context}) do traces = collect_traces(expr, context) %{ details: %{typing_traces: traces}, + span: expr |> get_meta() |> :elixir_env.calculate_span(key) |> Keyword.get(:span), message: IO.iodata_to_binary([ """ - expected a module (an atom) when invoking #{fun}/#{arity} in expression: + unknown key .#{key} in expression: #{expr_to_string(expr) |> indent(4)} """, empty_if(dot_var?(expr), """ - but got type: + the given type does not have the given key: #{to_quoted_string(type) |> indent(4)} """), - format_traces(traces), - format_hints(hints) - ]) - } - end - - def format_diagnostic({:badapply, expr, args_types, domain, clauses, context}) do - traces = collect_traces(expr, context) - {{:., _, [mod, fun]}, _, args} = expr - {mod, fun, args, converter} = :elixir_rewrite.erl_to_ex(mod, fun, args) - - explanation = - if i = Enum.find_index(args_types, &empty?/1) do - """ - the #{integer_to_ordinal(i + 1)} argument is empty (often represented as none()), \ - most likely because it is the result of an expression that always fails, such as \ - a `raise` or a previous invalid call. This causes any function called with this \ - value to fail - """ - else - """ - but expected one of: - #{clauses_args_to_quoted_string(clauses, converter)} - """ - end - - %{ - details: %{typing_traces: traces}, - message: - IO.iodata_to_binary([ - """ - incompatible types given to #{Exception.format_mfa(mod, fun, length(args))}: - - #{expr_to_string(expr) |> indent(4)} - - given types: - - #{args_to_quoted_string(args_types, domain, converter) |> indent(4)} - - """, - explanation, format_traces(traces) ]) } end - def format_diagnostic({:mismatched_comparison, expr, context}) do - traces = collect_traces(expr, context) - - %{ - details: %{typing_traces: traces}, - message: - IO.iodata_to_binary([ - """ - comparison between distinct types found: - - #{expr_to_string(expr) |> indent(4)} - """, - format_traces(traces), - """ - - While Elixir can compare across all types, you are comparing \ - across types which are always disjoint, and the result is either \ - always true or always false - """ - ]) - } - end - - def format_diagnostic({:struct_comparison, expr, context}) do - traces = collect_traces(expr, context) - - %{ - details: %{typing_traces: traces}, - message: - IO.iodata_to_binary([ - """ - comparison with structs found: - - #{expr_to_string(expr) |> indent(4)} - """, - format_traces(traces), - """ - - Comparison operators (>, <, >=, <=, min, and max) perform structural \ - and not semantic comparison. Comparing with a struct won't give meaningful \ - results. Structs that can be compared typically define a compare/2 function \ - within their modules that can be used for semantic comparison. - """ - ]) - } - end - - def format_diagnostic({:undefined, :module, module, fun, arity}) do - top = - if fun == :__struct__ and arity == 0 do - "struct #{inspect(module)}" - else - Exception.format_mfa(module, fun, arity) - end - - %{ - message: - IO.iodata_to_binary([ - top, - " is undefined (module ", - inspect(module), - " is not available or is yet to be defined)", - UndefinedFunctionError.hint_for_missing_module(module, fun, arity) - ]), - group: true - } - end - - def format_diagnostic({:undefined, :function, module, :__struct__, 0}) do - %{ - message: - "struct #{inspect(module)} is undefined (there is such module but it does not define a struct)", - group: true - } - end - - def format_diagnostic({:undefined, :function, module, fun, arity}) do - %{ - message: - IO.iodata_to_binary([ - Exception.format_mfa(module, fun, arity), - " is undefined or private", - UndefinedFunctionError.hint_for_loaded_module(module, fun, arity) - ]), - group: true - } - end - - def format_diagnostic({:deprecated, module, fun, arity, reason}) do - %{ - message: - IO.iodata_to_binary([ - Exception.format_mfa(module, fun, arity), - " is deprecated. ", - reason - ]), - group: true - } - end - - defp pluralize(1, singular, _), do: "1 #{singular}" - defp pluralize(i, _, plural), do: "#{i} #{plural}" - defp dot_var?(expr) do match?({{:., _, [var, _fun]}, _, _args} when is_var(var), expr) end - defp to_badapply_error({{:., _, [mod, fun]}, meta, _} = expr, args_types, stack, context) do - {_type, domain, [{args, _} | _] = clauses} = remote(mod, fun, length(args_types)) - error({:badapply, expr, args_types, domain || args, clauses, context}, meta, stack, context) - end - defp empty_if(condition, content) do if condition, do: "", else: content end - - defp format_mfa({{:., _, [mod, fun]}, _, args}) do - {mod, fun, args, _} = :elixir_rewrite.erl_to_ex(mod, fun, args) - Exception.format_mfa(mod, fun, length(args)) - end - - ## Algebra helpers - - alias Inspect.Algebra, as: IA - - defp clauses_args_to_quoted_string([{args, _return}], converter) do - "\n " <> (clause_args_to_quoted_string(args, converter) |> indent(4)) - end - - defp clauses_args_to_quoted_string(clauses, converter) do - clauses - |> Enum.with_index(fn {args, _return}, index -> - """ - - ##{index + 1} - #{clause_args_to_quoted_string(args, converter)}\ - """ - |> indent(4) - end) - |> Enum.join("\n") - end - - defp clause_args_to_quoted_string(args, converter) do - docs = Enum.map(args, &(&1 |> to_quoted() |> Code.Formatter.to_algebra())) - args_docs_to_quoted_string(converter.(docs)) - end - - defp args_to_quoted_string(args_types, domain, converter) do - ansi? = IO.ANSI.enabled?() - - docs = - Enum.zip_with(args_types, domain, fn actual, expected -> - doc = actual |> to_quoted() |> Code.Formatter.to_algebra() - - cond do - compatible?(actual, expected) -> doc - ansi? -> IA.concat(IA.color(doc, IO.ANSI.red()), IA.color(IA.empty(), IO.ANSI.reset())) - true -> IA.concat(["-", doc, "-"]) - end - end) - - args_docs_to_quoted_string(converter.(docs)) - end - - defp args_docs_to_quoted_string(docs) do - doc = IA.fold(docs, fn doc, acc -> IA.glue(IA.concat(doc, ","), acc) end) - - wrapped_docs = - case docs do - [_] -> IA.concat("(", IA.concat(doc, ")")) - _ -> IA.group(IA.glue(IA.nest(IA.glue("(", "", doc), 2), "", ")")) - end - - wrapped_docs - |> IA.format(98) - |> IO.iodata_to_binary() - |> case do - "(\n" <> _ = multiple_lines -> multiple_lines - single_line -> binary_slice(single_line, 1..-2//1) - end - end - - defp integer_to_ordinal(i) do - case rem(i, 10) do - 1 when rem(i, 100) != 11 -> "#{i}st" - 2 when rem(i, 100) != 12 -> "#{i}nd" - 3 when rem(i, 100) != 13 -> "#{i}rd" - _ -> "#{i}th" - end - end end diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex index 838461e6e2b..1f824a4e46e 100644 --- a/lib/elixir/lib/module/types/pattern.ex +++ b/lib/elixir/lib/module/types/pattern.ex @@ -715,7 +715,7 @@ defmodule Module.Types.Pattern do {args_type, context} = Enum.map_reduce(args, context, &of_guard(&1, dynamic(), expr, stack, &2)) - Of.apply(:erlang, function, args_type, call, stack, context) + Module.Types.Apply.remote(:erlang, function, args_type, call, stack, context) end # var diff --git a/lib/elixir/src/elixir_compiler.erl b/lib/elixir/src/elixir_compiler.erl index 5a12007ade6..0ee5e89a50a 100644 --- a/lib/elixir/src/elixir_compiler.erl +++ b/lib/elixir/src/elixir_compiler.erl @@ -198,6 +198,7 @@ bootstrap_files() -> <<"module/types/descr.ex">>, <<"module/types/of.ex">>, <<"module/types/pattern.ex">>, + <<"module/types/apply.ex">>, <<"module/types/expr.ex">>, <<"module/types.ex">>, <<"exception.ex">>, diff --git a/lib/elixir/test/elixir/module/types/type_helper.exs b/lib/elixir/test/elixir/module/types/type_helper.exs index 19c29b82d30..69e890fc7b9 100644 --- a/lib/elixir/test/elixir/module/types/type_helper.exs +++ b/lib/elixir/test/elixir/module/types/type_helper.exs @@ -142,7 +142,7 @@ defmodule TypeHelper do end defp new_context() do - Types.context() + Types.context({fn _fun_arity, :ok -> raise "no local lookup" end, :ok}) end @doc """ From ad37f609c57329aefb83809a36ee1f542260fc25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 14:43:25 +0100 Subject: [PATCH 03/22] Local inference works --- lib/elixir/lib/module/types.ex | 120 +++++++----------- lib/elixir/lib/module/types/apply.ex | 16 ++- lib/elixir/lib/module/types/expr.ex | 12 +- .../test/elixir/module/types/infer_test.exs | 21 +++ .../test/elixir/module/types/type_helper.exs | 5 +- 5 files changed, 92 insertions(+), 82 deletions(-) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 9efe13c31eb..af1c733f8e5 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -3,6 +3,7 @@ defmodule Module.Types do alias Module.Types.{Descr, Expr, Pattern} + # TODO: Local captures # TODO: Inference of tail recursion # TODO: Checking of unused private functions/clauses @@ -12,90 +13,52 @@ defmodule Module.Types do @doc false def infer(module, file, defs, env) do finder = &List.keyfind(defs, &1, 0) - handler = &infer_signature_for(&1, &2, module, file, finder, env) - context = context({handler, %{}}) + handler = &local_handler(&1, &2, &3, finder) + stack = stack(:infer, file, module, {:__info__, 1}, :all, env, handler) + context = context(%{}) {types, _context} = for {fun_arity, kind, _meta, _clauses} = def <- defs, kind == :def and fun_arity not in @no_infer, reduce: {[], context} do {types, context} -> - {_kind, inferred, context} = - infer_signature_for(fun_arity, context, module, file, fn _ -> def end, env) - + {_kind, inferred, context} = local_handler(fun_arity, stack, context, fn _ -> def end) {[{fun_arity, inferred} | types], context} end Map.new(types) end - defp infer_signature_for(fun_arity, context, module, file, finder, env) do - case context.local_handler do - {_, %{^fun_arity => {kind, inferred}}} -> - {kind, inferred, context} - - {_, _} -> - {{fun, arity}, kind, _meta, clauses} = finder.(fun_arity) - expected = List.duplicate(Descr.dynamic(), arity) - - stack = stack(:infer, file, module, fun_arity, :all, env) - context = update_local_state(context, &Map.put(&1, fun_arity, {kind, :none})) - - {pair_types, context} = - Enum.reduce(clauses, {[], context}, fn - {meta, args, guards, body}, {inferred, context} -> - context = context(context.local_handler) - - try do - {args_types, context} = - Pattern.of_head(args, guards, expected, :default, meta, stack, context) - - {return_type, context} = Expr.of_expr(body, stack, context) - {add_inferred(inferred, args_types, return_type, []), context} - rescue - e -> - internal_error!(e, __STACKTRACE__, kind, meta, module, fun, args, guards, body) - end - end) - - inferred = {:infer, Enum.reverse(pair_types)} - {kind, inferred, update_local_state(context, &Map.put(&1, fun_arity, {kind, inferred}))} - end - end - @doc false def warnings(module, file, defs, no_warn_undefined, cache) do finder = &List.keyfind(defs, &1, 0) - handler = &warnings_for(&1, &2, module, file, finder, no_warn_undefined, cache) - context = context({handler, %{}}) + handler = &local_handler(&1, &2, &3, finder) + stack = stack(:dynamic, file, module, {:__info__, 1}, no_warn_undefined, cache, handler) + context = context(%{}) context = Enum.reduce(defs, context, fn {fun_arity, _kind, _meta, _clauses} = def, context -> - finder = fn _ -> def end - - {_kind, _inferred, context} = - warnings_for(fun_arity, context, module, file, finder, no_warn_undefined, cache) - + {_kind, _inferred, context} = local_handler(fun_arity, stack, context, fn _ -> def end) context end) context.warnings end - defp warnings_for(fun_arity, context, module, file, finder, no_warn_undefined, cache) do - case context.local_handler do - {_, %{^fun_arity => {kind, inferred}}} -> + defp local_handler(fun_arity, stack, context, finder) do + case context.local_state do + %{^fun_arity => {kind, inferred}} -> {kind, inferred, context} - {_, _} -> - {{fun, arity}, kind, meta, clauses} = finder.(fun_arity) - expected = List.duplicate(Descr.dynamic(), arity) + local_state -> + {{fun, arity}, kind, meta, clauses} = + finder.(fun_arity) || raise "could not find #{inspect(fun_arity)}" - file = with_file_meta(meta, file) - stack = stack(:dynamic, file, module, fun_arity, no_warn_undefined, cache) - context = update_local_state(context, &Map.put(&1, fun_arity, {kind, :none})) + expected = List.duplicate(Descr.dynamic(), arity) + stack = stack |> fresh_stack(fun_arity) |> with_file_meta(meta) + context = put_in(context.local_state, Map.put(local_state, fun_arity, {kind, :none})) - {pair_types, context} = + {clauses_types, clauses_context} = Enum.reduce(clauses, {[], context}, fn {meta, args, guards, body}, {inferred, context} -> context = fresh_context(context) @@ -108,12 +71,13 @@ defmodule Module.Types do {add_inferred(inferred, args_types, return_type, []), context} rescue e -> - internal_error!(e, __STACKTRACE__, kind, meta, module, fun, args, guards, body) + internal_error!(e, __STACKTRACE__, kind, meta, fun, args, guards, body, stack) end end) - inferred = {:infer, Enum.reverse(pair_types)} - {kind, inferred, update_local_state(context, &Map.put(&1, fun_arity, {kind, inferred}))} + inferred = {:infer, Enum.reverse(clauses_types)} + context = update_in(context.local_state, &Map.put(&1, fun_arity, {kind, inferred})) + {kind, inferred, restore_context(context, clauses_context)} end end @@ -128,19 +92,19 @@ defmodule Module.Types do defp add_inferred([], args, return, acc), do: [{args, return} | Enum.reverse(acc)] - defp with_file_meta(meta, file) do + defp with_file_meta(stack, meta) do case Keyword.fetch(meta, :file) do - {:ok, {meta_file, _}} -> meta_file - :error -> file + {:ok, {meta_file, _}} -> %{stack | file: meta_file} + :error -> stack end end - defp internal_error!(e, stack, kind, meta, module, fun, args, guards, body) do + defp internal_error!(e, trace, kind, meta, fun, args, guards, body, stack) do def_expr = {kind, meta, [guards_to_expr(guards, {fun, [], args}), [do: body]]} exception = RuntimeError.exception(""" - found error while checking types for #{Exception.format_mfa(module, fun, length(args))}: + found error while checking types for #{Exception.format_mfa(stack.module, fun, length(args))}: #{Exception.format_banner(:error, e, stack)}\ @@ -151,7 +115,7 @@ defmodule Module.Types do Please report this bug at: https://github.com/elixir-lang/elixir/issues """) - reraise exception, stack + reraise exception, trace end defp guards_to_expr([], left) do @@ -163,7 +127,7 @@ defmodule Module.Types do end @doc false - def stack(mode, file, module, function, no_warn_undefined, cache) + def stack(mode, file, module, function, no_warn_undefined, cache, handler) when mode in [:static, :dynamic, :infer] do %{ # The fallback meta used for literals in patterns and guards @@ -200,31 +164,37 @@ defmodule Module.Types do # We may also want for applications with subtyping in dynamic mode to always # intersect with dynamic, but this mode may be too lax (to be decided based on # feedback). - mode: mode + mode: mode, + # The function for handling local calls + local_handler: handler } end @doc false - def context(local_handler, warnings \\ []) do + def context(local_state) do %{ # A list of all warnings found so far - warnings: warnings, + warnings: [], # All vars and their types vars: %{}, # Variables and arguments from patterns pattern_info: nil, # If type checking has found an error/failure failed: false, - # Local handler - local_handler: local_handler + # Local state + local_state: local_state } end - defp fresh_context(%{local_handler: local_handler, warnings: warnings}) do - context(local_handler, warnings) + defp fresh_stack(stack, function) do + %{stack | function: function} + end + + defp fresh_context(context) do + %{context | vars: %{}, failed: false} end - defp update_local_state(%{local_handler: {handler, state}} = context, fun) do - %{context | local_handler: {handler, fun.(state)}} + defp restore_context(%{vars: vars, failed: failed}, later_context) do + %{later_context | vars: vars, failed: failed} end end diff --git a/lib/elixir/lib/module/types/apply.ex b/lib/elixir/lib/module/types/apply.ex index 9296dbec896..c6a13f6faba 100644 --- a/lib/elixir/lib/module/types/apply.ex +++ b/lib/elixir/lib/module/types/apply.ex @@ -504,8 +504,20 @@ defmodule Module.Types.Apply do ## Local - def local(_fun, _args_types, _stack, context) do - {dynamic(), context} + def local(fun, args_types, meta, stack, context) do + arity = length(args_types) + + # TODO: If kind == :defp and stack != :infer, we want to track used clauses + {_kind, info, context} = stack.local_handler.({fun, arity}, stack, context) + + case apply_types(info, args_types, stack) do + {:ok, type} -> + {type, context} + + {:error, domain, clauses} -> + error = {:badlocal, fun, args_types, domain, clauses, context} + {error_type(), error(error, meta, stack, context)} + end end ## Application helpers diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex index ca415c2c279..67264fc9765 100644 --- a/lib/elixir/lib/module/types/expr.ex +++ b/lib/elixir/lib/module/types/expr.ex @@ -396,17 +396,23 @@ defmodule Module.Types.Expr do {fun(), context} end - # &foo/1 + # TODO: &foo/1 # TODO: & &1 def of_expr({:&, _meta, _arg}, _stack, context) do {fun(), context} end + # TODO: super + def of_expr({:super, _meta, args}, stack, context) when is_list(args) do + {_args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) + {dynamic(), context} + end + # Local calls - def of_expr({fun, _meta, args}, stack, context) + def of_expr({fun, meta, args}, stack, context) when is_atom(fun) and is_list(args) do {args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) - Apply.local(fun, args_types, stack, context) + Apply.local(fun, args_types, meta, stack, context) end # var diff --git a/lib/elixir/test/elixir/module/types/infer_test.exs b/lib/elixir/test/elixir/module/types/infer_test.exs index ce7c51f2dc8..975929273a6 100644 --- a/lib/elixir/test/elixir/module/types/infer_test.exs +++ b/lib/elixir/test/elixir/module/types/infer_test.exs @@ -61,4 +61,25 @@ defmodule Module.Types.InferTest do {[dynamic(atom([:error]))], atom([:five])} ]} end + + test "infers return types from private functions", config do + types = + infer config do + def pub(x), do: priv(x) + defp priv(:ok), do: :ok + defp priv(:error), do: :error + end + + assert types[{:pub, 1}] == {:infer, [{[dynamic()], dynamic(atom([:ok, :error]))}]} + assert types[{:priv, 1}] == nil + end + + test "infers return types even with loops", config do + types = + infer config do + def pub(x), do: pub(x) + end + + assert types[{:pub, 1}] == {:infer, [{[dynamic()], dynamic()}]} + end end diff --git a/lib/elixir/test/elixir/module/types/type_helper.exs b/lib/elixir/test/elixir/module/types/type_helper.exs index 69e890fc7b9..a852be13295 100644 --- a/lib/elixir/test/elixir/module/types/type_helper.exs +++ b/lib/elixir/test/elixir/module/types/type_helper.exs @@ -138,11 +138,12 @@ defmodule TypeHelper do defp new_stack(mode) do cache = if mode == :infer, do: :none, else: Module.ParallelChecker.test_cache() - Types.stack(mode, "types_test.ex", TypesTest, {:test, 0}, [], cache) + handler = fn _, _, _ -> raise "no local lookup" end + Types.stack(mode, "types_test.ex", TypesTest, {:test, 0}, [], cache, handler) end defp new_context() do - Types.context({fn _fun_arity, :ok -> raise "no local lookup" end, :ok}) + Types.context(%{}) end @doc """ From 5e56edf99c456ac5a9d05c1e3cb7eabd711c486f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 17:01:53 +0100 Subject: [PATCH 04/22] Optimize handling of type refinement --- lib/elixir/lib/module/types/pattern.ex | 32 ++++++++++++-------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex index 1f824a4e46e..1309eeb9651 100644 --- a/lib/elixir/lib/module/types/pattern.ex +++ b/lib/elixir/lib/module/types/pattern.ex @@ -134,12 +134,8 @@ defmodule Module.Types.Pattern do end defp of_pattern_recur(types, vars, info, tag, stack, context, callback) do - %{vars: context_vars} = context - {changed, context} = Enum.reduce(vars, {[], context}, fn {version, paths}, {changed, context} -> - current_type = context_vars[version][:type] - {var_changed?, context} = Enum.reduce(paths, {false, context}, fn [var, {:arg, index, expr} | path], {var_changed?, context} -> @@ -147,18 +143,20 @@ defmodule Module.Types.Pattern do case of_pattern_var(path, actual, true, info, context) do {type, reachable_var?} -> - case Of.refine_var(var, type, expr, stack, context) do - {:ok, type, context} -> - {var_changed? or - (reachable_var? and - (current_type == nil or not equal?(current_type, type))), context} - - {:error, _type, context} -> - throw({types, context}) + # If current type is already a subtype, there is nothing to refine. + with %{^version => %{type: current_type}} <- context.vars, + true <- subtype?(current_type, type) do + {var_changed?, context} + else + _ -> + case Of.refine_var(var, type, expr, stack, context) do + {:ok, _type, context} -> {var_changed? or reachable_var?, context} + {:error, _type, context} -> throw({types, context}) + end end :error -> - throw({types, to_badpattern_error(expr, tag, stack, context)}) + throw({types, badpattern_error(expr, tag, stack, context)}) end end) @@ -208,7 +206,7 @@ defmodule Module.Types.Pattern do end) end - defp to_badpattern_error(expr, tag, stack, context) do + defp badpattern_error(expr, tag, stack, context) do meta = if meta = get_meta(expr) do meta ++ Keyword.take(stack.meta, [:generated, :line]) @@ -224,7 +222,7 @@ defmodule Module.Types.Pattern do type = intersection(actual, expected) if empty?(type) do - {:error, to_badpattern_error(expr, tag, stack, context)} + {:error, badpattern_error(expr, tag, stack, context)} else {:ok, type, context} end @@ -251,8 +249,8 @@ defmodule Module.Types.Pattern do end # TODO: Implement domain key types - defp of_pattern_var([{:key, _key} | rest], _type, reachable_var?, info, context) do - of_pattern_var(rest, dynamic(), reachable_var?, info, context) + defp of_pattern_var([{:key, _key} | rest], _type, _reachable_var?, info, context) do + of_pattern_var(rest, dynamic(), false, info, context) end defp of_pattern_var([{:head, counter} | rest], type, _reachable_var?, info, context) do From e27e7d9c155cdb8f92f10a3d81e0dfecd3efe791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 17:51:30 +0100 Subject: [PATCH 05/22] Deal with super --- lib/elixir/lib/module/types/expr.ex | 10 +++++----- lib/elixir/test/elixir/module/types/infer_test.exs | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex index 67264fc9765..f2f9fc27cf5 100644 --- a/lib/elixir/lib/module/types/expr.ex +++ b/lib/elixir/lib/module/types/expr.ex @@ -397,15 +397,15 @@ defmodule Module.Types.Expr do end # TODO: &foo/1 - # TODO: & &1 def of_expr({:&, _meta, _arg}, _stack, context) do {fun(), context} end - # TODO: super - def of_expr({:super, _meta, args}, stack, context) when is_list(args) do - {_args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) - {dynamic(), context} + # Super + def of_expr({:super, meta, args}, stack, context) when is_list(args) do + {_kind, fun} = Keyword.fetch!(meta, :super) + {args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) + Apply.local(fun, args_types, meta, stack, context) end # Local calls diff --git a/lib/elixir/test/elixir/module/types/infer_test.exs b/lib/elixir/test/elixir/module/types/infer_test.exs index 975929273a6..106ab0475a8 100644 --- a/lib/elixir/test/elixir/module/types/infer_test.exs +++ b/lib/elixir/test/elixir/module/types/infer_test.exs @@ -74,6 +74,18 @@ defmodule Module.Types.InferTest do assert types[{:priv, 1}] == nil end + test "infers return types from super functions", config do + types = + infer config do + def pub(:ok), do: :ok + def pub(:error), do: :error + defoverridable pub: 1 + def pub(x), do: super(x) + end + + assert types[{:pub, 1}] == {:infer, [{[dynamic()], dynamic(atom([:ok, :error]))}]} + end + test "infers return types even with loops", config do types = infer config do From eaf62c5a62143af0ea10eb6a5fe1ff8b8c28f111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 21:30:53 +0100 Subject: [PATCH 06/22] Check for unused private functions --- lib/elixir/lib/module/types.ex | 87 +++++++--- lib/elixir/lib/module/types/apply.ex | 149 ++++++++++++++---- lib/elixir/lib/module/types/expr.ex | 12 +- lib/elixir/src/elixir_locals.erl | 4 +- .../elixir/module/types/integration_test.exs | 132 +++++++++++----- 5 files changed, 283 insertions(+), 101 deletions(-) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index af1c733f8e5..78e4c2303cc 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -1,11 +1,10 @@ defmodule Module.Types do @moduledoc false - alias Module.Types.{Descr, Expr, Pattern} + alias Module.Types.{Descr, Expr, Pattern, Helpers} - # TODO: Local captures - # TODO: Inference of tail recursion - # TODO: Checking of unused private functions/clauses + # TODO: Consider passing inferred types from infer into warnings + # TODO: Consider removing code from locals tracker # These functions are not inferred because they are added/managed by the compiler @no_infer [__protocol__: 1, behaviour_info: 1] @@ -42,33 +41,61 @@ defmodule Module.Types do context end) + context = + for {fun_arity, pending} <- context.local_used, pending != [], reduce: context do + context -> + {_fun_arity, kind, _meta, clauses} = List.keyfind(defs, fun_arity, 0) + {_kind, _inferred, mapping} = Map.fetch!(context.local_sigs, fun_arity) + + clauses_indexes = + for type_index <- pending, {clause_index, ^type_index} <- mapping, do: clause_index + + Enum.reduce(clauses_indexes, context, fn clause_index, context -> + {meta, _args, _guards, _body} = Enum.fetch!(clauses, clause_index) + stack = %{stack | function: fun_arity} + Helpers.warn(__MODULE__, {:unused_clause, kind, fun_arity}, meta, stack, context) + end) + end + context.warnings end defp local_handler(fun_arity, stack, context, finder) do - case context.local_state do - %{^fun_arity => {kind, inferred}} -> + case context.local_sigs do + %{^fun_arity => {kind, inferred, _mapping}} -> {kind, inferred, context} - local_state -> + %{^fun_arity => kind} when is_atom(kind) -> + {kind, :none, context} + + local_sigs -> {{fun, arity}, kind, meta, clauses} = finder.(fun_arity) || raise "could not find #{inspect(fun_arity)}" expected = List.duplicate(Descr.dynamic(), arity) stack = stack |> fresh_stack(fun_arity) |> with_file_meta(meta) - context = put_in(context.local_state, Map.put(local_state, fun_arity, {kind, :none})) + context = put_in(context.local_sigs, Map.put(local_sigs, fun_arity, kind)) - {clauses_types, clauses_context} = - Enum.reduce(clauses, {[], context}, fn - {meta, args, guards, body}, {inferred, context} -> + {_, _, mapping, clauses_types, clauses_context} = + Enum.reduce(clauses, {0, 0, [], [], context}, fn + {meta, args, guards, body}, {index, total, mapping, inferred, context} -> context = fresh_context(context) try do {args_types, context} = Pattern.of_head(args, guards, expected, :default, meta, stack, context) - {return_type, context} = Expr.of_expr(body, stack, context) - {add_inferred(inferred, args_types, return_type, []), context} + {return_type, context} = + Expr.of_expr(body, stack, context) + + {type_index, inferred} = + add_inferred(inferred, args_types, return_type, total - 1, []) + + if type_index == -1 do + {index + 1, total + 1, [{index, total} | mapping], inferred, context} + else + {index + 1, total, [{index, type_index} | mapping], inferred, context} + end rescue e -> internal_error!(e, __STACKTRACE__, kind, meta, fun, args, guards, body, stack) @@ -76,21 +103,23 @@ defmodule Module.Types do end) inferred = {:infer, Enum.reverse(clauses_types)} - context = update_in(context.local_state, &Map.put(&1, fun_arity, {kind, inferred})) - {kind, inferred, restore_context(context, clauses_context)} + triplet = {kind, inferred, mapping} + context = restore_context(context, clauses_context) + context = update_in(context.local_sigs, &Map.put(&1, fun_arity, triplet)) + {kind, inferred, context} end end # We check for term equality of types as an optimization # to reduce the amount of check we do at runtime. - defp add_inferred([{args, existing_return} | tail], args, return, acc), - do: Enum.reverse(acc, [{args, Descr.union(existing_return, return)} | tail]) + defp add_inferred([{args, existing_return} | tail], args, return, index, acc), + do: {index, Enum.reverse(acc, [{args, Descr.union(existing_return, return)} | tail])} - defp add_inferred([head | tail], args, return, acc), - do: add_inferred(tail, args, return, [head | acc]) + defp add_inferred([head | tail], args, return, index, acc), + do: add_inferred(tail, args, return, index - 1, [head | acc]) - defp add_inferred([], args, return, acc), - do: [{args, return} | Enum.reverse(acc)] + defp add_inferred([], args, return, -1, acc), + do: {-1, [{args, return} | Enum.reverse(acc)]} defp with_file_meta(stack, meta) do case Keyword.fetch(meta, :file) do @@ -171,7 +200,7 @@ defmodule Module.Types do end @doc false - def context(local_state) do + def context(local_sigs) do %{ # A list of all warnings found so far warnings: [], @@ -181,8 +210,10 @@ defmodule Module.Types do pattern_info: nil, # If type checking has found an error/failure failed: false, - # Local state - local_state: local_state + # Local signatures + local_sigs: local_sigs, + # Local clauses + local_used: %{} } end @@ -197,4 +228,12 @@ defmodule Module.Types do defp restore_context(%{vars: vars, failed: failed}, later_context) do %{later_context | vars: vars, failed: failed} end + + ## Diagnostics + + def format_diagnostic({:unused_clause, kind, {fun, arity}}) do + %{ + message: "this clause of #{kind} #{fun}/#{arity} is never used" + } + end end diff --git a/lib/elixir/lib/module/types/apply.ex b/lib/elixir/lib/module/types/apply.ex index c6a13f6faba..ed8c21fd945 100644 --- a/lib/elixir/lib/module/types/apply.ex +++ b/lib/elixir/lib/module/types/apply.ex @@ -2,6 +2,7 @@ defmodule Module.Types.Apply do # Typing functionality shared between Expr and Pattern. # Generic AST and Enum helpers go to Module.Types.Helpers. @moduledoc false + @max_clauses 32 alias Module.ParallelChecker import Module.Types.{Helpers, Descr} @@ -406,8 +407,8 @@ defmodule Module.Types.Apply do end defp apply_remote(info, args_types, expr, stack, context) do - case apply_types(info, args_types, stack) do - {:ok, type} -> + case apply_signature(info, args_types, stack) do + {:ok, _indexes, type} -> {type, context} {:error, domain, clauses} -> @@ -504,19 +505,54 @@ defmodule Module.Types.Apply do ## Local - def local(fun, args_types, meta, stack, context) do - arity = length(args_types) + @doc """ + Deal with local functions. + """ + def local(fun, args_types, expr, stack, context) do + fun_arity = {fun, length(args_types)} + {kind, info, context} = stack.local_handler.(fun_arity, stack, context) - # TODO: If kind == :defp and stack != :infer, we want to track used clauses - {_kind, info, context} = stack.local_handler.({fun, arity}, stack, context) + case apply_signature(info, args_types, stack) do + {:ok, indexes, type} -> + context = + if stack != :infer and kind == :defp do + update_in(context.local_used[fun_arity], fn current -> + if info == :none do + [] + else + (current || used_from_clauses(info)) -- indexes + end + end) + else + context + end - case apply_types(info, args_types, stack) do - {:ok, type} -> {type, context} {:error, domain, clauses} -> - error = {:badlocal, fun, args_types, domain, clauses, context} - {error_type(), error(error, meta, stack, context)} + error = {:badlocal, expr, args_types, domain, clauses, context} + {error_type(), error(error, elem(expr, 1), stack, context)} + end + end + + defp used_from_clauses({:infer, clauses}), + do: Enum.with_index(clauses, fn _, i -> i end) + + defp used_from_clauses({:strong, _, clauses}), + do: Enum.with_index(clauses, fn _, i -> i end) + + @doc """ + Deal with local captures. + """ + def local_capture(fun, arity, _meta, stack, context) do + fun_arity = {fun, arity} + {kind, _info, context} = stack.local_handler.(fun_arity, stack, context) + + if stack != :infer and kind == :defp do + # Mark all clauses as used, as the function is being exported. + {fun(), put_in(context.local_used[fun_arity], [])} + else + {fun(), context} end end @@ -530,33 +566,32 @@ defmodule Module.Types.Apply do end end - defp apply_types(:none, _args_types, _stack) do - {:ok, dynamic()} + defp apply_signature(:none, _args_types, _stack) do + {:ok, [], dynamic()} end - defp apply_types({:strong, nil, [{expected, return}] = clauses}, args_types, stack) do + defp apply_signature({:strong, nil, [{expected, return}] = clauses}, args_types, stack) do # Optimize single clauses as the domain is the single clause args. case zip_compatible?(args_types, expected) do - true -> {:ok, return(return, args_types, stack)} + true -> {:ok, [0], return(return, args_types, stack)} false -> {:error, expected, clauses} end end - defp apply_types({:strong, domain, clauses}, args_types, stack) do + defp apply_signature({:strong, domain, clauses}, args_types, stack) do # If the type is only gradual, the compatibility check is the same # as a non disjoint check. So we skip checking compatibility twice. with true <- zip_compatible_or_only_gradual?(args_types, domain), - [_ | _] = returns <- - for({expected, return} <- clauses, zip_not_disjoint?(args_types, expected), do: return) do - {:ok, returns |> Enum.reduce(&union/2) |> return(args_types, stack)} + {count, used, returns} when count > 0 <- apply_clauses(clauses, args_types, 0, 0, [], []) do + {:ok, used, returns |> Enum.reduce(&union/2) |> return(args_types, stack)} else _ -> {:error, domain, clauses} end end - defp apply_types({:infer, clauses}, args_types, _stack) do - case for({expected, return} <- clauses, zip_not_disjoint?(args_types, expected), do: return) do - [] -> + defp apply_signature({:infer, clauses}, args_types, _stack) do + case apply_clauses(clauses, args_types, 0, 0, [], []) do + {0, [], []} -> domain = clauses |> Enum.map(fn {args, _} -> args end) @@ -564,11 +599,26 @@ defmodule Module.Types.Apply do {:error, domain, clauses} - returns -> - {:ok, returns |> Enum.reduce(&union/2) |> dynamic()} + {count, used, _returns} when count > @max_clauses -> + {:ok, used, dynamic()} + + {_count, used, returns} -> + {:ok, used, returns |> Enum.reduce(&union/2) |> dynamic()} + end + end + + defp apply_clauses([{expected, return} | clauses], args_types, index, count, used, returns) do + if zip_not_disjoint?(args_types, expected) do + apply_clauses(clauses, args_types, index + 1, count + 1, [index | used], [return | returns]) + else + apply_clauses(clauses, args_types, index + 1, count, used, returns) end end + defp apply_clauses([], _args_types, _index, count, used, returns) do + {count, used, returns} + end + defp zip_compatible_or_only_gradual?([actual | actuals], [expected | expecteds]) do (only_gradual?(actual) or compatible?(actual, expected)) and zip_compatible_or_only_gradual?(actuals, expecteds) @@ -622,25 +672,49 @@ defmodule Module.Types.Apply do } end + def format_diagnostic({:badlocal, expr, args_types, domain, clauses, context}) do + traces = collect_traces(expr, context) + converter = &Function.identity/1 + {fun, _, _} = expr + + explanation = + empty_arg_reason(args_types) || + """ + but expected one of: + #{clauses_args_to_quoted_string(clauses, converter)} + """ + + %{ + details: %{typing_traces: traces}, + message: + IO.iodata_to_binary([ + """ + incompatible types given to #{fun}/#{length(args_types)}: + + #{expr_to_string(expr) |> indent(4)} + + given types: + + #{args_to_quoted_string(args_types, domain, converter) |> indent(4)} + + """, + explanation, + format_traces(traces) + ]) + } + end + def format_diagnostic({:badremote, expr, args_types, domain, clauses, context}) do traces = collect_traces(expr, context) {{:., _, [mod, fun]}, _, args} = expr {mod, fun, args, converter} = :elixir_rewrite.erl_to_ex(mod, fun, args) explanation = - if i = Enum.find_index(args_types, &empty?/1) do - """ - the #{integer_to_ordinal(i + 1)} argument is empty (often represented as none()), \ - most likely because it is the result of an expression that always fails, such as \ - a `raise` or a previous invalid call. This causes any function called with this \ - value to fail - """ - else + empty_arg_reason(converter.(args_types)) || """ but expected one of: #{clauses_args_to_quoted_string(clauses, converter)} """ - end %{ details: %{typing_traces: traces}, @@ -762,6 +836,17 @@ defmodule Module.Types.Apply do } end + defp empty_arg_reason(args_types) do + if i = Enum.find_index(args_types, &empty?/1) do + """ + the #{integer_to_ordinal(i + 1)} argument is empty (often represented as none()), \ + most likely because it is the result of an expression that always fails, such as \ + a `raise` or a previous invalid call. This causes any function called with this \ + value to fail + """ + end + end + defp pluralize(1, singular, _), do: "1 #{singular}" defp pluralize(i, _, plural), do: "#{i} #{plural}" diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex index f2f9fc27cf5..fb8d5bacf55 100644 --- a/lib/elixir/lib/module/types/expr.ex +++ b/lib/elixir/lib/module/types/expr.ex @@ -397,22 +397,22 @@ defmodule Module.Types.Expr do end # TODO: &foo/1 - def of_expr({:&, _meta, _arg}, _stack, context) do - {fun(), context} + def of_expr({:&, _meta, [{:/, _, [{fun, meta, _}, arity]}]}, stack, context) do + Apply.local_capture(fun, arity, meta, stack, context) end # Super - def of_expr({:super, meta, args}, stack, context) when is_list(args) do + def of_expr({:super, meta, args} = expr, stack, context) when is_list(args) do {_kind, fun} = Keyword.fetch!(meta, :super) {args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) - Apply.local(fun, args_types, meta, stack, context) + Apply.local(fun, args_types, expr, stack, context) end # Local calls - def of_expr({fun, meta, args}, stack, context) + def of_expr({fun, _meta, args} = expr, stack, context) when is_atom(fun) and is_list(args) do {args_types, context} = Enum.map_reduce(args, context, &of_expr(&1, stack, &2)) - Apply.local(fun, args_types, meta, stack, context) + Apply.local(fun, args_types, expr, stack, context) end # var diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl index 83b779c7171..a9802acdd98 100644 --- a/lib/elixir/src/elixir_locals.erl +++ b/lib/elixir/src/elixir_locals.erl @@ -106,9 +106,9 @@ ensure_no_undefined_local(Module, All, E) -> warn_unused_local(Module, All, Private, E) -> if_tracker(Module, [], fun(Tracker) -> - {Unreachable, Warnings} = ?locals:collect_unused_locals(Tracker, All, Private), + {_Unreachable, Warnings} = ?locals:collect_unused_locals(Tracker, All, Private), [elixir_errors:file_warn(Meta, E, ?MODULE, Error) || {Meta, Error} <- Warnings], - Unreachable + [] end). format_error({function_conflict, {Receiver, {Name, Arity}}}) -> diff --git a/lib/elixir/test/elixir/module/types/integration_test.exs b/lib/elixir/test/elixir/module/types/integration_test.exs index 4c6e1e79a6c..3208cd2cbd0 100644 --- a/lib/elixir/test/elixir/module/types/integration_test.exs +++ b/lib/elixir/test/elixir/module/types/integration_test.exs @@ -51,8 +51,10 @@ defmodule Module.Types.IntegrationTest do {{:behaviour_info, 1}, %{sig: :none}} ] end + end - test "type checks signatures" do + describe "type checking" do + test "inferred remote calls" do files = %{ "a.ex" => """ defmodule A do @@ -101,6 +103,98 @@ defmodule Module.Types.IntegrationTest do assert_warnings(files, warnings) end + + test "mismatched locals" do + files = %{ + "a.ex" => """ + defmodule A do + def error(), do: private(raise "oops") + def public(x), do: private(List.to_tuple(x)) + defp private(:ok), do: nil + end + """ + } + + warnings = [ + """ + warning: incompatible types given to private/1: + + private(raise RuntimeError.exception("oops")) + + """, + "the 1st argument is empty (often represented as none())", + """ + typing violation found at: + │ + 2 │ def error(), do: private(raise "oops") + │ ~ + │ + └─ a.ex:2:20: A.error/0 + """, + """ + warning: incompatible types given to private/1: + + private(List.to_tuple(x)) + """, + """ + typing violation found at: + │ + 3 │ def public(x), do: private(List.to_tuple(x)) + │ ~ + │ + └─ a.ex:3:22: A.public/1 + """ + ] + + assert_warnings(files, warnings) + end + + test "unused private clauses" do + files = %{ + "a.ex" => """ + defmodule A do + def public(x) do + private(List.to_tuple(x)) + end + + defp private(nil), do: nil + defp private("foo"), do: "foo" + defp private({:ok, ok}), do: ok + defp private({:error, error}), do: error + defp private("bar"), do: "bar" + end + """ + } + + warnings = [ + """ + warning: this clause of defp private/1 is never used + │ + 6 │ defp private(nil), do: nil + │ ~ + │ + └─ a.ex:6:8: A.private/1 + """, + """ + warning: this clause of defp private/1 is never used + │ + 7 │ defp private("foo"), do: "foo" + │ ~ + │ + └─ a.ex:7:8: A.private/1 + """, + """ + warning: this clause of defp private/1 is never used + │ + 10 │ defp private("bar"), do: "bar" + │ ~ + │ + └─ a.ex:10:8: A.private/1 + """ + ] + + assert_warnings(files, warnings) + end end describe "undefined warnings" do @@ -345,42 +439,6 @@ defmodule Module.Types.IntegrationTest do assert_warnings(files, warnings) end - test "protocols are checked, ignoring missing built-in impls" do - files = %{ - "a.ex" => """ - defprotocol AProtocol do - def func(arg) - end - - defmodule AImplementation do - defimpl AProtocol do - def func(_), do: B.no_func() - end - end - """ - } - - warnings = [ - "B.no_func/0 is undefined (module B is not available or is yet to be defined)", - "a.ex:7:24: AProtocol.AImplementation.func/1" - ] - - assert_warnings(files, warnings) - end - - test "handles Erlang ops" do - files = %{ - "a.ex" => """ - defmodule A do - def a(a, b), do: a and b - def b(a, b), do: a or b - end - """ - } - - assert_no_warnings(files) - end - test "hints exclude deprecated functions" do files = %{ "a.ex" => """ From 13dde559e849ddc6c909dae96162211e426ecb54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 21:42:49 +0100 Subject: [PATCH 07/22] Fix more tests --- lib/elixir/lib/calendar/time.ex | 11 ++--------- lib/elixir/lib/kernel.ex | 2 +- lib/elixir/lib/set.ex | 2 +- lib/elixir/src/elixir_locals.erl | 4 ++-- lib/elixir/test/elixir/kernel/quote_test.exs | 2 -- 5 files changed, 6 insertions(+), 15 deletions(-) diff --git a/lib/elixir/lib/calendar/time.ex b/lib/elixir/lib/calendar/time.ex index 7bfdb0c6d52..cd1d43576fb 100644 --- a/lib/elixir/lib/calendar/time.ex +++ b/lib/elixir/lib/calendar/time.ex @@ -797,15 +797,8 @@ defmodule Time do @doc since: "1.5.0" @spec convert!(Calendar.time(), Calendar.calendar()) :: t def convert!(time, calendar) do - case convert(time, calendar) do - {:ok, value} -> - value - - {:error, reason} -> - raise ArgumentError, - "cannot convert #{inspect(time)} to target calendar #{inspect(calendar)}, " <> - "reason: #{inspect(reason)}" - end + {:ok, value} = convert(time, calendar) + value end @doc """ diff --git a/lib/elixir/lib/kernel.ex b/lib/elixir/lib/kernel.ex index cbef8391339..0618f97db96 100644 --- a/lib/elixir/lib/kernel.ex +++ b/lib/elixir/lib/kernel.ex @@ -302,7 +302,7 @@ defmodule Kernel do @compile {:inline, bootstrapped?: 1} case :code.ensure_loaded(Kernel) do {:module, _} -> - defp bootstrapped?(_), do: true + defp bootstrapped?(module), do: is_atom(module) {:error, _} -> defp bootstrapped?(module), do: :code.ensure_loaded(module) == {:module, module} diff --git a/lib/elixir/lib/set.ex b/lib/elixir/lib/set.ex index b3c5f3c48a7..74a05ff5bed 100644 --- a/lib/elixir/lib/set.ex +++ b/lib/elixir/lib/set.ex @@ -90,7 +90,7 @@ defmodule Set do if target1 == target2 do target1.intersection(set1, set2) else - Enumerable.reduce(set1, {:cont, target1.new}, fn v, acc -> + Enumerable.reduce(set1, {:cont, target1.new()}, fn v, acc -> {:cont, if(target2.member?(set2, v), do: target1.put(acc, v), else: acc)} end) |> elem(1) diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl index a9802acdd98..83b779c7171 100644 --- a/lib/elixir/src/elixir_locals.erl +++ b/lib/elixir/src/elixir_locals.erl @@ -106,9 +106,9 @@ ensure_no_undefined_local(Module, All, E) -> warn_unused_local(Module, All, Private, E) -> if_tracker(Module, [], fun(Tracker) -> - {_Unreachable, Warnings} = ?locals:collect_unused_locals(Tracker, All, Private), + {Unreachable, Warnings} = ?locals:collect_unused_locals(Tracker, All, Private), [elixir_errors:file_warn(Meta, E, ?MODULE, Error) || {Meta, Error} <- Warnings], - [] + Unreachable end). format_error({function_conflict, {Receiver, {Name, Arity}}}) -> diff --git a/lib/elixir/test/elixir/kernel/quote_test.exs b/lib/elixir/test/elixir/kernel/quote_test.exs index 36f99d57c85..2b684f1680b 100644 --- a/lib/elixir/test/elixir/kernel/quote_test.exs +++ b/lib/elixir/test/elixir/kernel/quote_test.exs @@ -383,8 +383,6 @@ defmodule Kernel.QuoteTest.ErrorsTest do mod = Kernel.QuoteTest.ErrorsTest file = __ENV__.file |> Path.relative_to_cwd() |> String.to_charlist() assert [{^mod, :will_raise, 2, [file: ^file, line: @line] ++ _} | _] = __STACKTRACE__ - else - _ -> flunk("expected failure") end end From d267911f24e1f1cb27b1b1a8c7f5ba54317e2b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Nov 2024 22:01:28 +0100 Subject: [PATCH 08/22] Hide obvious failures from type system --- lib/elixir/test/elixir/kernel/raise_test.exs | 6 ++---- .../test/elixir/kernel/special_forms_test.exs | 4 +--- lib/elixir/test/elixir/kernel/with_test.exs | 2 +- lib/elixir/test/elixir/kernel_test.exs | 7 +++---- lib/elixir/test/elixir/map_test.exs | 18 +++++------------- lib/ex_unit/test/ex_unit/assertions_test.exs | 6 +++--- lib/ex_unit/test/ex_unit/callbacks_test.exs | 8 ++++++-- 7 files changed, 21 insertions(+), 30 deletions(-) diff --git a/lib/elixir/test/elixir/kernel/raise_test.exs b/lib/elixir/test/elixir/kernel/raise_test.exs index 35b23e84056..47320f9bef2 100644 --- a/lib/elixir/test/elixir/kernel/raise_test.exs +++ b/lib/elixir/test/elixir/kernel/raise_test.exs @@ -458,12 +458,10 @@ defmodule Kernel.RaiseTest do assert result == "no match of right hand side value: 0" end - defp empty_map(), do: %{} - test "bad key error" do result = try do - %{empty_map() | foo: :bar} + %{Process.get(:unused, %{}) | foo: :bar} rescue x in [KeyError] -> Exception.message(x) end @@ -472,7 +470,7 @@ defmodule Kernel.RaiseTest do result = try do - empty_map().foo + Process.get(:unused, %{}).foo rescue x in [KeyError] -> Exception.message(x) end diff --git a/lib/elixir/test/elixir/kernel/special_forms_test.exs b/lib/elixir/test/elixir/kernel/special_forms_test.exs index e048310c28e..c7fca8fe25f 100644 --- a/lib/elixir/test/elixir/kernel/special_forms_test.exs +++ b/lib/elixir/test/elixir/kernel/special_forms_test.exs @@ -68,12 +68,10 @@ defmodule Kernel.SpecialFormsTest do end end - def false_fun(), do: false - test "cond_clause error keeps line number in stacktrace" do try do cond do - false_fun() -> :ok + Process.get(:unused, false) -> :ok end rescue _ -> diff --git a/lib/elixir/test/elixir/kernel/with_test.exs b/lib/elixir/test/elixir/kernel/with_test.exs index c4a17f18e67..e2edfb3aecb 100644 --- a/lib/elixir/test/elixir/kernel/with_test.exs +++ b/lib/elixir/test/elixir/kernel/with_test.exs @@ -126,7 +126,7 @@ defmodule Kernel.WithTest do end assert_raise RuntimeError, fn -> - with({:ok, res} <- ok(42), res = res + oops(), do: res) + with({:ok, res} <- ok(42), oops(), do: res) end end diff --git a/lib/elixir/test/elixir/kernel_test.exs b/lib/elixir/test/elixir/kernel_test.exs index c97adc21519..7207c3b3e11 100644 --- a/lib/elixir/test/elixir/kernel_test.exs +++ b/lib/elixir/test/elixir/kernel_test.exs @@ -11,7 +11,6 @@ defmodule KernelTest do def id(arg), do: arg def id(arg1, arg2), do: {arg1, arg2} - def empty_list(), do: [] def empty_map, do: %{} defp purge(module) do @@ -1359,7 +1358,7 @@ defmodule KernelTest do assert is_map_key(Map.new(a: 1), :a) == true assert_raise BadMapError, fn -> - is_map_key(empty_list(), :a) + is_map_key(Process.get(:unused, []), :a) end case Map.new(a: 1) do @@ -1381,7 +1380,7 @@ defmodule KernelTest do test "tl/1" do assert tl([:one]) == [] assert tl([1, 2, 3]) == [2, 3] - assert_raise ArgumentError, fn -> tl(empty_list()) end + assert_raise ArgumentError, fn -> tl(Process.get(:unused, [])) end assert tl([:a | :b]) == :b assert tl([:a, :b | :c]) == [:b | :c] @@ -1389,7 +1388,7 @@ defmodule KernelTest do test "hd/1" do assert hd([1, 2, 3, 4]) == 1 - assert_raise ArgumentError, fn -> hd(empty_list()) end + assert_raise ArgumentError, fn -> hd(Process.get(:unused, [])) end assert hd([1 | 2]) == 1 end diff --git a/lib/elixir/test/elixir/map_test.exs b/lib/elixir/test/elixir/map_test.exs index b45b52017ff..553d1956f48 100644 --- a/lib/elixir/test/elixir/map_test.exs +++ b/lib/elixir/test/elixir/map_test.exs @@ -6,8 +6,7 @@ defmodule MapTest do doctest Map @sample %{a: 1, b: 2} - - defp sample, do: @sample + defp sample, do: Process.get(:unused, %{a: 1, b: 2}) test "maps in attributes" do assert @sample == %{a: 1, b: 2} @@ -82,7 +81,7 @@ defmodule MapTest do test "map_size/1" do assert map_size(%{}) == 0 - assert map_size(@sample) == 2 + assert map_size(sample()) == 2 end test "new/1" do @@ -282,8 +281,6 @@ defmodule MapTest do defstruct name: "john", age: 27 end - defp empty_map(), do: %{} - test "structs" do assert %ExternalUser{} == %{__struct__: ExternalUser, name: "john", age: 27} @@ -294,10 +291,6 @@ defmodule MapTest do %ExternalUser{name: name} = %ExternalUser{} assert name == "john" - - assert_raise BadStructError, "expected a struct named MapTest.ExternalUser, got: %{}", fn -> - %ExternalUser{empty_map() | name: "meg"} - end end describe "structs with variable name" do @@ -325,12 +318,11 @@ defmodule MapTest do end end - defp foo(), do: "foo" defp destruct1(%module{}), do: module defp destruct2(%_{}), do: :ok test "does not match" do - invalid_struct = %{__struct__: foo()} + invalid_struct = Process.get(:unused, %{__struct__: "foo"}) assert_raise CaseClauseError, fn -> case invalid_struct do @@ -345,7 +337,7 @@ defmodule MapTest do end assert_raise CaseClauseError, fn -> - foo = foo() + foo = Process.get(:unused, "foo") case invalid_struct do %^foo{} -> :ok @@ -370,7 +362,7 @@ defmodule MapTest do end assert_raise MatchError, fn -> - foo = foo() + foo = Process.get(:unused, "foo") %^foo{} = invalid_struct end end diff --git a/lib/ex_unit/test/ex_unit/assertions_test.exs b/lib/ex_unit/test/ex_unit/assertions_test.exs index 35adec39079..e2094f44535 100644 --- a/lib/ex_unit/test/ex_unit/assertions_test.exs +++ b/lib/ex_unit/test/ex_unit/assertions_test.exs @@ -314,13 +314,13 @@ defmodule ExUnit.AssertionsTest do true = assert match?({2, 1}, Value.tuple()) try do - assert match?({:ok, _}, error(true)) + assert match?({:ok, _}, Process.get(:unused, :ok)) flunk("This should never be tested") rescue error in [ExUnit.AssertionError] -> "match (match?) failed" = error.message - "assert match?({:ok, _}, error(true))" = Macro.to_string(error.expr) - "{:error, true}" = Macro.to_string(error.right) + "assert match?({:ok, _}, Process.get(:unused, :ok))" = Macro.to_string(error.expr) + ":ok" = Macro.to_string(error.right) end end diff --git a/lib/ex_unit/test/ex_unit/callbacks_test.exs b/lib/ex_unit/test/ex_unit/callbacks_test.exs index 7778b8ceb65..dcd6eda2667 100644 --- a/lib/ex_unit/test/ex_unit/callbacks_test.exs +++ b/lib/ex_unit/test/ex_unit/callbacks_test.exs @@ -446,10 +446,14 @@ defmodule ExUnit.CallbacksNoTests do use ExUnit.Case, async: true setup_all do - raise "never run" + if :rand.uniform() >= 0 do + raise "never run" + end end setup do - raise "never run" + if :rand.uniform() >= 0 do + raise "never run" + end end end From 453a4357acf7f725734898248bcbcb7be5369453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 09:32:45 +0100 Subject: [PATCH 09/22] Optimize vars recomputation in patterns --- lib/elixir/lib/module/types/pattern.ex | 52 ++++++++++++-------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex index 1309eeb9651..9cec8e640d9 100644 --- a/lib/elixir/lib/module/types/pattern.ex +++ b/lib/elixir/lib/module/types/pattern.ex @@ -114,26 +114,37 @@ defmodule Module.Types.Pattern do {type, context} end + defp all_single_path?(vars, info, index) do + info + |> Map.get(index, []) + |> Enum.all?(fn version -> match?([_], Map.fetch!(vars, version)) end) + end + defp of_pattern_recur(types, tag, stack, context, callback) do - %{pattern_info: {pattern_vars, pattern_info, _counter}} = context + %{pattern_info: {vars, info, _counter}} = context context = nilify_pattern_info(context) - pattern_vars = Map.to_list(pattern_vars) changed = :lists.seq(0, length(types) - 1) + # If all variables in a given index have a single path, + # then there are no changes to propagate + unchangeable = for index <- changed, all_single_path?(vars, info, index), do: index + + vars = Map.to_list(vars) + try do case callback.(types, changed, context) do {:ok, types, context} -> - of_pattern_recur(types, pattern_vars, pattern_info, tag, stack, context, callback) + of_pattern_recur(types, unchangeable, vars, info, tag, stack, context, callback) {:error, context} -> - {types, error_vars(pattern_vars, context)} + {types, error_vars(vars, context)} end catch - {types, context} -> {types, error_vars(pattern_vars, context)} + {types, context} -> {types, error_vars(vars, context)} end end - defp of_pattern_recur(types, vars, info, tag, stack, context, callback) do + defp of_pattern_recur(types, unchangeable, vars, info, tag, stack, context, callback) do {changed, context} = Enum.reduce(vars, {[], context}, fn {version, paths}, {changed, context} -> {var_changed?, context} = @@ -165,23 +176,12 @@ defmodule Module.Types.Pattern do {changed, context} true -> - case paths do - # A single change, check if there are other variables in this index. - [[_var, {:arg, index, _} | _]] -> - case info do - %{^index => true} -> {[index | changed], context} - %{^index => false} -> {changed, context} - end - - # Several changes, we have to recompute all indexes. - _ -> - var_changed = Enum.map(paths, fn [_var, {:arg, index, _} | _] -> index end) - {var_changed ++ changed, context} - end + var_changed = Enum.map(paths, fn [_var, {:arg, index, _} | _] -> index end) + {var_changed ++ changed, context} end end) - case :lists.usort(changed) do + case :lists.usort(changed) -- unchangeable do [] -> {types, context} @@ -192,7 +192,7 @@ defmodule Module.Types.Pattern do {types, context} {:ok, types, context} -> - of_pattern_recur(types, vars, info, tag, stack, context, callback) + of_pattern_recur(types, unchangeable, vars, info, tag, stack, context, callback) {:error, context} -> {types, error_vars(vars, context)} @@ -487,14 +487,8 @@ defmodule Module.Types.Pattern do paths = [[var | path] | Map.get(vars, version, [])] vars = Map.put(vars, version, paths) - # Our goal here is to compute if an argument has more than one variable. - info = - case info do - %{^arg => false} -> %{info | arg => true} - %{^arg => true} -> info - %{} -> Map.put(info, arg, false) - end - + # Stores all variables used at any given argument + info = Map.update(info, arg, [version], &[version | &1]) {{:var, version}, %{context | pattern_info: {vars, info, counter}}} end From ba4664d1bcd340385965f533954374f7b3a631f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 13:51:13 +0100 Subject: [PATCH 10/22] Start moving unreachable checks to types --- lib/elixir/lib/code.ex | 14 ++------------ lib/elixir/lib/module/parallel_checker.ex | 11 +++-------- lib/elixir/lib/module/types.ex | 19 +++++++++++++++---- lib/elixir/src/elixir.erl | 1 - lib/elixir/src/elixir_compiler.erl | 1 - lib/elixir/src/elixir_def.erl | 15 +++++++++++---- lib/elixir/src/elixir_dispatch.erl | 10 +++++----- lib/elixir/src/elixir_map.erl | 2 +- lib/elixir/src/elixir_module.erl | 6 ++---- lib/mix/lib/mix/compilers/test.ex | 2 +- 10 files changed, 40 insertions(+), 41 deletions(-) diff --git a/lib/elixir/lib/code.ex b/lib/elixir/lib/code.ex index ecb2c857b2b..48f6fb9077b 100644 --- a/lib/elixir/lib/code.ex +++ b/lib/elixir/lib/code.ex @@ -249,7 +249,6 @@ defmodule Code do :debug_info, :ignore_already_consolidated, :ignore_module_conflict, - :infer_signatures, :relative_paths ] @@ -1562,8 +1561,8 @@ defmodule Code do ## Examples - Code.compiler_options(infer_signatures: false) - #=> %{infer_signatures: true} + Code.compiler_options(ignore_module_conflict: true) + #=> %{ignore_module_conflict: false} """ @spec compiler_options(Enumerable.t({atom, term})) :: %{optional(atom) => term} @@ -1647,15 +1646,6 @@ defmodule Code do * `:ignore_module_conflict` - when `true`, does not warn when a module has already been defined. Defaults to `false`. - * `:infer_signatures` (since v1.18.0) - when `false`, it disables module-local - signature inference used when type checking remote calls to the compiled - module. Type checking will be executed regardless of this value of this option. - Defaults to `true`. - - `mix test` automatically disables this option via the `:test_elixirc_options` - project configuration, as there is typically no need to store infer signatures - for test files. - * `:relative_paths` - when `true`, uses relative paths in quoted nodes, warnings, and errors generated by the compiler. Note disabling this option won't affect runtime warnings and errors. Defaults to `true`. diff --git a/lib/elixir/lib/module/parallel_checker.ex b/lib/elixir/lib/module/parallel_checker.ex index a64ab47123a..29d322c1e08 100644 --- a/lib/elixir/lib/module/parallel_checker.ex +++ b/lib/elixir/lib/module/parallel_checker.ex @@ -47,15 +47,10 @@ defmodule Module.ParallelChecker do @doc """ Spawns a process that runs the parallel checker. """ - def spawn(pid_checker, module_map, log?, infer_types?, env) do + def spawn(pid_checker, module_map, log?, env) do %{module: module, definitions: definitions, file: file} = module_map - - module_map = - if infer_types? do - %{module_map | signatures: Module.Types.infer(module, file, definitions, env)} - else - module_map - end + {signatures, unreachable} = Module.Types.infer(module, file, definitions, env) + module_map = %{module_map | signatures: signatures, unreachable: unreachable} with {pid, checker} <- pid_checker do ets = :gen_server.call(checker, :ets, :infinity) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 78e4c2303cc..9ccd392c5e5 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -16,16 +16,27 @@ defmodule Module.Types do stack = stack(:infer, file, module, {:__info__, 1}, :all, env, handler) context = context(%{}) - {types, _context} = + {types, %{local_sigs: local_sigs}} = for {fun_arity, kind, _meta, _clauses} = def <- defs, - kind == :def and fun_arity not in @no_infer, + kind == :def or kind == :defmacro, reduce: {[], context} do {types, context} -> {_kind, inferred, context} = local_handler(fun_arity, stack, context, fn _ -> def end) - {[{fun_arity, inferred} | types], context} + + if kind == :def and fun_arity not in @no_infer do + {[{fun_arity, inferred} | types], context} + else + {types, context} + end end - Map.new(types) + unreachable = + for {fun_arity, kind, meta, _clauses} <- defs, + kind == :defp or kind == :defmacrop, + not is_map_key(local_sigs, fun_arity), + do: {fun_arity, meta} + + {Map.new(types), unreachable} end @doc false diff --git a/lib/elixir/src/elixir.erl b/lib/elixir/src/elixir.erl index c6042be8094..7c568d511e7 100644 --- a/lib/elixir/src/elixir.erl +++ b/lib/elixir/src/elixir.erl @@ -88,7 +88,6 @@ start(_Type, _Args) -> %% Compiler options {docs, true}, - {infer_signatures, true}, {ignore_already_consolidated, false}, {ignore_module_conflict, false}, {on_undefined_variable, raise}, diff --git a/lib/elixir/src/elixir_compiler.erl b/lib/elixir/src/elixir_compiler.erl index 0ee5e89a50a..a30731a79cd 100644 --- a/lib/elixir/src/elixir_compiler.erl +++ b/lib/elixir/src/elixir_compiler.erl @@ -144,7 +144,6 @@ bootstrap() -> elixir_config:static(#{bootstrap => true}), elixir_config:put(docs, false), elixir_config:put(ignore_module_conflict, true), - elixir_config:put(infer_signatures, false), elixir_config:put(on_undefined_variable, raise), elixir_config:put(parser_options, []), elixir_config:put(relative_paths, false), diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl index 77fc35a341d..f9425e9c2c9 100644 --- a/lib/elixir/src/elixir_def.erl +++ b/lib/elixir/src/elixir_def.erl @@ -37,9 +37,11 @@ fun_for(Meta, Module, Name, Arity, Kinds, External) -> {[{_, Kind, LocalMeta, _, _, _}], ClausesPairs} -> case (Kinds == all) orelse (lists:member(Kind, Kinds)) of true -> - Local = {value, fun(Fun, Args) -> invoke_local(Meta, Module, Fun, Args, External) end}, + TrackLocal = (Kind == defmacrop), + TrackLocal andalso track_local(Module, Tuple), + Local = {value, fun(Fun, Args) -> invoke_local(Meta, Module, Fun, Args, TrackLocal, External) end}, Clauses = [Clause || {_, Clause} <- ClausesPairs], - elixir_erl:definition_to_anonymous(Kind, LocalMeta, Clauses, Local, External); + {Kind, elixir_erl:definition_to_anonymous(Kind, LocalMeta, Clauses, Local, External)}; false -> false end; @@ -49,17 +51,22 @@ fun_for(Meta, Module, Name, Arity, Kinds, External) -> _:_ -> false end. -invoke_local(Meta, Module, ErlName, Args, External) -> +invoke_local(Meta, Module, ErlName, Args, TrackLocal, 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 -> + {Kind, Fun} -> + (TrackLocal and (Kind == defp)) andalso track_local(Module, {Name, Arity}), apply(Fun, Args) end. +track_local(Module, FunArity) -> + {_, Bag} = elixir_module:data_tables(Module), + ets:insert(Bag, {macro_private_calls, FunArity}). + invoke_external(Meta, Mod, Name, Args, E) -> is_map(E) andalso elixir_env:trace({require, Meta, Mod, []}, E), apply(Mod, Name, Args). diff --git a/lib/elixir/src/elixir_dispatch.erl b/lib/elixir/src/elixir_dispatch.erl index 1f6574bb736..7e70a928de8 100644 --- a/lib/elixir/src/elixir_dispatch.erl +++ b/lib/elixir/src/elixir_dispatch.erl @@ -171,23 +171,23 @@ expand_import(Meta, Name, Arity, E, Extra, AllowLocals, Trace) -> _ -> Local = AllowLocals andalso elixir_def:local_for(Meta, Name, Arity, [defmacro, defmacrop], E), - case Dispatch of + case {Dispatch, Local} of %% There is a local and an import. This is a conflict unless %% the receiver is the same as module (happens on bootstrap). - {_, Receiver} when Local /= false, Receiver /= Module -> + {{_, Receiver}, {_, _}} when Receiver /= Module -> {conflict, Receiver}; %% There is no local. Dispatch the import. - _ when Local == false -> + {_, false} -> do_expand_import(Dispatch, Meta, Name, Arity, Module, E, Trace); %% Dispatch to the local. - _ -> + {_, {_Kind, Fun}} -> Trace andalso begin elixir_env:trace({local_macro, Meta, Name, Arity}, E), elixir_locals:record_local(Tuple, Module, ?key(E, function), Meta, true) end, - {macro, Module, expander_macro_fun(Meta, Local, Module, Name, E)} + {macro, Module, expander_macro_fun(Meta, Fun, Module, Name, E)} end end. diff --git a/lib/elixir/src/elixir_map.erl b/lib/elixir/src/elixir_map.erl index d2b0fdac573..1512d786c5b 100644 --- a/lib/elixir/src/elixir_map.erl +++ b/lib/elixir/src/elixir_map.erl @@ -192,7 +192,7 @@ maybe_load_struct(Meta, Name, Assocs, E) -> false -> Name:'__struct__'(Assocs); - ExternalFun -> + {_, ExternalFun} -> %% There is an inherent race condition when using external_for. %% By the time we got to execute the function, the ETS table %% with temporary definitions for the given module may no longer diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index ade9e5071bd..6c7ca892d13 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -540,12 +540,10 @@ spawn_parallel_checker(CheckerInfo, ModuleMap, E) -> _ -> true end, - Infer = elixir_config:get(infer_signatures), - if %% We need this clause for bootstrap reasons - CheckerInfo /= nil; Infer -> - 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, ModuleMap, Log, Infer, E); + CheckerInfo /= nil -> + 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, ModuleMap, Log, E); true -> ModuleMap end. diff --git a/lib/mix/lib/mix/compilers/test.ex b/lib/mix/lib/mix/compilers/test.ex index 1cd86d20e30..0616179069c 100644 --- a/lib/mix/lib/mix/compilers/test.ex +++ b/lib/mix/lib/mix/compilers/test.ex @@ -23,7 +23,7 @@ defmodule Mix.Compilers.Test do """ def require_and_run(matched_test_files, test_paths, elixirc_opts, opts) do elixirc_opts = - Keyword.merge([docs: false, debug_info: false, infer_signatures: false], elixirc_opts) + Keyword.merge([docs: false, debug_info: false], elixirc_opts) previous_opts = Code.compiler_options(elixirc_opts) From 6d7ea45883e28cdf4590ad47f07ac7a2e195a937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 16:56:24 +0100 Subject: [PATCH 11/22] Move unreachable tracker to typing --- lib/elixir/lib/module/parallel_checker.ex | 7 +- lib/elixir/lib/module/types.ex | 67 +++++++++++++++++-- lib/elixir/src/elixir_module.erl | 81 ++++++++++------------- 3 files changed, 103 insertions(+), 52 deletions(-) diff --git a/lib/elixir/lib/module/parallel_checker.ex b/lib/elixir/lib/module/parallel_checker.ex index 29d322c1e08..2ccd112b8cf 100644 --- a/lib/elixir/lib/module/parallel_checker.ex +++ b/lib/elixir/lib/module/parallel_checker.ex @@ -47,9 +47,12 @@ defmodule Module.ParallelChecker do @doc """ Spawns a process that runs the parallel checker. """ - def spawn(pid_checker, module_map, log?, env) do + def spawn(pid_checker, module_map, log?, private, used, env) do %{module: module, definitions: definitions, file: file} = module_map - {signatures, unreachable} = Module.Types.infer(module, file, definitions, env) + + {signatures, unreachable} = + Module.Types.infer(module, file, definitions, private, used, env) + module_map = %{module_map | signatures: signatures, unreachable: unreachable} with {pid, checker} <- pid_checker do diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 9ccd392c5e5..518b8c7dd11 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -10,7 +10,7 @@ defmodule Module.Types do @no_infer [__protocol__: 1, behaviour_info: 1] @doc false - def infer(module, file, defs, env) do + def infer(module, file, defs, private, used, env) do finder = &List.keyfind(defs, &1, 0) handler = &local_handler(&1, &2, &3, finder) stack = stack(:infer, file, module, {:__info__, 1}, :all, env, handler) @@ -31,14 +31,55 @@ defmodule Module.Types do end unreachable = - for {fun_arity, kind, meta, _clauses} <- defs, - kind == :defp or kind == :defmacrop, + for {fun_arity, _kind, _meta, _defaults} = info <- private, + maybe_warn_unused(info, local_sigs, used, env), not is_map_key(local_sigs, fun_arity), - do: {fun_arity, meta} + do: fun_arity {Map.new(types), unreachable} end + defp maybe_warn_unused({_fun_arity, _kind, false, _}, _reachable, _used, _env) do + :ok + end + + defp maybe_warn_unused({fun_arity, kind, meta, 0}, reachable, used, env) do + case meta == false or Map.has_key?(reachable, fun_arity) or fun_arity in used do + true -> :ok + false -> :elixir_errors.file_warn(meta, env, __MODULE__, {:unused_def, fun_arity, kind}) + end + + :ok + end + + defp maybe_warn_unused({tuple, kind, meta, default}, reachable, used, env) when default > 0 do + {name, arity} = tuple + min = arity - default + max = arity + + case min_reachable_default(max, min, :none, name, reachable, used) do + :none -> :elixir_errors.file_warn(meta, env, __MODULE__, {:unused_def, tuple, kind}) + ^min -> :ok + ^max -> :elixir_errors.file_warn(meta, env, __MODULE__, {:unused_args, tuple}) + diff -> :elixir_errors.file_warn(meta, env, __MODULE__, {:unused_args, tuple, diff}) + end + + :ok + end + + defp min_reachable_default(max, min, last, name, reachable, used) when max >= min do + fun_arity = {name, max} + + case Map.has_key?(reachable, fun_arity) or fun_arity in used do + true -> min_reachable_default(max - 1, min, max, name, reachable, used) + false -> min_reachable_default(max - 1, min, last, name, reachable, used) + end + end + + defp min_reachable_default(_max, _min, last, _name, _reachable, _used) do + last + end + @doc false def warnings(module, file, defs, no_warn_undefined, cache) do finder = &List.keyfind(defs, &1, 0) @@ -247,4 +288,22 @@ defmodule Module.Types do message: "this clause of #{kind} #{fun}/#{arity} is never used" } end + + ## Module errors + + def format_error({:unused_args, {name, arity}}), + do: "default values for the optional arguments in #{name}/#{arity} are never used" + + def format_error({:unused_args, {name, arity}, count}) when arity - count == 1, + do: "the default value for the last optional argument in #{name}/#{arity} is never used" + + def format_error({:unused_args, {name, arity}, count}), + do: + "the default values for the last #{arity - count} optional arguments in #{name}/#{arity} are never used" + + def format_error({:unused_def, {name, arity}, :defp}), + do: "function #{name}/#{arity} is unused" + + def format_error({:unused_def, {name, arity}, :defmacrop}), + do: "macro #{name}/#{arity} is unused" end diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index 6c7ca892d13..8fce02202ad 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -147,7 +147,7 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> NifsAttribute = lists:keyfind(nifs, 1, Attributes), validate_nifs_attribute(NifsAttribute, AllDefinitions, Line, E), - Unreachable = elixir_locals:warn_unused_local(Module, AllDefinitions, NewPrivate, E), + % Unreachable = elixir_locals:warn_unused_local(Module, AllDefinitions, NewPrivate, E), elixir_locals:ensure_no_undefined_local(Module, AllDefinitions, E), elixir_locals:ensure_no_import_conflict(Module, AllDefinitions, E), @@ -160,20 +160,14 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> 'Elixir.Module':'__check_attributes__'(E, DataSet, DataBag), RawCompileOpts = bag_lookup_element(DataBag, {accumulate, compile}, 2), - CompileOpts = validate_compile_opts(RawCompileOpts, AllDefinitions, Unreachable, Line, E), + CompileOpts = validate_compile_opts(RawCompileOpts, AllDefinitions, Line, E), Impls = bag_lookup_element(DataBag, impls, 2), AfterVerify = bag_lookup_element(DataBag, {accumulate, after_verify}, 2), [elixir_env:trace({remote_function, [], VerifyMod, VerifyFun, 1}, CallbackE) || {VerifyMod, VerifyFun} <- AfterVerify], - %% Compute signatures only if the module is valid. - case ets:member(DataSet, {elixir, taint}) of - true -> elixir_errors:compile_error(E); - false -> ok - end, - - ModuleMapWithoutSignatures = #{ + PartialModuleMap = #{ struct => get_struct(DataSet), module => Module, anno => Anno, @@ -181,16 +175,20 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> relative_file => elixir_utils:relative_to_cwd(File), attributes => Attributes, definitions => AllDefinitions, - unreachable => Unreachable, after_verify => AfterVerify, compile_opts => CompileOpts, deprecated => get_deprecated(DataBag), defines_behaviour => defines_behaviour(DataBag), impls => Impls, + unreachable => [], signatures => #{} }, - ModuleMap = spawn_parallel_checker(CheckerInfo, ModuleMapWithoutSignatures, CallbackE), + %% Compute signatures only if the module is valid. + compile_error_if_tainted(DataSet, E), + ModuleMap = spawn_parallel_checker(DataBag, CheckerInfo, PartialModuleMap, NewPrivate, E), + compile_error_if_tainted(DataSet, E), + Binary = elixir_erl:compile(ModuleMap), Autoload = proplists:get_value(autoload, CompileOpts, true), {Binary, PersistedAttributes, Autoload} @@ -223,41 +221,33 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> elixir_code_server:call({undefmodule, Ref}) end. -validate_compile_opts(Opts, Defs, Unreachable, Line, E) -> - lists:flatmap(fun (Opt) -> validate_compile_opt(Opt, Defs, Unreachable, Line, E) end, Opts). +compile_error_if_tainted(DataSet, E) -> + case ets:member(DataSet, {elixir, taint}) of + true -> elixir_errors:compile_error(E); + false -> ok + end. + +validate_compile_opts(Opts, Defs, Line, E) -> + lists:flatmap(fun (Opt) -> validate_compile_opt(Opt, Defs, Line, E) end, Opts). %% TODO: Make this an error on v2.0 -validate_compile_opt({parse_transform, Module} = Opt, _Defs, _Unreachable, Line, E) -> +validate_compile_opt({parse_transform, Module} = Opt, _Defs, Line, E) -> elixir_errors:file_warn([{line, Line}], E, ?MODULE, {parse_transform, Module}), [Opt]; -validate_compile_opt({inline, Inlines}, Defs, Unreachable, Line, E) -> - case validate_inlines(Inlines, Defs, Unreachable, []) of - {ok, []} -> - []; - {ok, FilteredInlines} -> - [{inline, FilteredInlines}]; - {error, Reason} -> - elixir_errors:module_error([{line, Line}], E, ?MODULE, Reason), - [] - end; -validate_compile_opt(Opt, Defs, Unreachable, Line, E) when is_list(Opt) -> - validate_compile_opts(Opt, Defs, Unreachable, Line, E); -validate_compile_opt(Opt, _Defs, _Unreachable, _Line, _E) -> - [Opt]. - -validate_inlines([Inline | Inlines], Defs, Unreachable, Acc) -> - case lists:keyfind(Inline, 1, Defs) of +validate_compile_opt({inline, Inlines} = Opt, Defs, Line, E) -> + [case lists:keyfind(Inline, 1, Defs) of false -> - {error, {undefined_function, {compile, inline}, Inline}}; + elixir_errors:module_error([{line, Line}], E, ?MODULE, {undefined_function, {compile, inline}, Inline}); {_Def, Type, _Meta, _Clauses} when Type == defmacro; Type == defmacrop -> - {error, {bad_macro, {compile, inline}, Inline}}; + elixir_errors:module_error([{line, Line}], E, ?MODULE, {bad_macro, {compile, inline}, Inline}); _ -> - case lists:member(Inline, Unreachable) of - true -> validate_inlines(Inlines, Defs, Unreachable, Acc); - false -> validate_inlines(Inlines, Defs, Unreachable, [Inline | Acc]) - end - end; -validate_inlines([], _Defs, _Unreachable, Acc) -> {ok, Acc}. + ok + end || Inline <- Inlines], + [Opt]; +validate_compile_opt(Opt, Defs, Line, E) when is_list(Opt) -> + validate_compile_opts(Opt, Defs, Line, E); +validate_compile_opt(Opt, _Defs, _Line, _E) -> + [Opt]. validate_on_load_attribute({on_load, Def}, Defs, Private, Line, E) -> case lists:keyfind(Def, 1, Defs) of @@ -533,19 +523,18 @@ checker_info() -> _ -> 'Elixir.Module.ParallelChecker':get() end. -spawn_parallel_checker(CheckerInfo, ModuleMap, E) -> +spawn_parallel_checker(DataBag, CheckerInfo, ModuleMap, Private, E) -> Log = case erlang:get(elixir_code_diagnostics) of {_, false} -> false; _ -> true end, - if - %% We need this clause for bootstrap reasons - CheckerInfo /= nil -> - 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, ModuleMap, Log, E); - true -> - ModuleMap + Used = bag_lookup_element(DataBag, macro_private_calls, 2), + + case elixir_config:is_bootstrap() of + true -> ModuleMap; + false -> 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, ModuleMap, Log, Private, Used, E) end. make_module_available(Module, Binary) -> From 3601ad7b752b0dddb43c7cb03c034403b1f5aab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 17:04:22 +0100 Subject: [PATCH 12/22] Refactor --- lib/elixir/lib/module/types.ex | 44 +++++++++++++++++----------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 518b8c7dd11..ad32f732b3b 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -2,9 +2,7 @@ defmodule Module.Types do @moduledoc false alias Module.Types.{Descr, Expr, Pattern, Helpers} - # TODO: Consider passing inferred types from infer into warnings - # TODO: Consider removing code from locals tracker # These functions are not inferred because they are added/managed by the compiler @no_infer [__protocol__: 1, behaviour_info: 1] @@ -32,18 +30,18 @@ defmodule Module.Types do unreachable = for {fun_arity, _kind, _meta, _defaults} = info <- private, - maybe_warn_unused(info, local_sigs, used, env), + warn_unused_def(info, local_sigs, used, env), not is_map_key(local_sigs, fun_arity), do: fun_arity {Map.new(types), unreachable} end - defp maybe_warn_unused({_fun_arity, _kind, false, _}, _reachable, _used, _env) do + defp warn_unused_def({_fun_arity, _kind, false, _}, _reachable, _used, _env) do :ok end - defp maybe_warn_unused({fun_arity, kind, meta, 0}, reachable, used, env) do + defp warn_unused_def({fun_arity, kind, meta, 0}, reachable, used, env) do case meta == false or Map.has_key?(reachable, fun_arity) or fun_arity in used do true -> :ok false -> :elixir_errors.file_warn(meta, env, __MODULE__, {:unused_def, fun_arity, kind}) @@ -52,7 +50,7 @@ defmodule Module.Types do :ok end - defp maybe_warn_unused({tuple, kind, meta, default}, reachable, used, env) when default > 0 do + defp warn_unused_def({tuple, kind, meta, default}, reachable, used, env) when default > 0 do {name, arity} = tuple min = arity - default max = arity @@ -93,25 +91,27 @@ defmodule Module.Types do context end) - context = - for {fun_arity, pending} <- context.local_used, pending != [], reduce: context do - context -> - {_fun_arity, kind, _meta, clauses} = List.keyfind(defs, fun_arity, 0) - {_kind, _inferred, mapping} = Map.fetch!(context.local_sigs, fun_arity) - - clauses_indexes = - for type_index <- pending, {clause_index, ^type_index} <- mapping, do: clause_index - - Enum.reduce(clauses_indexes, context, fn clause_index, context -> - {meta, _args, _guards, _body} = Enum.fetch!(clauses, clause_index) - stack = %{stack | function: fun_arity} - Helpers.warn(__MODULE__, {:unused_clause, kind, fun_arity}, meta, stack, context) - end) - end - + context = warn_unused_clauses(defs, stack, context) context.warnings end + defp warn_unused_clauses(defs, stack, context) do + for {fun_arity, pending} <- context.local_used, pending != [], reduce: context do + context -> + {_fun_arity, kind, _meta, clauses} = List.keyfind(defs, fun_arity, 0) + {_kind, _inferred, mapping} = Map.fetch!(context.local_sigs, fun_arity) + + clauses_indexes = + for type_index <- pending, {clause_index, ^type_index} <- mapping, do: clause_index + + Enum.reduce(clauses_indexes, context, fn clause_index, context -> + {meta, _args, _guards, _body} = Enum.fetch!(clauses, clause_index) + stack = %{stack | function: fun_arity} + Helpers.warn(__MODULE__, {:unused_clause, kind, fun_arity}, meta, stack, context) + end) + end + end + defp local_handler(fun_arity, stack, context, finder) do case context.local_sigs do %{^fun_arity => {kind, inferred, _mapping}} -> From 43e2b9d0eac442907b34ef34a0f72a95a0f628cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 18:26:59 +0100 Subject: [PATCH 13/22] Move locals to types --- lib/elixir/lib/module.ex | 13 +- lib/elixir/lib/module/types.ex | 142 ++++++++++++------ lib/elixir/lib/module/types/apply.ex | 60 +++++--- lib/elixir/lib/module/types/helpers.ex | 7 + lib/elixir/src/elixir_def.erl | 1 - lib/elixir/src/elixir_dispatch.erl | 6 +- lib/elixir/src/elixir_expand.erl | 3 +- lib/elixir/src/elixir_locals.erl | 47 +----- lib/elixir/src/elixir_module.erl | 3 - lib/elixir/src/elixir_overridable.erl | 17 +-- .../test/elixir/kernel/warning_test.exs | 31 ++++ .../test/elixir/module/types/type_helper.exs | 2 +- 12 files changed, 182 insertions(+), 150 deletions(-) diff --git a/lib/elixir/lib/module.ex b/lib/elixir/lib/module.ex index e47834f4d1a..b6b747196db 100644 --- a/lib/elixir/lib/module.ex +++ b/lib/elixir/lib/module.ex @@ -1415,15 +1415,7 @@ defmodule Module do def delete_definition(module, {name, arity}) when is_atom(module) and is_atom(name) and is_integer(arity) do assert_not_readonly!(__ENV__.function, module) - - case :elixir_def.take_definition(module, {name, arity}) do - false -> - false - - _ -> - :elixir_locals.yank({name, arity}, module) - true - end + :elixir_def.take_definition(module, {name, arity}) != false end @doc """ @@ -1452,8 +1444,7 @@ defmodule Module do "overridable because it was not defined" clause -> - neighbours = :elixir_locals.yank(tuple, module) - :elixir_overridable.record_overridable(module, tuple, clause, neighbours) + :elixir_overridable.record_overridable(module, tuple, clause) end other -> diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index ad32f732b3b..a215062d524 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -9,17 +9,34 @@ defmodule Module.Types do @doc false def infer(module, file, defs, private, used, env) do - finder = &List.keyfind(defs, &1, 0) - handler = &local_handler(&1, &2, &3, finder) + finder = &:lists.keyfind(&1, 1, defs) + + handler = fn meta, fun_arity, stack, context -> + case local_handler(meta, fun_arity, stack, context, finder) do + false -> + undefined_function!(:undefined_function, meta, fun_arity, stack, env) + false + + {kind, _, _} = triplet -> + if (kind == :defmacro or kind == :defmacrop) and not Keyword.has_key?(meta, :super) do + undefined_function!(:incorrect_dispatch, meta, fun_arity, stack, env) + false + else + triplet + end + end + end + stack = stack(:infer, file, module, {:__info__, 1}, :all, env, handler) context = context(%{}) {types, %{local_sigs: local_sigs}} = - for {fun_arity, kind, _meta, _clauses} = def <- defs, + for {fun_arity, kind, meta, _clauses} = def <- defs, kind == :def or kind == :defmacro, reduce: {[], context} do {types, context} -> - {_kind, inferred, context} = local_handler(fun_arity, stack, context, fn _ -> def end) + {_kind, inferred, context} = + local_handler(meta, fun_arity, stack, context, fn _ -> def end) if kind == :def and fun_arity not in @no_infer do {[{fun_arity, inferred} | types], context} @@ -37,6 +54,12 @@ defmodule Module.Types do {Map.new(types), unreachable} end + defp undefined_function!(reason, meta, {fun, arity}, stack, env) do + env = %{env | function: stack.function, file: stack.file} + tuple = {reason, {fun, arity}, stack.module} + :elixir_errors.module_error(Helpers.with_span(meta, fun), env, __MODULE__, tuple) + end + defp warn_unused_def({_fun_arity, _kind, false, _}, _reachable, _used, _env) do :ok end @@ -80,14 +103,16 @@ defmodule Module.Types do @doc false def warnings(module, file, defs, no_warn_undefined, cache) do - finder = &List.keyfind(defs, &1, 0) - handler = &local_handler(&1, &2, &3, finder) + finder = &:lists.keyfind(&1, 1, defs) + handler = &local_handler(&1, &2, &3, &4, finder) stack = stack(:dynamic, file, module, {:__info__, 1}, no_warn_undefined, cache, handler) context = context(%{}) context = - Enum.reduce(defs, context, fn {fun_arity, _kind, _meta, _clauses} = def, context -> - {_kind, _inferred, context} = local_handler(fun_arity, stack, context, fn _ -> def end) + Enum.reduce(defs, context, fn {fun_arity, _kind, meta, _clauses} = def, context -> + {_kind, _inferred, context} = + local_handler(meta, fun_arity, stack, context, fn _ -> def end) + context end) @@ -112,7 +137,7 @@ defmodule Module.Types do end end - defp local_handler(fun_arity, stack, context, finder) do + defp local_handler(_meta, fun_arity, stack, context, finder) do case context.local_sigs do %{^fun_arity => {kind, inferred, _mapping}} -> {kind, inferred, context} @@ -121,47 +146,58 @@ defmodule Module.Types do {kind, :none, context} local_sigs -> - {{fun, arity}, kind, meta, clauses} = - finder.(fun_arity) || raise "could not find #{inspect(fun_arity)}" - - expected = List.duplicate(Descr.dynamic(), arity) - stack = stack |> fresh_stack(fun_arity) |> with_file_meta(meta) - context = put_in(context.local_sigs, Map.put(local_sigs, fun_arity, kind)) - - {_, _, mapping, clauses_types, clauses_context} = - Enum.reduce(clauses, {0, 0, [], [], context}, fn - {meta, args, guards, body}, {index, total, mapping, inferred, context} -> - context = fresh_context(context) - - try do - {args_types, context} = - Pattern.of_head(args, guards, expected, :default, meta, stack, context) - - {return_type, context} = - Expr.of_expr(body, stack, context) - - {type_index, inferred} = - add_inferred(inferred, args_types, return_type, total - 1, []) - - if type_index == -1 do - {index + 1, total + 1, [{index, total} | mapping], inferred, context} - else - {index + 1, total, [{index, type_index} | mapping], inferred, context} - end - rescue - e -> - internal_error!(e, __STACKTRACE__, kind, meta, fun, args, guards, body, stack) - end - end) - - inferred = {:infer, Enum.reverse(clauses_types)} - triplet = {kind, inferred, mapping} - context = restore_context(context, clauses_context) - context = update_in(context.local_sigs, &Map.put(&1, fun_arity, triplet)) - {kind, inferred, context} + case finder.(fun_arity) do + {fun_arity, kind, meta, clauses} -> + context = put_in(context.local_sigs, Map.put(local_sigs, fun_arity, kind)) + + {inferred, mapping, context} = + local_handler(fun_arity, kind, meta, clauses, stack, context) + + context = + update_in(context.local_sigs, &Map.put(&1, fun_arity, {kind, inferred, mapping})) + + {kind, inferred, context} + + false -> + false + end end end + defp local_handler({fun, arity} = fun_arity, kind, meta, clauses, stack, context) do + expected = List.duplicate(Descr.dynamic(), arity) + stack = stack |> fresh_stack(fun_arity) |> with_file_meta(meta) + + {_, _, mapping, clauses_types, clauses_context} = + Enum.reduce(clauses, {0, 0, [], [], context}, fn + {meta, args, guards, body}, {index, total, mapping, inferred, context} -> + context = fresh_context(context) + + try do + {args_types, context} = + Pattern.of_head(args, guards, expected, :default, meta, stack, context) + + {return_type, context} = + Expr.of_expr(body, stack, context) + + {type_index, inferred} = + add_inferred(inferred, args_types, return_type, total - 1, []) + + if type_index == -1 do + {index + 1, total + 1, [{index, total} | mapping], inferred, context} + else + {index + 1, total, [{index, type_index} | mapping], inferred, context} + end + rescue + e -> + internal_error!(e, __STACKTRACE__, kind, meta, fun, args, guards, body, stack) + end + end) + + inferred = {:infer, Enum.reverse(clauses_types)} + {inferred, mapping, restore_context(context, clauses_context)} + end + # We check for term equality of types as an optimization # to reduce the amount of check we do at runtime. defp add_inferred([{args, existing_return} | tail], args, return, index, acc), @@ -306,4 +342,16 @@ defmodule Module.Types do def format_error({:unused_def, {name, arity}, :defmacrop}), do: "macro #{name}/#{arity} is unused" + + def format_error({:undefined_function, {f, a}, _}) + when {f, a} in [__info__: 1, behaviour_info: 1, module_info: 1, module_info: 0], + do: + "undefined function #{f}/#{a} (this function is auto-generated by the compiler and must always be called as a remote, as in __MODULE__.#{f}/#{a})" + + def format_error({:undefined_function, {f, a}, module}), + do: + "undefined function #{f}/#{a} (expected #{inspect(module)} to define such a function or for it to be imported, but none are available)" + + def format_error({:incorrect_dispatch, {f, a}, _module}), + do: "cannot invoke macro #{f}/#{a} before its definition" end diff --git a/lib/elixir/lib/module/types/apply.ex b/lib/elixir/lib/module/types/apply.ex index ed8c21fd945..55f9fe97783 100644 --- a/lib/elixir/lib/module/types/apply.ex +++ b/lib/elixir/lib/module/types/apply.ex @@ -508,30 +508,35 @@ defmodule Module.Types.Apply do @doc """ Deal with local functions. """ - def local(fun, args_types, expr, stack, context) do + def local(fun, args_types, {_, meta, _} = expr, stack, context) do fun_arity = {fun, length(args_types)} - {kind, info, context} = stack.local_handler.(fun_arity, stack, context) - case apply_signature(info, args_types, stack) do - {:ok, indexes, type} -> - context = - if stack != :infer and kind == :defp do - update_in(context.local_used[fun_arity], fn current -> - if info == :none do - [] + case stack.local_handler.(meta, fun_arity, stack, context) do + false -> + {dynamic(), context} + + {kind, info, context} -> + case apply_signature(info, args_types, stack) do + {:ok, indexes, type} -> + context = + if stack != :infer and kind == :defp do + update_in(context.local_used[fun_arity], fn current -> + if info == :none do + [] + else + (current || used_from_clauses(info)) -- indexes + end + end) else - (current || used_from_clauses(info)) -- indexes + context end - end) - else - context - end - {type, context} + {type, context} - {:error, domain, clauses} -> - error = {:badlocal, expr, args_types, domain, clauses, context} - {error_type(), error(error, elem(expr, 1), stack, context)} + {:error, domain, clauses} -> + error = {:badlocal, expr, args_types, domain, clauses, context} + {error_type(), error(error, with_span(meta, fun), stack, context)} + end end end @@ -544,15 +549,20 @@ defmodule Module.Types.Apply do @doc """ Deal with local captures. """ - def local_capture(fun, arity, _meta, stack, context) do + def local_capture(fun, arity, meta, stack, context) do fun_arity = {fun, arity} - {kind, _info, context} = stack.local_handler.(fun_arity, stack, context) - if stack != :infer and kind == :defp do - # Mark all clauses as used, as the function is being exported. - {fun(), put_in(context.local_used[fun_arity], [])} - else - {fun(), context} + case stack.local_handler.(meta, fun_arity, stack, context) do + false -> + {fun(), context} + + {kind, _info, context} -> + if stack != :infer and kind == :defp do + # Mark all clauses as used, as the function is being exported. + {fun(), put_in(context.local_used[fun_arity], [])} + else + {fun(), context} + end end end diff --git a/lib/elixir/lib/module/types/helpers.ex b/lib/elixir/lib/module/types/helpers.ex index 51362ce0c4e..fa1774adea4 100644 --- a/lib/elixir/lib/module/types/helpers.ex +++ b/lib/elixir/lib/module/types/helpers.ex @@ -38,6 +38,13 @@ defmodule Module.Types.Helpers do def get_meta({_, meta, _}), do: meta def get_meta(_other), do: [] + @doc """ + Attaches span information. + """ + def with_span(meta, name) do + :elixir_env.calculate_span(meta, name) + end + ## Warnings @doc """ diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl index f9425e9c2c9..b6820df30bb 100644 --- a/lib/elixir/src/elixir_def.erl +++ b/lib/elixir/src/elixir_def.erl @@ -217,7 +217,6 @@ store_definition(Meta, Kind, CheckClauses, Name, Arity, DefaultsArgs, Guards, Bo Clause <- def_to_clauses(Kind, Meta, Args, Guards, Body, E)], DefaultsLength = length(Defaults), - elixir_locals:record_defaults(Tuple, Kind, Module, DefaultsLength, Meta), check_previous_defaults(Meta, Module, Name, Arity, Kind, DefaultsLength, E), store_definition(CheckClauses, Kind, Meta, Name, Arity, File, diff --git a/lib/elixir/src/elixir_dispatch.erl b/lib/elixir/src/elixir_dispatch.erl index 7e70a928de8..990aff4b616 100644 --- a/lib/elixir/src/elixir_dispatch.erl +++ b/lib/elixir/src/elixir_dispatch.erl @@ -74,7 +74,6 @@ import_function(Meta, Name, Arity, E) -> 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), {local, Name, Arity}; _ -> false @@ -183,10 +182,7 @@ expand_import(Meta, Name, Arity, E, Extra, AllowLocals, Trace) -> %% Dispatch to the local. {_, {_Kind, Fun}} -> - Trace andalso begin - elixir_env:trace({local_macro, Meta, Name, Arity}, E), - elixir_locals:record_local(Tuple, Module, ?key(E, function), Meta, true) - end, + Trace andalso elixir_env:trace({local_macro, Meta, Name, Arity}, E), {macro, Module, expander_macro_fun(Meta, Fun, Module, Name, E)} end end. diff --git a/lib/elixir/src/elixir_expand.erl b/lib/elixir/src/elixir_expand.erl index 7e1146b2b2b..113b52b45db 100644 --- a/lib/elixir/src/elixir_expand.erl +++ b/lib/elixir/src/elixir_expand.erl @@ -900,8 +900,7 @@ expand_local(Meta, Name, Args, S, #{module := Module, function := Function, cont nil -> Arity = length(Args), - elixir_env:trace({local_function, Meta, Name, Arity}, E), - elixir_locals:record_local({Name, Arity}, Module, Function, Meta, false) + elixir_env:trace({local_function, Meta, Name, Arity}, E) end, {EArgs, SA, EA} = expand_args(Args, S, E), diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl index 83b779c7171..ef85bb7f78e 100644 --- a/lib/elixir/src/elixir_locals.erl +++ b/lib/elixir/src/elixir_locals.erl @@ -4,7 +4,6 @@ setup/1, stop/1, cache_env/1, get_cached_env/1, record_local/5, record_import/4, record_defaults/5, yank/2, reattach/6, ensure_no_import_conflict/3, - warn_unused_local/4, ensure_no_undefined_local/3, format_error/1 ]). @@ -97,50 +96,6 @@ ensure_no_import_conflict(Module, All, E) -> ok end). -ensure_no_undefined_local(Module, All, E) -> - if_tracker(Module, [], fun(Tracker) -> - [elixir_errors:module_error(Meta, E#{function := Function, file := File}, ?MODULE, {Error, Tuple, Module}) - || {Function, Meta, File, Tuple, Error} <- ?locals:collect_undefined_locals(Tracker, All, ?key(E, file))], - ok - end). - -warn_unused_local(Module, All, Private, E) -> - if_tracker(Module, [], fun(Tracker) -> - {Unreachable, Warnings} = ?locals:collect_unused_locals(Tracker, All, Private), - [elixir_errors:file_warn(Meta, E, ?MODULE, Error) || {Meta, Error} <- Warnings], - Unreachable - end). - format_error({function_conflict, {Receiver, {Name, Arity}}}) -> io_lib:format("imported ~ts.~ts/~B conflicts with local function", - [elixir_aliases:inspect(Receiver), Name, Arity]); - -format_error({unused_args, {Name, Arity}}) -> - io_lib:format("default values for the optional arguments in ~ts/~B are never used", [Name, Arity]); - -format_error({unused_args, {Name, Arity}, Count}) when Arity - Count == 1 -> - io_lib:format("the default value for the last optional argument in ~ts/~B is never used", [Name, Arity]); - -format_error({unused_args, {Name, Arity}, Count}) -> - io_lib:format("the default values for the last ~B optional arguments in ~ts/~B are never used", [Arity - Count, Name, Arity]); - -format_error({unused_def, {Name, Arity}, defp}) -> - io_lib:format("function ~ts/~B is unused", [Name, Arity]); - -format_error({unused_def, {Name, Arity}, defmacrop}) -> - io_lib:format("macro ~ts/~B is unused", [Name, Arity]); - -format_error({undefined_function, {F, A}, _}) - when F == '__info__', A == 1; - F == 'behaviour_info', A == 1; - F == 'module_info', A == 1; - F == 'module_info', A == 0 -> - io_lib:format("undefined function ~ts/~B (this function is auto-generated by the compiler " - "and must always be called as a remote, as in __MODULE__.~ts/~B)", [F, A, F, A]); - -format_error({undefined_function, {F, A}, Module}) -> - io_lib:format("undefined function ~ts/~B (expected ~ts to define such a function or " - "for it to be imported, but none are available)", [F, A, elixir_aliases:inspect(Module)]); - -format_error({incorrect_dispatch, {F, A}, _Module}) -> - io_lib:format("cannot invoke macro ~ts/~B before its definition", [F, A]). + [elixir_aliases:inspect(Receiver), Name, Arity]). diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index 8fce02202ad..20071fcf209 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -146,9 +146,6 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> NifsAttribute = lists:keyfind(nifs, 1, Attributes), validate_nifs_attribute(NifsAttribute, AllDefinitions, Line, E), - - % Unreachable = elixir_locals:warn_unused_local(Module, AllDefinitions, NewPrivate, E), - elixir_locals:ensure_no_undefined_local(Module, AllDefinitions, E), elixir_locals:ensure_no_import_conflict(Module, AllDefinitions, E), %% We stop tracking locals here to avoid race conditions in case after_load diff --git a/lib/elixir/src/elixir_overridable.erl b/lib/elixir/src/elixir_overridable.erl index 1ddb8b9c705..c08b7ec37a2 100644 --- a/lib/elixir/src/elixir_overridable.erl +++ b/lib/elixir/src/elixir_overridable.erl @@ -1,10 +1,10 @@ % Holds the logic responsible for defining overridable functions and handling super. -module(elixir_overridable). -export([overridables_for/1, overridable_for/2, - record_overridable/4, super/4, + record_overridable/3, super/4, store_not_overridden/1, format_error/1]). -include("elixir.hrl"). --define(overridden_pos, 5). +-define(overridden_pos, 4). overridables_for(Module) -> {_, Bag} = elixir_module:data_tables(Module), @@ -22,20 +22,20 @@ overridable_for(Module, Tuple) -> [] -> not_overridable end. -record_overridable(Module, Tuple, Def, Neighbours) -> +record_overridable(Module, Tuple, Def) -> {Set, Bag} = elixir_module:data_tables(Module), - case ets:insert_new(Set, {{overridable, Tuple}, 1, Def, Neighbours, false}) of + case ets:insert_new(Set, {{overridable, Tuple}, 1, Def, false}) of true -> ets:insert(Bag, {overridables, Tuple}); false -> - [{_, Count, PreviousDef, _, _}] = ets:lookup(Set, {overridable, Tuple}), + [{_, Count, PreviousDef, _}] = ets:lookup(Set, {overridable, Tuple}), {{_, Kind, Meta, File, _, _}, _} = Def, {{_, PreviousKind, _, _, _, _}, _} = PreviousDef, case is_valid_kind(Kind, PreviousKind) of true -> - ets:insert(Set, {{overridable, Tuple}, Count + 1, Def, Neighbours, false}); + ets:insert(Set, {{overridable, Tuple}, Count + 1, Def, false}); false -> elixir_errors:file_error(Meta, File, ?MODULE, {bad_kind, Module, Tuple, Kind}) end @@ -74,7 +74,7 @@ store_not_overridden(Module) -> %% Private -store(Set, Module, Tuple, {_, Count, Def, Neighbours, Overridden}, Hidden) -> +store(Set, Module, Tuple, {_, Count, Def, Overridden}, Hidden) -> {{{def, {Name, Arity}}, Kind, Meta, File, _Check, {Defaults, _HasBody, _LastDefaults}}, Clauses} = Def, @@ -92,8 +92,7 @@ store(Set, Module, Tuple, {_, Count, Def, Neighbours, Overridden}, Hidden) -> false -> ets:update_element(Set, {overridable, Tuple}, {?overridden_pos, true}), elixir_def:store_definition(none, FinalKind, Meta, FinalName, FinalArity, - File, Module, Defaults, FinalClauses), - elixir_locals:reattach({FinalName, FinalArity}, FinalKind, Module, Tuple, Neighbours, Meta); + File, Module, Defaults, FinalClauses); true -> ok end, diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs index 1f0f975adbc..0e3073996c5 100644 --- a/lib/elixir/test/elixir/kernel/warning_test.exs +++ b/lib/elixir/test/elixir/kernel/warning_test.exs @@ -645,6 +645,37 @@ defmodule Kernel.WarningTest do purge([Sample, Sample2]) end + test "unused function conditionall inside macro" do + assert_warn_eval( + ["nofile:3:8: ", "function error/0 is unused"], + """ + defmodule Sample do + defp ok, do: :ok + defp error, do: :error + + defmacrop hello(x) do + if x, do: ok(), else: error() + end + + def uses_hello, do: hello(true) + end + """ + ) + + assert_warn_eval( + ["nofile:2:13: ", "macro hello/0 is unused\n"], + ~S""" + defmodule Sample2 do + defmacrop hello do + quote do: unquote(1) + end + end + """ + ) + after + purge([Sample, Sample2]) + end + test "shadowing" do assert capture_eval(""" defmodule Sample do diff --git a/lib/elixir/test/elixir/module/types/type_helper.exs b/lib/elixir/test/elixir/module/types/type_helper.exs index a852be13295..c56c999bcf2 100644 --- a/lib/elixir/test/elixir/module/types/type_helper.exs +++ b/lib/elixir/test/elixir/module/types/type_helper.exs @@ -138,7 +138,7 @@ defmodule TypeHelper do defp new_stack(mode) do cache = if mode == :infer, do: :none, else: Module.ParallelChecker.test_cache() - handler = fn _, _, _ -> raise "no local lookup" end + handler = fn _, _, _, _ -> raise "no local lookup" end Types.stack(mode, "types_test.ex", TypesTest, {:test, 0}, [], cache, handler) end From 108f9756abcdfc4f2fc5ad1ba1f281b5e172a1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 18:42:13 +0100 Subject: [PATCH 14/22] Remove locals tracker --- lib/elixir/lib/module/locals_tracker.ex | 258 ------------------ lib/elixir/src/elixir_compiler.erl | 1 - lib/elixir/src/elixir_locals.erl | 68 ++--- lib/elixir/src/elixir_module.erl | 3 - .../elixir/module/locals_tracker_test.exs | 135 --------- 5 files changed, 18 insertions(+), 447 deletions(-) delete mode 100644 lib/elixir/lib/module/locals_tracker.ex delete mode 100644 lib/elixir/test/elixir/module/locals_tracker_test.exs diff --git a/lib/elixir/lib/module/locals_tracker.ex b/lib/elixir/lib/module/locals_tracker.ex deleted file mode 100644 index b389bc3d038..00000000000 --- a/lib/elixir/lib/module/locals_tracker.ex +++ /dev/null @@ -1,258 +0,0 @@ -# This is an Elixir module responsible for tracking -# calls in order to extract Elixir modules' behaviour -# during compilation time. -# -# ## Implementation -# -# The implementation uses ETS to track all dependencies -# resembling a graph. The keys and what they point to are: -# -# * `:reattach` points to `{name, arity}` -# * `{:local, {name, arity}}` points to `{{name, arity}, line, macro_dispatch?}` -# * `{:import, {name, arity}}` points to `Module` -# -# This is built on top of the internal module tables. -defmodule Module.LocalsTracker do - @moduledoc false - - @defmacros [:defmacro, :defmacrop] - - @doc """ - Adds and tracks defaults for a definition into the tracker. - """ - def add_defaults({_set, bag}, kind, {name, arity} = pair, defaults, meta) do - for i <- :lists.seq(arity - defaults, arity - 1) do - put_edge(bag, {:local, {name, i}}, {pair, get_line(meta), kind in @defmacros}) - end - - :ok - end - - @doc """ - Adds a local dispatch from-to the given target. - """ - def add_local({_set, bag}, from, to, meta, macro_dispatch?) - when is_tuple(from) and is_tuple(to) and is_boolean(macro_dispatch?) do - put_edge(bag, {:local, from}, {to, get_position(meta), macro_dispatch?}) - :ok - end - - @doc """ - Adds an import dispatch to the given target. - """ - def add_import({set, _bag}, function, module, imported) - when is_tuple(function) and is_atom(module) do - put_edge(set, {:import, imported}, module) - :ok - end - - @doc """ - Yanks a local node. Returns its in and out vertices in a tuple. - """ - def yank({_set, bag}, local) do - :lists.usort(take_out_neighbours(bag, {:local, local})) - end - - @doc """ - Reattach a previously yanked node. - """ - def reattach({_set, bag}, tuple, kind, function, out_neighbours, meta) do - for out_neighbour <- out_neighbours do - put_edge(bag, {:local, function}, out_neighbour) - end - - # Make a call from the old function to the new one - if function != tuple do - put_edge(bag, {:local, function}, {tuple, get_line(meta), kind in @defmacros}) - end - - # Finally marked the new one as reattached - put_edge(bag, :reattach, tuple) - :ok - end - - @doc """ - Collect all conflicting imports with the given functions - """ - def collect_imports_conflicts({set, _bag}, all_defined) do - for {pair, _, meta, _} <- all_defined, n = out_neighbour(set, {:import, pair}) do - {meta, {n, pair}} - end - end - - @doc """ - Collect all unused definitions based on the private - given, also accounting the expected number of default - clauses a private function have. - """ - def collect_unused_locals({_set, bag}, all_defined, private) do - reachable = - Enum.reduce(all_defined, %{}, fn {pair, kind, _, _}, acc -> - if kind in [:def, :defmacro] do - reachable_from(bag, pair, acc) - else - acc - end - end) - - reattached = :lists.usort(out_neighbours(bag, :reattach)) - {unreachable(reachable, reattached, private), collect_warnings(reachable, private)} - end - - @doc """ - Collect undefined functions based on local calls and existing definitions. - """ - def collect_undefined_locals({set, bag}, all_defined, file) do - undefined = - for {pair, _, meta, _} <- all_defined, - {{local_name, _} = local, position, macro_dispatch?} <- - out_neighbours(bag, {:local, pair}), - error = undefined_local_error(set, local, macro_dispatch?) do - file = - case Keyword.get(meta, :file) do - {keep_file, _keep_line} -> keep_file - nil -> file - end - - meta = build_meta(position, local_name) - {pair, meta, file, local, error} - end - - :lists.usort(undefined) - end - - defp undefined_local_error(set, local, true) do - case :ets.member(set, {:def, local}) do - true -> false - false -> :undefined_function - end - end - - defp undefined_local_error(set, local, false) do - try do - if :ets.lookup_element(set, {:def, local}, 2) in @defmacros do - :incorrect_dispatch - else - false - end - catch - _, _ -> :undefined_function - end - end - - defp unreachable(reachable, reattached, private) do - for {tuple, kind, _, _} <- private, - not reachable?(tuple, kind, reachable, reattached), - do: tuple - end - - defp reachable?(tuple, :defmacrop, reachable, reattached) do - # All private macros are unreachable unless they have been - # reattached and they are reachable. - :lists.member(tuple, reattached) and Map.has_key?(reachable, tuple) - end - - defp reachable?(tuple, :defp, reachable, _reattached) do - Map.has_key?(reachable, tuple) - end - - defp collect_warnings(reachable, private) do - :lists.foldl(&collect_warnings(&1, &2, reachable), [], private) - end - - defp collect_warnings({_, _, false, _}, acc, _reachable) do - acc - end - - defp collect_warnings({tuple, kind, meta, 0}, acc, reachable) do - if Map.has_key?(reachable, tuple) do - acc - else - [{meta, {:unused_def, tuple, kind}} | acc] - end - end - - defp collect_warnings({tuple, kind, meta, default}, acc, reachable) when default > 0 do - {name, arity} = tuple - min = arity - default - max = arity - - case min_reachable_default(max, min, :none, name, reachable) do - :none -> [{meta, {:unused_def, tuple, kind}} | acc] - ^min -> acc - ^max -> [{meta, {:unused_args, tuple}} | acc] - diff -> [{meta, {:unused_args, tuple, diff}} | acc] - end - end - - defp min_reachable_default(max, min, last, name, reachable) when max >= min do - case Map.has_key?(reachable, {name, max}) do - true -> min_reachable_default(max - 1, min, max, name, reachable) - false -> min_reachable_default(max - 1, min, last, name, reachable) - end - end - - defp min_reachable_default(_max, _min, last, _name, _reachable) do - last - end - - @doc """ - Returns all local nodes reachable from `vertex`. - - By default, all public functions are reachable. - A private function is only reachable if it has - a public function that it invokes directly. - """ - def reachable_from({_, bag}, local) do - bag - |> reachable_from(local, %{}) - |> Map.keys() - end - - defp reachable_from(bag, local, vertices) do - vertices = Map.put(vertices, local, true) - - Enum.reduce(out_neighbours(bag, {:local, local}), vertices, fn {local, _line, _}, acc -> - case acc do - %{^local => true} -> acc - _ -> reachable_from(bag, local, acc) - end - end) - end - - defp get_line(meta), do: Keyword.get(meta, :line) - defp get_position(meta), do: {get_line(meta), meta[:column]} - - defp build_meta(position, function_name) do - case position do - {line, nil} -> [line: line] - {line, col} -> :elixir_env.calculate_span([line: line, column: col], function_name) - end - end - - ## Lightweight digraph implementation - - defp put_edge(d, from, to) do - :ets.insert(d, {from, to}) - end - - defp out_neighbour(d, from) do - try do - :ets.lookup_element(d, from, 2) - catch - :error, :badarg -> nil - end - end - - defp out_neighbours(d, from) do - try do - :ets.lookup_element(d, from, 2) - catch - :error, :badarg -> [] - end - end - - defp take_out_neighbours(d, from) do - Keyword.values(:ets.take(d, from)) - end -end diff --git a/lib/elixir/src/elixir_compiler.erl b/lib/elixir/src/elixir_compiler.erl index a30731a79cd..8ba322b88fd 100644 --- a/lib/elixir/src/elixir_compiler.erl +++ b/lib/elixir/src/elixir_compiler.erl @@ -190,7 +190,6 @@ bootstrap_files() -> [ <<"list/chars.ex">>, <<"bitwise.ex">>, - <<"module/locals_tracker.ex">>, <<"module/parallel_checker.ex">>, <<"module/behaviour.ex">>, <<"module/types/helpers.ex">>, diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl index ef85bb7f78e..b1a088afeb5 100644 --- a/lib/elixir/src/elixir_locals.erl +++ b/lib/elixir/src/elixir_locals.erl @@ -1,63 +1,28 @@ %% Module responsible for tracking invocations of module calls. -module(elixir_locals). -export([ - setup/1, stop/1, cache_env/1, get_cached_env/1, - record_local/5, record_import/4, record_defaults/5, - yank/2, reattach/6, ensure_no_import_conflict/3, + setup/1, cache_env/1, get_cached_env/1, + record_import/4, + ensure_no_import_conflict/3, format_error/1 ]). -include("elixir.hrl"). -define(cache_key, {elixir, cache_env}). --define(locals_key, {elixir, locals}). --define(locals, 'Elixir.Module.LocalsTracker'). setup({DataSet, _DataBag}) -> ets:insert(DataSet, {?cache_key, 0}), - - case elixir_config:is_bootstrap() of - false -> ets:insert(DataSet, {?locals_key, true}); - true -> ok - end, - ok. -stop({DataSet, _DataBag}) -> - ets:delete(DataSet, ?locals_key). - -yank(Tuple, Module) -> - if_tracker(Module, fun(Tracker) -> ?locals:yank(Tracker, Tuple) end). - -reattach(Tuple, Kind, Module, Function, Neighbours, Meta) -> - if_tracker(Module, fun(Tracker) -> ?locals:reattach(Tracker, Tuple, Kind, Function, Neighbours, Meta) end). - -record_local(_Tuple, _Module, nil, _Meta, _IsMacroDispatch) -> - ok; -record_local(Tuple, Module, Function, Meta, IsMacroDispatch) -> - if_tracker(Module, fun(Tracker) -> ?locals:add_local(Tracker, Function, Tuple, Meta, IsMacroDispatch), ok end). - record_import(_Tuple, Receiver, Module, Function) when Function == nil; Module == Receiver -> false; -record_import(Tuple, Receiver, Module, Function) -> - if_tracker(Module, fun(Tracker) -> ?locals:add_import(Tracker, Function, Receiver, Tuple), ok end). - -record_defaults(_Tuple, _Kind, _Module, 0, _Meta) -> - ok; -record_defaults(Tuple, Kind, Module, Defaults, Meta) -> - if_tracker(Module, fun(Tracker) -> ?locals:add_defaults(Tracker, Kind, Tuple, Defaults, Meta), ok end). - -if_tracker(Module, Callback) -> - if_tracker(Module, ok, Callback). - -if_tracker(Module, Default, Callback) -> +record_import(Tuple, Receiver, Module, _Function) -> try - {DataSet, _} = Tables = elixir_module:data_tables(Module), - {ets:member(DataSet, ?locals_key), Tables} - of - {true, Tracker} -> Callback(Tracker); - {false, _} -> Default + {Set, _Bag} = elixir_module:data_tables(Module), + ets:insert(Set, {{import, Tuple}, Receiver}), + true catch - error:badarg -> Default + error:badarg -> false end. %% CACHING @@ -89,13 +54,16 @@ get_cached_env(Env) -> ensure_no_import_conflict('Elixir.Kernel', _All, _E) -> ok; -ensure_no_import_conflict(Module, All, E) -> - if_tracker(Module, ok, fun(Tracker) -> - [elixir_errors:module_error(Meta, E, ?MODULE, {function_conflict, Error}) - || {Meta, Error} <- ?locals:collect_imports_conflicts(Tracker, All)], - ok - end). +ensure_no_import_conflict(Module, AllDefinitions, E) -> + {Set, _} = elixir_module:data_tables(Module), + + [try + Receiver = ets:lookup_element(Set, {import, Pair}, 2), + elixir_errors:module_error(Meta, E, ?MODULE, {import_conflict, Receiver, Pair}) + catch + error:badarg -> false + end || {Pair, _, Meta, _} <- AllDefinitions]. -format_error({function_conflict, {Receiver, {Name, Arity}}}) -> +format_error({import_conflict, Receiver, {Name, Arity}}) -> io_lib:format("imported ~ts.~ts/~B conflicts with local function", [elixir_aliases:inspect(Receiver), Name, Arity]). diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index 20071fcf209..51bdf82269a 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -148,9 +148,6 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> validate_nifs_attribute(NifsAttribute, AllDefinitions, Line, E), elixir_locals:ensure_no_import_conflict(Module, AllDefinitions, E), - %% We stop tracking locals here to avoid race conditions in case after_load - %% evaluates code in a separate process that may write to locals table. - elixir_locals:stop({DataSet, DataBag}), make_readonly(Module), (not elixir_config:is_bootstrap()) andalso diff --git a/lib/elixir/test/elixir/module/locals_tracker_test.exs b/lib/elixir/test/elixir/module/locals_tracker_test.exs deleted file mode 100644 index d50a2379936..00000000000 --- a/lib/elixir/test/elixir/module/locals_tracker_test.exs +++ /dev/null @@ -1,135 +0,0 @@ -Code.require_file("../test_helper.exs", __DIR__) - -defmodule Module.LocalsTrackerTest do - use ExUnit.Case, async: true - - alias Module.LocalsTracker, as: D - - setup do - set = :ets.new(__MODULE__, [:set, :public]) - bag = :ets.new(__MODULE__, [:duplicate_bag, :public]) - [ref: {set, bag}] - end - - ## Locals - - test "functions are reachable when connected through another one", config do - D.add_local(config[:ref], {:public, 1}, {:private, 1}, [line: 1], false) - assert {:private, 1} in D.reachable_from(config[:ref], {:public, 1}) - end - - test "can yank and reattach nodes", config do - D.add_local(config[:ref], {:foo, 1}, {:bar, 1}, [line: 1], false) - - outfoo = D.yank(config[:ref], {:foo, 1}) - outbar = D.yank(config[:ref], {:bar, 1}) - - D.reattach(config[:ref], {:bar, 1}, :defp, {:bar, 1}, outbar, line: 2) - D.reattach(config[:ref], {:foo, 1}, :def, {:foo, 1}, outfoo, line: 3) - assert {:bar, 1} in D.reachable_from(config[:ref], {:foo, 1}) - end - - @used [ - {{:public, 1}, :def, [], 0} - ] - - test "unused private definitions are marked as so", config do - D.add_local(config[:ref], {:public, 1}, {:private, 1}, [line: 1], false) - - unused = D.collect_unused_locals(config[:ref], @used, [{{:private, 0}, :defp, [], 0}]) - assert unused == {[private: 0], [{[], {:unused_def, {:private, 0}, :defp}}]} - - unused = D.collect_unused_locals(config[:ref], @used, [{{:private, 1}, :defp, [], 0}]) - assert unused == {[], []} - end - - @unused [ - {{:private, 3}, :defp, [], 3} - ] - - test "preserves column information on retrieval", config do - D.add_local(config[:ref], {:public, 1}, {:private, 1}, [line: 1, column: 1], false) - - undefined = D.collect_undefined_locals(config[:ref], @used, "foo.exs") - - assert undefined == [ - {{:public, 1}, [span: {1, 8}, line: 1, column: 1], "foo.exs", {:private, 1}, - :undefined_function} - ] - end - - test "private definitions with unused default arguments", config do - unused = D.collect_unused_locals(config[:ref], @used, @unused) - assert unused == {[private: 3], [{[], {:unused_def, {:private, 3}, :defp}}]} - - D.add_local(config[:ref], {:public, 1}, {:private, 3}, [line: 1], false) - unused = D.collect_unused_locals(config[:ref], @used, @unused) - assert unused == {[], [{[], {:unused_args, {:private, 3}}}]} - end - - test "private definitions with some unused default arguments", config do - D.add_local(config[:ref], {:public, 1}, {:private, 1}, [line: 1], false) - unused = D.collect_unused_locals(config[:ref], @used, @unused) - assert unused == {[private: 3], [{[], {:unused_args, {:private, 3}, 1}}]} - end - - test "private definitions with all used default arguments", config do - D.add_local(config[:ref], {:public, 1}, {:private, 0}, [line: 1], false) - unused = D.collect_unused_locals(config[:ref], @used, @unused) - assert unused == {[private: 3], []} - end - - ### Undefined functions - - test "undefined functions are marked as so", config do - D.add_local(config[:ref], {:public, 1}, {:private, 1}, [line: 1], false) - - undefined = D.collect_undefined_locals(config[:ref], @used, "foo.exs") - assert undefined == [{{:public, 1}, [line: 1], "foo.exs", {:private, 1}, :undefined_function}] - end - - ### Incorrect dispatches - - test "incorrect dispatches are marked as so", config do - {set, _bag} = config[:ref] - :ets.insert(set, {{:def, {:macro, 1}}, :defmacro, [], "nofile", false, {0, true, 0}}) - definitions = [{{:public, 1}, :def, [], 0}, {{:macro, 1}, :defmacro, [], 0}] - - D.add_local(config[:ref], {:public, 1}, {:macro, 1}, [line: 5], false) - - undefined = D.collect_undefined_locals(config[:ref], definitions, "foo.exs") - assert undefined == [{{:public, 1}, [line: 5], "foo.exs", {:macro, 1}, :incorrect_dispatch}] - end - - ## Defaults - - test "defaults are connected to last clause only", config do - D.add_defaults(config[:ref], :defp, {:foo, 4}, 2, line: 1) - D.add_local(config[:ref], {:public, 1}, {:foo, 2}, [line: 2], false) - assert {:foo, 2} in D.reachable_from(config[:ref], {:public, 1}) - refute {:foo, 3} in D.reachable_from(config[:ref], {:public, 1}) - assert {:foo, 4} in D.reachable_from(config[:ref], {:public, 1}) - end - - ## Imports - - test "find import conflicts", config do - entries = [{{:conflict, 1}, :def, [], []}] - refute {[], {Module, {:conflict, 1}}} in D.collect_imports_conflicts(config[:ref], entries) - - D.add_local(config[:ref], {:public, 1}, {:foo, 2}, [line: 1], false) - D.add_import(config[:ref], {:foo, 2}, Module, {:conflict, 1}) - D.add_import(config[:ref], {:foo, 2}, Module, {:conflict, 1}) - assert {[], {Module, {:conflict, 1}}} in D.collect_imports_conflicts(config[:ref], entries) - end - - defmodule NoPrivate do - defmacrop foo(), do: bar() - defp bar(), do: :baz - def baz(), do: foo() - end - - test "does not include unreachable locals" do - assert NoPrivate.module_info(:functions) |> Keyword.take([:foo, :bar, :"MACRO-foo"]) == [] - end -end From 6c9602b2ecbd31005e739f20aebf726edef54b05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 18:53:58 +0100 Subject: [PATCH 15/22] Get rid of elixir_locals too --- lib/elixir/lib/kernel.ex | 4 +- lib/elixir/lib/kernel/cli.ex | 2 +- lib/elixir/lib/kernel/typespec.ex | 10 ++--- lib/elixir/src/elixir_bootstrap.erl | 2 +- lib/elixir/src/elixir_def.erl | 4 +- lib/elixir/src/elixir_dispatch.erl | 6 +-- lib/elixir/src/elixir_import.erl | 35 ++++++++++++++- lib/elixir/src/elixir_locals.erl | 69 ----------------------------- lib/elixir/src/elixir_module.erl | 33 +++++++++++--- 9 files changed, 74 insertions(+), 91 deletions(-) delete mode 100644 lib/elixir/src/elixir_locals.erl diff --git a/lib/elixir/lib/kernel.ex b/lib/elixir/lib/kernel.ex index 0618f97db96..7a5237a9db0 100644 --- a/lib/elixir/lib/kernel.ex +++ b/lib/elixir/lib/kernel.ex @@ -3646,7 +3646,7 @@ defmodule Kernel do :ok true -> - pos = :elixir_locals.cache_env(__CALLER__) + pos = :elixir_module.cache_env(__CALLER__) %{line: line, file: file, module: module} = __CALLER__ quote do @@ -5323,7 +5323,7 @@ defmodule Kernel do key end - pos = :elixir_locals.cache_env(env) + pos = :elixir_module.cache_env(env) quote do :elixir_def.store_definition(unquote(kind), unquote(store), unquote(pos)) diff --git a/lib/elixir/lib/kernel/cli.ex b/lib/elixir/lib/kernel/cli.ex index 8a38fba5e48..c4b302b236b 100644 --- a/lib/elixir/lib/kernel/cli.ex +++ b/lib/elixir/lib/kernel/cli.ex @@ -196,7 +196,7 @@ defmodule Kernel.CLI do end @elixir_internals [:elixir, :elixir_aliases, :elixir_expand, :elixir_compiler, :elixir_module] ++ - [:elixir_clauses, :elixir_lexical, :elixir_def, :elixir_map, :elixir_locals] ++ + [:elixir_clauses, :elixir_lexical, :elixir_def, :elixir_map] ++ [:elixir_erl, :elixir_erl_clauses, :elixir_erl_compiler, :elixir_erl_pass] ++ [Kernel.ErrorHandler, Module.ParallelChecker] diff --git a/lib/elixir/lib/kernel/typespec.ex b/lib/elixir/lib/kernel/typespec.ex index 8dc5c0f8b61..942cd23455f 100644 --- a/lib/elixir/lib/kernel/typespec.ex +++ b/lib/elixir/lib/kernel/typespec.ex @@ -138,7 +138,7 @@ defmodule Kernel.Typespec do case type_to_signature(expr) do {name, arity} = signature when signature in @reserved_signatures -> compile_error( - :elixir_locals.get_cached_env(pos), + :elixir_module.get_cached_env(pos), "type #{name}/#{arity} is a reserved type and it cannot be defined" ) @@ -247,7 +247,7 @@ defmodule Kernel.Typespec do defp collect_defined_type_pairs(type_typespecs) do fun = fn {_kind, expr, pos}, type_pairs -> - %{file: file, line: line} = env = :elixir_locals.get_cached_env(pos) + %{file: file, line: line} = env = :elixir_module.get_cached_env(pos) case type_to_signature(expr) do {name, arity} = type_pair -> @@ -292,7 +292,7 @@ defmodule Kernel.Typespec do defp translate_type({kind, {:"::", _, [{name, meta, args}, definition]}, pos}, state) when is_list(meta) do - caller = :elixir_locals.get_cached_env(pos) + caller = :elixir_module.get_cached_env(pos) state = clean_local_state(state) args = @@ -349,12 +349,12 @@ defmodule Kernel.Typespec do defp underspecified?(_kind, _arity, _spec), do: false defp translate_spec({kind, {:when, _meta, [spec, guard]}, pos}, state) do - caller = :elixir_locals.get_cached_env(pos) + caller = :elixir_module.get_cached_env(pos) translate_spec(kind, spec, guard, caller, state) end defp translate_spec({kind, spec, pos}, state) do - caller = :elixir_locals.get_cached_env(pos) + caller = :elixir_module.get_cached_env(pos) translate_spec(kind, spec, [], caller, state) end diff --git a/lib/elixir/src/elixir_bootstrap.erl b/lib/elixir/src/elixir_bootstrap.erl index c24dfc38892..3d273ff280e 100644 --- a/lib/elixir/src/elixir_bootstrap.erl +++ b/lib/elixir/src/elixir_bootstrap.erl @@ -50,7 +50,7 @@ define({Line, _S, #{module := Module} = E}, Kind, Call, Expr) -> Key end, - Args = [Kind, Store, elixir_locals:cache_env(E#{line := Line})], + Args = [Kind, Store, elixir_module:cache_env(E#{line := Line})], {{'.', [], [elixir_def, store_definition]}, [], Args}. unless_loaded(Fun, Args, Callback) -> diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl index b6820df30bb..9d64565c5ba 100644 --- a/lib/elixir/src/elixir_def.erl +++ b/lib/elixir/src/elixir_def.erl @@ -135,10 +135,10 @@ head_and_definition_meta(_, _Meta, _HeadDefaults, [{_, _, HeadMeta, _} | _]) -> %% Section for storing definitions store_definition(Kind, {Call, Body}, Pos) -> - E = elixir_locals:get_cached_env(Pos), + E = elixir_module:get_cached_env(Pos), store_definition(Kind, false, Call, Body, E); store_definition(Kind, Key, Pos) -> - #{module := Module} = E = elixir_locals:get_cached_env(Pos), + #{module := Module} = E = elixir_module:get_cached_env(Pos), {Call, Body} = elixir_module:read_cache(Module, Key), store_definition(Kind, true, Call, Body, E). diff --git a/lib/elixir/src/elixir_dispatch.erl b/lib/elixir/src/elixir_dispatch.erl index 990aff4b616..8fd5a6d5b04 100644 --- a/lib/elixir/src/elixir_dispatch.erl +++ b/lib/elixir/src/elixir_dispatch.erl @@ -55,7 +55,7 @@ import_function(Meta, Name, Arity, E) -> case find_import_by_name_arity(Meta, Tuple, [], E) of {function, Receiver} -> elixir_env:trace({imported_function, Meta, Receiver, Name, Arity}, E), - elixir_locals:record_import(Tuple, Receiver, ?key(E, module), ?key(E, function)), + elixir_import:record(Tuple, Receiver, ?key(E, module), ?key(E, function)), remote_function(Meta, Receiver, Name, Arity, E); {macro, _Receiver} -> false; @@ -192,13 +192,13 @@ do_expand_import(Result, Meta, Name, Arity, Module, E, Trace) -> {function, Receiver} -> Trace andalso begin elixir_env:trace({imported_function, Meta, Receiver, Name, Arity}, E), - elixir_locals:record_import({Name, Arity}, Receiver, Module, ?key(E, function)) + elixir_import:record({Name, Arity}, Receiver, Module, ?key(E, function)) end, {function, Receiver, Name}; {macro, Receiver} -> Trace andalso begin elixir_env:trace({imported_macro, Meta, Receiver, Name, Arity}, E), - elixir_locals:record_import({Name, Arity}, Receiver, Module, ?key(E, function)) + elixir_import:record({Name, Arity}, Receiver, Module, ?key(E, function)) end, {macro, Receiver, expander_macro_named(Meta, Receiver, Name, Arity, E)}; {import, Receiver} -> diff --git a/lib/elixir/src/elixir_import.erl b/lib/elixir/src/elixir_import.erl index b979066c663..0327549edb8 100644 --- a/lib/elixir/src/elixir_import.erl +++ b/lib/elixir/src/elixir_import.erl @@ -2,7 +2,9 @@ %% between local functions and imports. %% For imports dispatch, please check elixir_dispatch. -module(elixir_import). --export([import/6, import/7, special_form/2, format_error/1]). +-export([import/6, import/7, special_form/2, + record/4, ensure_no_local_conflict/3, + format_error/1]). -compile(inline_list_funcs). -include("elixir.hrl"). @@ -135,6 +137,31 @@ calculate_key(Meta, Key, Old, New, E, Warn) -> {true, FinalSet, [{Key, FinalSet} | keydelete(Key, Old)]} end. +%% Record function calls for local conflicts + +record(_Tuple, Receiver, Module, Function) + when Function == nil; Module == Receiver -> false; +record(Tuple, Receiver, Module, _Function) -> + try + {Set, _Bag} = elixir_module:data_tables(Module), + ets:insert(Set, {{import, Tuple}, Receiver}), + true + catch + error:badarg -> false + end. + +ensure_no_local_conflict('Elixir.Kernel', _All, _E) -> + ok; +ensure_no_local_conflict(Module, AllDefinitions, E) -> + {Set, _} = elixir_module:data_tables(Module), + + [try + Receiver = ets:lookup_element(Set, {import, Pair}, 2), + elixir_errors:module_error(Meta, E, ?MODULE, {import_conflict, Receiver, Pair}) + catch + error:badarg -> false + end || {Pair, _, Meta, _} <- AllDefinitions]. + %% Retrieve functions and macros from modules get_functions(Module, InfoCallback) -> @@ -229,7 +256,11 @@ format_error({special_form_conflict, {Receiver, Name, Arity}}) -> [elixir_aliases:inspect(Receiver), Name, Arity]); format_error({no_macros, Module}) -> - io_lib:format("could not load macros from module ~ts", [elixir_aliases:inspect(Module)]). + io_lib:format("could not load macros from module ~ts", [elixir_aliases:inspect(Module)]); + +format_error({import_conflict, Receiver, {Name, Arity}}) -> + io_lib:format("imported ~ts.~ts/~B conflicts with local function", + [elixir_aliases:inspect(Receiver), Name, Arity]). %% LIST HELPERS diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl deleted file mode 100644 index b1a088afeb5..00000000000 --- a/lib/elixir/src/elixir_locals.erl +++ /dev/null @@ -1,69 +0,0 @@ -%% Module responsible for tracking invocations of module calls. --module(elixir_locals). --export([ - setup/1, cache_env/1, get_cached_env/1, - record_import/4, - ensure_no_import_conflict/3, - format_error/1 -]). - --include("elixir.hrl"). --define(cache_key, {elixir, cache_env}). - -setup({DataSet, _DataBag}) -> - ets:insert(DataSet, {?cache_key, 0}), - ok. - -record_import(_Tuple, Receiver, Module, Function) - when Function == nil; Module == Receiver -> false; -record_import(Tuple, Receiver, Module, _Function) -> - try - {Set, _Bag} = elixir_module:data_tables(Module), - ets:insert(Set, {{import, Tuple}, Receiver}), - true - catch - error:badarg -> false - end. - -%% CACHING - -cache_env(#{line := Line, module := Module} = E) -> - {Set, _} = elixir_module:data_tables(Module), - Cache = elixir_env:reset_vars(E#{line := nil}), - PrevKey = ets:lookup_element(Set, ?cache_key, 2), - - Pos = - case ets:lookup(Set, {cache_env, PrevKey}) of - [{_, Cache}] -> - PrevKey; - _ -> - NewKey = PrevKey + 1, - ets:insert(Set, [{{cache_env, NewKey}, Cache}, {?cache_key, NewKey}]), - NewKey - end, - - {Module, {Line, Pos}}. - -get_cached_env({Module, {Line, Pos}}) -> - {Set, _} = elixir_module:data_tables(Module), - (ets:lookup_element(Set, {cache_env, Pos}, 2))#{line := Line}; -get_cached_env(Env) -> - Env. - -%% ERROR HANDLING - -ensure_no_import_conflict('Elixir.Kernel', _All, _E) -> - ok; -ensure_no_import_conflict(Module, AllDefinitions, E) -> - {Set, _} = elixir_module:data_tables(Module), - - [try - Receiver = ets:lookup_element(Set, {import, Pair}, 2), - elixir_errors:module_error(Meta, E, ?MODULE, {import_conflict, Receiver, Pair}) - catch - error:badarg -> false - end || {Pair, _, Meta, _} <- AllDefinitions]. - -format_error({import_conflict, Receiver, {Name, Arity}}) -> - io_lib:format("imported ~ts.~ts/~B conflicts with local function", - [elixir_aliases:inspect(Receiver), Name, Arity]). diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index 51bdf82269a..f033d6f486e 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -1,9 +1,10 @@ -module(elixir_module). -export([file/1, data_tables/1, is_open/1, mode/1, delete_definition_attributes/6, compile/6, expand_callback/6, format_error/1, compiler_modules/0, - write_cache/3, read_cache/2, next_counter/1, taint/1]). + write_cache/3, read_cache/2, next_counter/1, taint/1, cache_env/1, get_cached_env/1]). -include("elixir.hrl"). -define(counter_attr, {elixir, counter}). +-define(cache_key, {elixir, cache_env}). %% Stores modules currently being defined by the compiler @@ -71,6 +72,29 @@ taint(Module) -> _:_ -> false end. +cache_env(#{line := Line, module := Module} = E) -> + {Set, _} = data_tables(Module), + Cache = elixir_env:reset_vars(E#{line := nil}), + PrevKey = ets:lookup_element(Set, ?cache_key, 2), + + Pos = + case ets:lookup(Set, {cache_env, PrevKey}) of + [{_, Cache}] -> + PrevKey; + _ -> + NewKey = PrevKey + 1, + ets:insert(Set, [{{cache_env, NewKey}, Cache}, {?cache_key, NewKey}]), + NewKey + end, + + {Module, {Line, Pos}}. + +get_cached_env({Module, {Line, Pos}}) -> + {Set, _} = data_tables(Module), + (ets:lookup_element(Set, {cache_env, Pos}, 2))#{line := Line}; +get_cached_env(Env) -> + Env. + %% Compilation hook compile(Meta, Module, Block, Vars, Prune, Env) -> @@ -146,7 +170,7 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> NifsAttribute = lists:keyfind(nifs, 1, Attributes), validate_nifs_attribute(NifsAttribute, AllDefinitions, Line, E), - elixir_locals:ensure_no_import_conflict(Module, AllDefinitions, E), + elixir_import:ensure_no_local_conflict(Module, AllDefinitions, E), make_readonly(Module), @@ -330,7 +354,6 @@ build(Module, Line, File, E) -> %% * {{type, Tuple}, ...}, {{opaque, Tuple}, ...} %% * {{callback, Tuple}, ...}, {{macrocallback, Tuple}, ...} %% * {{def, Tuple}, ...} (from elixir_def) - %% * {{import, Tuple}, ...} (from elixir_locals) %% * {{overridable, Tuple}, ...} (from elixir_overridable) %% DataSet = ets:new(Module, [set, public]), @@ -346,8 +369,6 @@ build(Module, Line, File, E) -> %% * {overridables, ...} (from elixir_overridable) %% * {{default, Name}, ...} (from elixir_def) %% * {{clauses, Tuple}, ...} (from elixir_def) - %% * {reattach, ...} (from elixir_locals) - %% * {{local, Tuple}, ...} (from elixir_locals) %% DataBag = ets:new(Module, [duplicate_bag, public]), @@ -374,6 +395,7 @@ build(Module, Line, File, E) -> {optional_callbacks, [], accumulate, []}, % Others + {?cache_key, 0}, {?counter_attr, 0} ]), @@ -390,7 +412,6 @@ build(Module, Line, File, E) -> %% Setup definition related modules Tables = {DataSet, DataBag}, elixir_def:setup(Tables), - elixir_locals:setup(Tables), Tuple = {Module, Tables, Line, File, all}, Ref = From a08de459434b27f655d33583c4079deb0a3fa067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 19:25:16 +0100 Subject: [PATCH 16/22] Clean up --- lib/elixir/lib/module/parallel_checker.ex | 17 +--- lib/elixir/lib/module/types.ex | 2 - lib/elixir/src/elixir_module.erl | 89 +++++++++++-------- lib/elixir/test/elixir/kernel/errors_test.exs | 4 +- 4 files changed, 58 insertions(+), 54 deletions(-) diff --git a/lib/elixir/lib/module/parallel_checker.ex b/lib/elixir/lib/module/parallel_checker.ex index 2ccd112b8cf..3fbab3b6c44 100644 --- a/lib/elixir/lib/module/parallel_checker.ex +++ b/lib/elixir/lib/module/parallel_checker.ex @@ -47,20 +47,9 @@ defmodule Module.ParallelChecker do @doc """ Spawns a process that runs the parallel checker. """ - def spawn(pid_checker, module_map, log?, private, used, env) do - %{module: module, definitions: definitions, file: file} = module_map - - {signatures, unreachable} = - Module.Types.infer(module, file, definitions, private, used, env) - - module_map = %{module_map | signatures: signatures, unreachable: unreachable} - - with {pid, checker} <- pid_checker do - ets = :gen_server.call(checker, :ets, :infinity) - inner_spawn(pid, checker, module, cache_from_module_map(ets, module_map), log?) - end - - module_map + def spawn({pid, checker}, module, module_map, log?) do + ets = :gen_server.call(checker, :ets, :infinity) + inner_spawn(pid, checker, module, cache_from_module_map(ets, module_map), log?) end defp inner_spawn(pid, checker, module, info, log?) do diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index a215062d524..630127397ef 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -1,8 +1,6 @@ defmodule Module.Types do @moduledoc false - alias Module.Types.{Descr, Expr, Pattern, Helpers} - # TODO: Consider passing inferred types from infer into warnings # These functions are not inferred because they are added/managed by the compiler @no_infer [__protocol__: 1, behaviour_info: 1] diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index f033d6f486e..e887db3f3c8 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -171,21 +171,31 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> NifsAttribute = lists:keyfind(nifs, 1, Attributes), validate_nifs_attribute(NifsAttribute, AllDefinitions, Line, E), elixir_import:ensure_no_local_conflict(Module, AllDefinitions, E), - make_readonly(Module), (not elixir_config:is_bootstrap()) andalso 'Elixir.Module':'__check_attributes__'(E, DataSet, DataBag), - RawCompileOpts = bag_lookup_element(DataBag, {accumulate, compile}, 2), - CompileOpts = validate_compile_opts(RawCompileOpts, AllDefinitions, Line, E), - Impls = bag_lookup_element(DataBag, impls, 2), - AfterVerify = bag_lookup_element(DataBag, {accumulate, after_verify}, 2), [elixir_env:trace({remote_function, [], VerifyMod, VerifyFun, 1}, CallbackE) || {VerifyMod, VerifyFun} <- AfterVerify], - PartialModuleMap = #{ + %% Ensure there are no errors before we infer types + compile_error_if_tainted(DataSet, E), + + {Signatures, Unreachable} = + case elixir_config:is_bootstrap() of + true -> {#{}, []}; + false -> + Used = bag_lookup_element(DataBag, macro_private_calls, 2), + 'Elixir.Module.Types':infer(Module, File, AllDefinitions, NewPrivate, Used, E) + end, + + RawCompileOpts = bag_lookup_element(DataBag, {accumulate, compile}, 2), + CompileOpts = validate_compile_opts(RawCompileOpts, AllDefinitions, Unreachable, Line, E), + Impls = bag_lookup_element(DataBag, impls, 2), + + ModuleMap = #{ struct => get_struct(DataSet), module => Module, anno => Anno, @@ -198,17 +208,14 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> deprecated => get_deprecated(DataBag), defines_behaviour => defines_behaviour(DataBag), impls => Impls, - unreachable => [], - signatures => #{} + unreachable => Unreachable, + signatures => Signatures }, - %% Compute signatures only if the module is valid. - compile_error_if_tainted(DataSet, E), - ModuleMap = spawn_parallel_checker(DataBag, CheckerInfo, PartialModuleMap, NewPrivate, E), compile_error_if_tainted(DataSet, E), - Binary = elixir_erl:compile(ModuleMap), Autoload = proplists:get_value(autoload, CompileOpts, true), + spawn_parallel_checker(CheckerInfo, Module, ModuleMap), {Binary, PersistedAttributes, Autoload} end), @@ -218,7 +225,7 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> elixir_env:trace({on_module, Binary, none}, ModuleE), warn_unused_attributes(DataSet, DataBag, PersistedAttributes, E), make_module_available(Module, Binary), - (CheckerInfo == nil) andalso + (CheckerInfo == undefined) andalso [VerifyMod:VerifyFun(Module) || {VerifyMod, VerifyFun} <- bag_lookup_element(DataBag, {accumulate, after_verify}, 2)], {module, Module, Binary, Result} @@ -245,27 +252,41 @@ compile_error_if_tainted(DataSet, E) -> false -> ok end. -validate_compile_opts(Opts, Defs, Line, E) -> - lists:flatmap(fun (Opt) -> validate_compile_opt(Opt, Defs, Line, E) end, Opts). +validate_compile_opts(Opts, Defs, Unreachable, Line, E) -> + lists:flatmap(fun (Opt) -> validate_compile_opt(Opt, Defs, Unreachable, Line, E) end, Opts). %% TODO: Make this an error on v2.0 -validate_compile_opt({parse_transform, Module} = Opt, _Defs, Line, E) -> +validate_compile_opt({parse_transform, Module} = Opt, _Defs, _Unreachable, Line, E) -> elixir_errors:file_warn([{line, Line}], E, ?MODULE, {parse_transform, Module}), [Opt]; -validate_compile_opt({inline, Inlines} = Opt, Defs, Line, E) -> - [case lists:keyfind(Inline, 1, Defs) of +validate_compile_opt({inline, Inlines}, Defs, Unreachable, Line, E) -> + case validate_inlines(Inlines, Defs, Unreachable, []) of + {ok, []} -> + []; + {ok, FilteredInlines} -> + [{inline, FilteredInlines}]; + {error, Reason} -> + elixir_errors:module_error([{line, Line}], E, ?MODULE, Reason), + [] + end; +validate_compile_opt(Opt, Defs, Unreachable, Line, E) when is_list(Opt) -> + validate_compile_opts(Opt, Defs, Unreachable, Line, E); +validate_compile_opt(Opt, _Defs, _Unreachable, _Line, _E) -> + [Opt]. + +validate_inlines([Inline | Inlines], Defs, Unreachable, Acc) -> + case lists:keyfind(Inline, 1, Defs) of false -> - elixir_errors:module_error([{line, Line}], E, ?MODULE, {undefined_function, {compile, inline}, Inline}); + {error, {undefined_function, {compile, inline}, Inline}}; {_Def, Type, _Meta, _Clauses} when Type == defmacro; Type == defmacrop -> - elixir_errors:module_error([{line, Line}], E, ?MODULE, {bad_macro, {compile, inline}, Inline}); + {error, {bad_macro, {compile, inline}, Inline}}; _ -> - ok - end || Inline <- Inlines], - [Opt]; -validate_compile_opt(Opt, Defs, Line, E) when is_list(Opt) -> - validate_compile_opts(Opt, Defs, Line, E); -validate_compile_opt(Opt, _Defs, _Line, _E) -> - [Opt]. + case lists:member(Inline, Unreachable) of + true -> validate_inlines(Inlines, Defs, Unreachable, Acc); + false -> validate_inlines(Inlines, Defs, Unreachable, [Inline | Acc]) + end + end; +validate_inlines([], _Defs, _Unreachable, Acc) -> {ok, Acc}. validate_on_load_attribute({on_load, Def}, Defs, Private, Line, E) -> case lists:keyfind(Def, 1, Defs) of @@ -534,23 +555,19 @@ beam_location(ModuleAsCharlist) -> checker_info() -> case get(elixir_checker_info) of - undefined -> nil; + undefined -> undefined; _ -> 'Elixir.Module.ParallelChecker':get() end. -spawn_parallel_checker(DataBag, CheckerInfo, ModuleMap, Private, E) -> +spawn_parallel_checker(undefined, _Module, _ModuleMap) -> + ok; +spawn_parallel_checker(CheckerInfo, Module, ModuleMap) -> Log = case erlang:get(elixir_code_diagnostics) of {_, false} -> false; _ -> true end, - - Used = bag_lookup_element(DataBag, macro_private_calls, 2), - - case elixir_config:is_bootstrap() of - true -> ModuleMap; - false -> 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, ModuleMap, Log, Private, Used, E) - end. + 'Elixir.Module.ParallelChecker':spawn(CheckerInfo, Module, ModuleMap, Log). make_module_available(Module, Binary) -> case get(elixir_module_binaries) of diff --git a/lib/elixir/test/elixir/kernel/errors_test.exs b/lib/elixir/test/elixir/kernel/errors_test.exs index 8b18d752598..48905a17be8 100644 --- a/lib/elixir/test/elixir/kernel/errors_test.exs +++ b/lib/elixir/test/elixir/kernel/errors_test.exs @@ -739,8 +739,8 @@ defmodule Kernel.ErrorsTest do ) assert_compile_error( - ["nofile:1: ", "undefined function foo/1 given to @compile :inline"], - ~c"defmodule Test do @compile {:inline, foo: 1}; defmacro foo(_) end" + ["nofile:1: ", "macro foo/1 given to @compile :inline"], + ~c"defmodule Test do @compile {:inline, foo: 1}; defmacro foo(_), do: :ok end" ) end From 6fabacaff36e6bff006f69b755a0e7c5c255b4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 21:40:25 +0100 Subject: [PATCH 17/22] Optimize macro traversals --- lib/elixir/lib/module/types.ex | 62 +++++++++++++------ lib/elixir/lib/module/types/apply.ex | 26 ++++++-- lib/elixir/lib/module/types/expr.ex | 29 ++++++--- lib/elixir/lib/module/types/pattern.ex | 12 +++- lib/elixir/src/elixir_def.erl | 16 +++-- lib/elixir/src/elixir_dispatch.erl | 10 +-- lib/elixir/src/elixir_map.erl | 2 +- lib/elixir/src/elixir_module.erl | 4 +- .../test/elixir/kernel/warning_test.exs | 31 ---------- 9 files changed, 114 insertions(+), 78 deletions(-) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 630127397ef..75c531a652a 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -6,7 +6,8 @@ defmodule Module.Types do @no_infer [__protocol__: 1, behaviour_info: 1] @doc false - def infer(module, file, defs, private, used, env) do + def infer(module, file, defs, private, defmacrop, env) do + defmacrop = Map.from_keys(defmacrop, []) finder = &:lists.keyfind(&1, 1, defs) handler = fn meta, fun_arity, stack, context -> @@ -29,23 +30,38 @@ defmodule Module.Types do context = context(%{}) {types, %{local_sigs: local_sigs}} = - for {fun_arity, kind, meta, _clauses} = def <- defs, - kind == :def or kind == :defmacro, - reduce: {[], context} do + for {fun_arity, kind, meta, clauses} = def <- defs, reduce: {[], context} do {types, context} -> - {_kind, inferred, context} = - local_handler(meta, fun_arity, stack, context, fn _ -> def end) - - if kind == :def and fun_arity not in @no_infer do - {[{fun_arity, inferred} | types], context} - else - {types, context} + cond do + kind in [:def, :defmacro] -> + {_kind, inferred, context} = + local_handler(meta, fun_arity, stack, context, fn _ -> def end) + + if kind == :def and fun_arity not in @no_infer do + {[{fun_arity, inferred} | types], context} + else + {types, context} + end + + kind == :defmacrop and is_map_key(defmacrop, fun_arity) -> + # Bypass the caching structure for defmacrop, that's because + # we don't need them stored in the signatures when we perform + # unreachable checks. This may cause defmacrop to be traversed + # twice if it uses default arguments (which is the only way + # to refer to another defmacrop in definitions). + {_kind, _inferred, context} = + local_handler(fun_arity, kind, meta, clauses, stack, context) + + {types, context} + + true -> + {types, context} end end unreachable = for {fun_arity, _kind, _meta, _defaults} = info <- private, - warn_unused_def(info, local_sigs, used, env), + warn_unused_def(info, local_sigs, defmacrop, env), not is_map_key(local_sigs, fun_arity), do: fun_arity @@ -63,7 +79,7 @@ defmodule Module.Types do end defp warn_unused_def({fun_arity, kind, meta, 0}, reachable, used, env) do - case meta == false or Map.has_key?(reachable, fun_arity) or fun_arity in used do + case is_map_key(reachable, fun_arity) or is_map_key(used, fun_arity) do true -> :ok false -> :elixir_errors.file_warn(meta, env, __MODULE__, {:unused_def, fun_arity, kind}) end @@ -89,7 +105,7 @@ defmodule Module.Types do defp min_reachable_default(max, min, last, name, reachable, used) when max >= min do fun_arity = {name, max} - case Map.has_key?(reachable, fun_arity) or fun_arity in used do + case is_map_key(reachable, fun_arity) or is_map_key(used, fun_arity) do true -> min_reachable_default(max - 1, min, max, name, reachable, used) false -> min_reachable_default(max - 1, min, last, name, reachable, used) end @@ -164,7 +180,7 @@ defmodule Module.Types do defp local_handler({fun, arity} = fun_arity, kind, meta, clauses, stack, context) do expected = List.duplicate(Descr.dynamic(), arity) - stack = stack |> fresh_stack(fun_arity) |> with_file_meta(meta) + stack = stack |> fresh_stack(kind, fun_arity) |> with_file_meta(meta) {_, _, mapping, clauses_types, clauses_context} = Enum.reduce(clauses, {0, 0, [], [], context}, fn @@ -243,7 +259,7 @@ defmodule Module.Types do @doc false def stack(mode, file, module, function, no_warn_undefined, cache, handler) - when mode in [:static, :dynamic, :infer] do + when mode in [:static, :dynamic, :infer, :traversal] do %{ # The fallback meta used for literals in patterns and guards meta: [], @@ -275,6 +291,9 @@ defmodule Module.Types do # # * :infer - Same as :dynamic but skips remote calls. # + # * :traversal - Focused mostly on traversing AST, skips most type system + # operations. Used by macros and functions which already have signatures. + # # The mode may also control exhaustiveness checks in the future (to be decided). # We may also want for applications with subtyping in dynamic mode to always # intersect with dynamic, but this mode may be too lax (to be decided based on @@ -303,8 +322,15 @@ defmodule Module.Types do } end - defp fresh_stack(stack, function) do - %{stack | function: function} + defp fresh_stack(%{mode: mode} = stack, kind, function) do + mode = + cond do + kind in [:defmacro, :defmacrop] and mode == :infer -> :traversal + kind in [:def, :defp] and mode == :traversal -> :infer + true -> mode + end + + %{stack | function: function, mode: mode} end defp fresh_context(context) do diff --git a/lib/elixir/lib/module/types/apply.ex b/lib/elixir/lib/module/types/apply.ex index 55f9fe97783..5c6312b841c 100644 --- a/lib/elixir/lib/module/types/apply.ex +++ b/lib/elixir/lib/module/types/apply.ex @@ -2,7 +2,11 @@ defmodule Module.Types.Apply do # Typing functionality shared between Expr and Pattern. # Generic AST and Enum helpers go to Module.Types.Helpers. @moduledoc false - @max_clauses 32 + + # We limit the size of the union for two reasons: + # To avoid really large outputs in reports and to + # reduce the computation cost of inferred code. + @max_clauses 16 alias Module.ParallelChecker import Module.Types.{Helpers, Descr} @@ -188,7 +192,6 @@ defmodule Module.Types.Apply do {:erlang, :list_to_float, [{[non_empty_list(integer())], float()}]}, {:erlang, :list_to_integer, [{[non_empty_list(integer())], integer()}]}, {:erlang, :list_to_integer, [{[non_empty_list(integer()), integer()], integer()}]}, - {:erlang, :list_to_tuple, [{[list(term())], dynamic(open_tuple([]))}]}, {:erlang, :make_ref, [{[], reference()}]}, {:erlang, :map_size, [{[open_map()], integer()}]}, {:erlang, :node, [{[], atom()}]}, @@ -217,6 +220,7 @@ defmodule Module.Types.Apply do {:erlang, :element, [{[integer(), open_tuple([])], dynamic()}]}, {:erlang, :insert_element, [{[integer(), open_tuple([]), term()], dynamic(open_tuple([]))}]}, + {:erlang, :list_to_tuple, [{[list(term())], dynamic(open_tuple([]))}]}, {:erlang, :max, [{[term(), term()], dynamic()}]}, {:erlang, :min, [{[term(), term()], dynamic()}]}, {:erlang, :orelse, [{[boolean(), term()], dynamic()}]}, @@ -256,6 +260,10 @@ defmodule Module.Types.Apply do Used only by info functions. """ + def remote(_name, _args_types, _expr, %{mode: :traversal}, context) do + {dynamic(), context} + end + def remote(name, args_types, expr, stack, context) do arity = length(args_types) @@ -268,6 +276,10 @@ defmodule Module.Types.Apply do @doc """ Applies a function in a given module. """ + def remote(_module, _fun, _args_types, _expr, %{mode: :traversal}, context) do + {dynamic(), context} + end + def remote(:erlang, :element, [_, tuple], {_, meta, [index, _]} = expr, stack, context) when is_integer(index) do case tuple_fetch(tuple, index - 1) do @@ -515,11 +527,14 @@ defmodule Module.Types.Apply do false -> {dynamic(), context} + {_kind, _info, context} when stack.mode == :traversal -> + {dynamic(), context} + {kind, info, context} -> case apply_signature(info, args_types, stack) do {:ok, indexes, type} -> context = - if stack != :infer and kind == :defp do + if stack.mode != :infer and kind == :defp do update_in(context.local_used[fun_arity], fn current -> if info == :none do [] @@ -554,10 +569,13 @@ defmodule Module.Types.Apply do case stack.local_handler.(meta, fun_arity, stack, context) do false -> + {dynamic(fun()), context} + + {_kind, _info, context} when stack.mode == :traversal -> {fun(), context} {kind, _info, context} -> - if stack != :infer and kind == :defp do + if stack.mode != :infer and kind == :defp do # Mark all clauses as used, as the function is being exported. {fun(), put_in(context.local_used[fun_arity], [])} else diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex index fb8d5bacf55..6705f94afd7 100644 --- a/lib/elixir/lib/module/types/expr.ex +++ b/lib/elixir/lib/module/types/expr.ex @@ -221,7 +221,7 @@ defmodule Module.Types.Expr do {head_type, context} = of_expr(head, stack, context) context = - if stack.mode == :infer do + if stack.mode in [:infer, :traversal] do context else case truthness(head_type) do @@ -386,14 +386,19 @@ defmodule Module.Types.Expr do ) when is_atom(name) and is_integer(arity) do {remote_type, context} = of_expr(remote, stack, context) + # TODO: We cannot return the unions of functions. Do we forbid this? # Do we check it is always the same return type? Do we simply say it is a function? {mods, context} = Of.modules(remote_type, name, arity, expr, meta, stack, context) context = - Enum.reduce(mods, context, &(Apply.signature(&1, name, arity, meta, stack, &2) |> elem(1))) + Enum.reduce( + mods, + context, + &(Apply.signature(&1, name, arity, meta, stack, &2) |> elem(1)) + ) - {fun(), context} + {dynamic(fun()), context} end # TODO: &foo/1 @@ -416,8 +421,12 @@ defmodule Module.Types.Expr do end # var - def of_expr(var, _stack, context) when is_var(var) do - {Of.var(var, context), context} + def of_expr(var, stack, context) when is_var(var) do + if stack.mode == :traversal do + {dynamic(), context} + else + {Of.var(var, context), context} + end end ## Try @@ -551,7 +560,7 @@ defmodule Module.Types.Expr do defp dynamic_unless_static({_, _} = output, %{mode: :static}), do: output defp dynamic_unless_static({type, context}, %{mode: _}), do: {dynamic(type), context} - defp of_clauses(clauses, expected, info, stack, {acc, context}) do + defp of_clauses(clauses, expected, info, %{mode: mode} = stack, {acc, context}) do %{failed: failed?} = context Enum.reduce(clauses, {acc, context}, fn {:->, meta, [head, body]}, {acc, context} -> @@ -559,7 +568,13 @@ defmodule Module.Types.Expr do {patterns, guards} = extract_head(head) {_types, context} = Pattern.of_head(patterns, guards, expected, info, meta, stack, context) {body, context} = of_expr(body, stack, context) - {union(acc, body), set_failed(context, failed?)} + context = set_failed(context, failed?) + + if mode == :traversal do + {dynamic(), context} + else + {union(acc, body), context} + end end) end diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex index 9cec8e640d9..77f51a16d7c 100644 --- a/lib/elixir/lib/module/types/pattern.ex +++ b/lib/elixir/lib/module/types/pattern.ex @@ -25,6 +25,10 @@ defmodule Module.Types.Pattern do is refined, we restart at step 2. """ + def of_head(_patterns, _guards, expected, _tag, _meta, %{mode: :traversal}, context) do + {expected, context} + end + def of_head(patterns, guards, expected, tag, meta, stack, context) do stack = %{stack | meta: meta} @@ -98,7 +102,13 @@ defmodule Module.Types.Pattern do This version tracks the whole expression in tracing, instead of only the pattern. """ - def of_match(pattern, guards \\ [], expected, expr, tag, stack, context) do + def of_match(pattern, guards \\ [], expected, expr, tag, stack, context) + + def of_match(_pattern, _guards, expected, _expr, _tag, %{mode: :traversal}, context) do + {expected, context} + end + + def of_match(pattern, guards, expected, expr, tag, stack, context) do context = init_pattern_info(context) {tree, context} = of_pattern(pattern, [{:arg, 0, expr}], stack, context) diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl index 9d64565c5ba..bca01d5dc9a 100644 --- a/lib/elixir/src/elixir_def.erl +++ b/lib/elixir/src/elixir_def.erl @@ -37,11 +37,10 @@ fun_for(Meta, Module, Name, Arity, Kinds, External) -> {[{_, Kind, LocalMeta, _, _, _}], ClausesPairs} -> case (Kinds == all) orelse (lists:member(Kind, Kinds)) of true -> - TrackLocal = (Kind == defmacrop), - TrackLocal andalso track_local(Module, Tuple), - Local = {value, fun(Fun, Args) -> invoke_local(Meta, Module, Fun, Args, TrackLocal, External) end}, + (Kind == defmacrop) andalso track_defmacrop(Module, Tuple), + Local = {value, fun(Fun, Args) -> invoke_local(Meta, Module, Fun, Args, External) end}, Clauses = [Clause || {_, Clause} <- ClausesPairs], - {Kind, elixir_erl:definition_to_anonymous(Kind, LocalMeta, Clauses, Local, External)}; + elixir_erl:definition_to_anonymous(Kind, LocalMeta, Clauses, Local, External); false -> false end; @@ -51,21 +50,20 @@ fun_for(Meta, Module, Name, Arity, Kinds, External) -> _:_ -> false end. -invoke_local(Meta, Module, ErlName, Args, TrackLocal, External) -> +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]); - {Kind, Fun} -> - (TrackLocal and (Kind == defp)) andalso track_local(Module, {Name, Arity}), + Fun -> apply(Fun, Args) end. -track_local(Module, FunArity) -> +track_defmacrop(Module, FunArity) -> {_, Bag} = elixir_module:data_tables(Module), - ets:insert(Bag, {macro_private_calls, FunArity}). + ets:insert(Bag, {defmacrop_calls, FunArity}). invoke_external(Meta, Mod, Name, Args, E) -> is_map(E) andalso elixir_env:trace({require, Meta, Mod, []}, E), diff --git a/lib/elixir/src/elixir_dispatch.erl b/lib/elixir/src/elixir_dispatch.erl index 8fd5a6d5b04..121b1683642 100644 --- a/lib/elixir/src/elixir_dispatch.erl +++ b/lib/elixir/src/elixir_dispatch.erl @@ -170,20 +170,20 @@ expand_import(Meta, Name, Arity, E, Extra, AllowLocals, Trace) -> _ -> Local = AllowLocals andalso elixir_def:local_for(Meta, Name, Arity, [defmacro, defmacrop], E), - case {Dispatch, Local} of + case Dispatch of %% There is a local and an import. This is a conflict unless %% the receiver is the same as module (happens on bootstrap). - {{_, Receiver}, {_, _}} when Receiver /= Module -> + {_, Receiver} when Local /= false, Receiver /= Module -> {conflict, Receiver}; %% There is no local. Dispatch the import. - {_, false} -> + _ when Local == false -> do_expand_import(Dispatch, Meta, Name, Arity, Module, E, Trace); %% Dispatch to the local. - {_, {_Kind, Fun}} -> + _ -> Trace andalso elixir_env:trace({local_macro, Meta, Name, Arity}, E), - {macro, Module, expander_macro_fun(Meta, Fun, Module, Name, E)} + {macro, Module, expander_macro_fun(Meta, Local, Module, Name, E)} end end. diff --git a/lib/elixir/src/elixir_map.erl b/lib/elixir/src/elixir_map.erl index 1512d786c5b..d2b0fdac573 100644 --- a/lib/elixir/src/elixir_map.erl +++ b/lib/elixir/src/elixir_map.erl @@ -192,7 +192,7 @@ maybe_load_struct(Meta, Name, Assocs, E) -> false -> Name:'__struct__'(Assocs); - {_, ExternalFun} -> + ExternalFun -> %% There is an inherent race condition when using external_for. %% By the time we got to execute the function, the ETS table %% with temporary definitions for the given module may no longer diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index e887db3f3c8..053eb71cced 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -187,8 +187,8 @@ compile(Meta, Module, ModuleAsCharlist, Block, Vars, Prune, E) -> case elixir_config:is_bootstrap() of true -> {#{}, []}; false -> - Used = bag_lookup_element(DataBag, macro_private_calls, 2), - 'Elixir.Module.Types':infer(Module, File, AllDefinitions, NewPrivate, Used, E) + Defmacrop = bag_lookup_element(DataBag, defmacrop_calls, 2), + 'Elixir.Module.Types':infer(Module, File, AllDefinitions, NewPrivate, Defmacrop, E) end, RawCompileOpts = bag_lookup_element(DataBag, {accumulate, compile}, 2), diff --git a/lib/elixir/test/elixir/kernel/warning_test.exs b/lib/elixir/test/elixir/kernel/warning_test.exs index 0e3073996c5..1f0f975adbc 100644 --- a/lib/elixir/test/elixir/kernel/warning_test.exs +++ b/lib/elixir/test/elixir/kernel/warning_test.exs @@ -645,37 +645,6 @@ defmodule Kernel.WarningTest do purge([Sample, Sample2]) end - test "unused function conditionall inside macro" do - assert_warn_eval( - ["nofile:3:8: ", "function error/0 is unused"], - """ - defmodule Sample do - defp ok, do: :ok - defp error, do: :error - - defmacrop hello(x) do - if x, do: ok(), else: error() - end - - def uses_hello, do: hello(true) - end - """ - ) - - assert_warn_eval( - ["nofile:2:13: ", "macro hello/0 is unused\n"], - ~S""" - defmodule Sample2 do - defmacrop hello do - quote do: unquote(1) - end - end - """ - ) - after - purge([Sample, Sample2]) - end - test "shadowing" do assert capture_eval(""" defmodule Sample do From 1322027a525e005832d10466a9faa7d7404ce036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 22:42:01 +0100 Subject: [PATCH 18/22] Bring infer signatures back --- lib/elixir/lib/code.ex | 14 +++- lib/elixir/lib/module/types.ex | 112 +++++++++++++++++------------- lib/elixir/src/elixir.erl | 1 + lib/mix/lib/mix/compilers/test.ex | 2 +- 4 files changed, 77 insertions(+), 52 deletions(-) diff --git a/lib/elixir/lib/code.ex b/lib/elixir/lib/code.ex index 48f6fb9077b..8546ad03930 100644 --- a/lib/elixir/lib/code.ex +++ b/lib/elixir/lib/code.ex @@ -249,6 +249,7 @@ defmodule Code do :debug_info, :ignore_already_consolidated, :ignore_module_conflict, + i:nfer_signatures, :relative_paths ] @@ -1561,8 +1562,8 @@ defmodule Code do ## Examples - Code.compiler_options(ignore_module_conflict: true) - #=> %{ignore_module_conflict: false} + Code.compiler_options(infer_signatures: false) + #=> %{infer_signatures: true} """ @spec compiler_options(Enumerable.t({atom, term})) :: %{optional(atom) => term} @@ -1646,6 +1647,15 @@ defmodule Code do * `:ignore_module_conflict` - when `true`, does not warn when a module has already been defined. Defaults to `false`. + * `:infer_signatures` (since v1.18.0) - when `false`, it disables module-local + signature inference used when type checking remote calls to the compiled + module. Type checking will be executed regardless of the value of this option. + Defaults to `true`. + + `mix test` automatically disables this option via the `:test_elixirc_options` + project configuration, as there is typically no need to store infer signatures + for test files. + * `:relative_paths` - when `true`, uses relative paths in quoted nodes, warnings, and errors generated by the compiler. Note disabling this option won't affect runtime warnings and errors. Defaults to `true`. diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 75c531a652a..5e0c67aa050 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -2,13 +2,48 @@ defmodule Module.Types do @moduledoc false alias Module.Types.{Descr, Expr, Pattern, Helpers} + # The mode controls what happens on function application when + # there are gradual arguments. Non-gradual arguments always + # perform subtyping and return its output (OUT). + # + # * :strict - Requires types signatures (not implemented). + # * Strong arrows with gradual performs subtyping and returns OUT + # * Weak arrows with gradual performs subtyping and returns OUT + # + # * :static - Type signatures have been given. + # * Strong arrows with gradual performs compatibility and returns OUT + # * Weak arrows with gradual performs compatibility and returns dynamic() + # + # * :dynamic - Type signatures have not been given. + # * Strong arrows with gradual performs compatibility and returns dynamic(OUT) + # * Weak arrows with gradual performs compatibility and returns dynamic() + # + # * :infer - Same as :dynamic but skips remote calls. + # + # * :traversal - Focused mostly on traversing AST, skips most type system + # operations. Used by macros and when skipping inference. + # + # The mode may also control exhaustiveness checks in the future (to be decided). + # We may also want for applications with subtyping in dynamic mode to always + # intersect with dynamic, but this mode may be too lax (to be decided based on + # feedback). + @modes [:static, :dynamic, :infer, :traversal] + # These functions are not inferred because they are added/managed by the compiler @no_infer [__protocol__: 1, behaviour_info: 1] @doc false def infer(module, file, defs, private, defmacrop, env) do + infer_signatures? = :elixir_config.get(:infer_signatures) defmacrop = Map.from_keys(defmacrop, []) - finder = &:lists.keyfind(&1, 1, defs) + + finder = + fn fun_arity -> + case :lists.keyfind(fun_arity, 1, defs) do + {_, kind, _, _} = clause -> {infer_mode(kind, infer_signatures?), clause} + false -> false + end + end handler = fn meta, fun_arity, stack, context -> case local_handler(meta, fun_arity, stack, context, finder) do @@ -34,10 +69,10 @@ defmodule Module.Types do {types, context} -> cond do kind in [:def, :defmacro] -> - {_kind, inferred, context} = - local_handler(meta, fun_arity, stack, context, fn _ -> def end) + finder = fn _ -> {infer_mode(kind, infer_signatures?), def} end + {_kind, inferred, context} = local_handler(meta, fun_arity, stack, context, finder) - if kind == :def and fun_arity not in @no_infer do + if infer_signatures? and kind == :def and fun_arity not in @no_infer do {[{fun_arity, inferred} | types], context} else {types, context} @@ -48,9 +83,10 @@ defmodule Module.Types do # we don't need them stored in the signatures when we perform # unreachable checks. This may cause defmacrop to be traversed # twice if it uses default arguments (which is the only way - # to refer to another defmacrop in definitions). + # to refer to another defmacrop in definitions) but that should + # be cheap anyway. {_kind, _inferred, context} = - local_handler(fun_arity, kind, meta, clauses, stack, context) + local_handler(fun_arity, kind, meta, clauses, :traversal, stack, context) {types, context} @@ -68,6 +104,10 @@ defmodule Module.Types do {Map.new(types), unreachable} end + defp infer_mode(kind, infer_signatures?) do + if infer_signatures? and kind in [:def, :defp], do: :infer, else: :traversal + end + defp undefined_function!(reason, meta, {fun, arity}, stack, env) do env = %{env | function: stack.function, file: stack.file} tuple = {reason, {fun, arity}, stack.module} @@ -117,16 +157,21 @@ defmodule Module.Types do @doc false def warnings(module, file, defs, no_warn_undefined, cache) do - finder = &:lists.keyfind(&1, 1, defs) + finder = fn fun_arity -> + case :lists.keyfind(fun_arity, 1, defs) do + {_, _, _, _} = clause -> {:dynamic, clause} + false -> false + end + end + handler = &local_handler(&1, &2, &3, &4, finder) stack = stack(:dynamic, file, module, {:__info__, 1}, no_warn_undefined, cache, handler) context = context(%{}) context = Enum.reduce(defs, context, fn {fun_arity, _kind, meta, _clauses} = def, context -> - {_kind, _inferred, context} = - local_handler(meta, fun_arity, stack, context, fn _ -> def end) - + finder = fn _ -> {:dynamic, def} end + {_kind, _inferred, context} = local_handler(meta, fun_arity, stack, context, finder) context end) @@ -161,11 +206,11 @@ defmodule Module.Types do local_sigs -> case finder.(fun_arity) do - {fun_arity, kind, meta, clauses} -> + {mode, {fun_arity, kind, meta, clauses}} -> context = put_in(context.local_sigs, Map.put(local_sigs, fun_arity, kind)) {inferred, mapping, context} = - local_handler(fun_arity, kind, meta, clauses, stack, context) + local_handler(fun_arity, kind, meta, clauses, mode, stack, context) context = update_in(context.local_sigs, &Map.put(&1, fun_arity, {kind, inferred, mapping})) @@ -178,9 +223,9 @@ defmodule Module.Types do end end - defp local_handler({fun, arity} = fun_arity, kind, meta, clauses, stack, context) do + defp local_handler({fun, arity} = fun_arity, kind, meta, clauses, mode, stack, context) do expected = List.duplicate(Descr.dynamic(), arity) - stack = stack |> fresh_stack(kind, fun_arity) |> with_file_meta(meta) + stack = stack |> fresh_stack(mode, fun_arity) |> with_file_meta(meta) {_, _, mapping, clauses_types, clauses_context} = Enum.reduce(clauses, {0, 0, [], [], context}, fn @@ -259,7 +304,7 @@ defmodule Module.Types do @doc false def stack(mode, file, module, function, no_warn_undefined, cache, handler) - when mode in [:static, :dynamic, :infer, :traversal] do + when mode in @modes do %{ # The fallback meta used for literals in patterns and guards meta: [], @@ -273,31 +318,7 @@ defmodule Module.Types do no_warn_undefined: no_warn_undefined, # A tuple with cache information or a Macro.Env struct indicating no remote traversals cache: cache, - # The mode controls what happens on function application when - # there are gradual arguments. Non-gradual arguments always - # perform subtyping and return its output (OUT). - # - # * :strict - Requires types signatures (not implemented). - # * Strong arrows with gradual performs subtyping and returns OUT - # * Weak arrows with gradual performs subtyping and returns OUT - # - # * :static - Type signatures have been given. - # * Strong arrows with gradual performs compatibility and returns OUT - # * Weak arrows with gradual performs compatibility and returns dynamic() - # - # * :dynamic - Type signatures have not been given. - # * Strong arrows with gradual performs compatibility and returns dynamic(OUT) - # * Weak arrows with gradual performs compatibility and returns dynamic() - # - # * :infer - Same as :dynamic but skips remote calls. - # - # * :traversal - Focused mostly on traversing AST, skips most type system - # operations. Used by macros and functions which already have signatures. - # - # The mode may also control exhaustiveness checks in the future (to be decided). - # We may also want for applications with subtyping in dynamic mode to always - # intersect with dynamic, but this mode may be too lax (to be decided based on - # feedback). + # The mode to be used, see the @modes attribute mode: mode, # The function for handling local calls local_handler: handler @@ -322,15 +343,8 @@ defmodule Module.Types do } end - defp fresh_stack(%{mode: mode} = stack, kind, function) do - mode = - cond do - kind in [:defmacro, :defmacrop] and mode == :infer -> :traversal - kind in [:def, :defp] and mode == :traversal -> :infer - true -> mode - end - - %{stack | function: function, mode: mode} + defp fresh_stack(stack, mode, function) when mode in @modes do + %{stack | mode: mode, function: function} end defp fresh_context(context) do diff --git a/lib/elixir/src/elixir.erl b/lib/elixir/src/elixir.erl index 7c568d511e7..b53f67017af 100644 --- a/lib/elixir/src/elixir.erl +++ b/lib/elixir/src/elixir.erl @@ -90,6 +90,7 @@ start(_Type, _Args) -> {docs, true}, {ignore_already_consolidated, false}, {ignore_module_conflict, false}, + {infer_signatures, true}, {on_undefined_variable, raise}, {parser_options, [{columns, true}]}, {debug_info, true}, diff --git a/lib/mix/lib/mix/compilers/test.ex b/lib/mix/lib/mix/compilers/test.ex index 0616179069c..1cd86d20e30 100644 --- a/lib/mix/lib/mix/compilers/test.ex +++ b/lib/mix/lib/mix/compilers/test.ex @@ -23,7 +23,7 @@ defmodule Mix.Compilers.Test do """ def require_and_run(matched_test_files, test_paths, elixirc_opts, opts) do elixirc_opts = - Keyword.merge([docs: false, debug_info: false], elixirc_opts) + Keyword.merge([docs: false, debug_info: false, infer_signatures: false], elixirc_opts) previous_opts = Code.compiler_options(elixirc_opts) From 5413679642ab2554c57df247c1e19b57c8ba537a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 22:42:34 +0100 Subject: [PATCH 19/22] Fix typo --- lib/elixir/lib/code.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/code.ex b/lib/elixir/lib/code.ex index 8546ad03930..1a4a400f06b 100644 --- a/lib/elixir/lib/code.ex +++ b/lib/elixir/lib/code.ex @@ -249,7 +249,7 @@ defmodule Code do :debug_info, :ignore_already_consolidated, :ignore_module_conflict, - i:nfer_signatures, + :infer_signatures, :relative_paths ] From 954a8c679b66e7af03ec24a1c6dacc06dc9512f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 22:47:25 +0100 Subject: [PATCH 20/22] Less indirection --- lib/elixir/lib/module/types.ex | 14 ++++++-------- .../test/elixir/module/types/type_helper.exs | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/elixir/lib/module/types.ex b/lib/elixir/lib/module/types.ex index 5e0c67aa050..10891f2387d 100644 --- a/lib/elixir/lib/module/types.ex +++ b/lib/elixir/lib/module/types.ex @@ -62,10 +62,9 @@ defmodule Module.Types do end stack = stack(:infer, file, module, {:__info__, 1}, :all, env, handler) - context = context(%{}) {types, %{local_sigs: local_sigs}} = - for {fun_arity, kind, meta, clauses} = def <- defs, reduce: {[], context} do + for {fun_arity, kind, meta, clauses} = def <- defs, reduce: {[], context()} do {types, context} -> cond do kind in [:def, :defmacro] -> @@ -166,10 +165,9 @@ defmodule Module.Types do handler = &local_handler(&1, &2, &3, &4, finder) stack = stack(:dynamic, file, module, {:__info__, 1}, no_warn_undefined, cache, handler) - context = context(%{}) context = - Enum.reduce(defs, context, fn {fun_arity, _kind, meta, _clauses} = def, context -> + Enum.reduce(defs, context(), fn {fun_arity, _kind, meta, _clauses} = def, context -> finder = fn _ -> {:dynamic, def} end {_kind, _inferred, context} = local_handler(meta, fun_arity, stack, context, finder) context @@ -326,7 +324,7 @@ defmodule Module.Types do end @doc false - def context(local_sigs) do + def context() do %{ # A list of all warnings found so far warnings: [], @@ -336,9 +334,9 @@ defmodule Module.Types do pattern_info: nil, # If type checking has found an error/failure failed: false, - # Local signatures - local_sigs: local_sigs, - # Local clauses + # Local signatures used by local handler + local_sigs: %{}, + # Track which clauses have been used across private local calls local_used: %{} } end diff --git a/lib/elixir/test/elixir/module/types/type_helper.exs b/lib/elixir/test/elixir/module/types/type_helper.exs index c56c999bcf2..e17187fd841 100644 --- a/lib/elixir/test/elixir/module/types/type_helper.exs +++ b/lib/elixir/test/elixir/module/types/type_helper.exs @@ -143,7 +143,7 @@ defmodule TypeHelper do end defp new_context() do - Types.context(%{}) + Types.context() end @doc """ From 6da90be6aa3eb4c7cf834ea6ad632410b1e89e1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 22:58:55 +0100 Subject: [PATCH 21/22] Optimize more traversals --- lib/elixir/lib/module/types/expr.ex | 31 +++++++++++++++++++++-------- lib/elixir/lib/module/types/of.ex | 11 ++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex index 6705f94afd7..5bc129a7ee2 100644 --- a/lib/elixir/lib/module/types/expr.ex +++ b/lib/elixir/lib/module/types/expr.ex @@ -78,14 +78,35 @@ defmodule Module.Types.Expr do {prefix, suffix} = unpack_list(list, []) {prefix, context} = Enum.map_reduce(prefix, context, &of_expr(&1, stack, &2)) {suffix, context} = of_expr(suffix, stack, context) - {non_empty_list(Enum.reduce(prefix, &union/2), suffix), context} + + if stack.mode == :traversal do + {dynamic(), context} + else + {non_empty_list(Enum.reduce(prefix, &union/2), suffix), context} + end end # {left, right} def of_expr({left, right}, stack, context) do {left, context} = of_expr(left, stack, context) {right, context} = of_expr(right, stack, context) - {tuple([left, right]), context} + + if stack.mode == :traversal do + {dynamic(), context} + else + {tuple([left, right]), context} + end + end + + # {...} + def of_expr({:{}, _meta, exprs}, stack, context) do + {types, context} = Enum.map_reduce(exprs, context, &of_expr(&1, stack, &2)) + + if stack.mode == :traversal do + {dynamic(), context} + else + {tuple(types), context} + end end # <<...>>> @@ -103,12 +124,6 @@ defmodule Module.Types.Expr do {@stacktrace, context} end - # {...} - def of_expr({:{}, _meta, exprs}, stack, context) do - {types, context} = Enum.map_reduce(exprs, context, &of_expr(&1, stack, &2)) - {tuple(types), context} - end - # left = right def of_expr({:=, _, [left_expr, right_expr]} = expr, stack, context) do {left_expr, right_expr} = repack_match(left_expr, right_expr) diff --git a/lib/elixir/lib/module/types/of.ex b/lib/elixir/lib/module/types/of.ex index efd2631baed..f477316c7f9 100644 --- a/lib/elixir/lib/module/types/of.ex +++ b/lib/elixir/lib/module/types/of.ex @@ -115,6 +115,17 @@ defmodule Module.Types.Of do @doc """ Builds permutation of maps according to the given keys. """ + def permutate_map(pairs, %{mode: :traversal} = stack, context, of_fun, _of_map) do + context = + Enum.reduce(pairs, context, fn {key, value}, context -> + {_, context} = of_fun.(key, stack, context) + {_, context} = of_fun.(value, stack, context) + context + end) + + {dynamic(), context} + end + def permutate_map(pairs, stack, context, of_fun, of_map) do {dynamic?, fallback, single, multiple, assert, context} = Enum.reduce(pairs, {false, none(), [], [], [], context}, fn From e8f7c90bd3990592f2a45de39a175c741ca1d5dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 11 Nov 2024 23:06:10 +0100 Subject: [PATCH 22/22] Update CHANGELOG --- CHANGELOG.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb9c043c14d..7405319ea9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,15 @@ TODO. * Type inference of patterns (typing inference of guards will be part of an upcoming release) -* [Support for tuples and lists as composite types](https://elixir-lang.org/blog/2024/08/28/typing-lists-and-tuples/) as well as type checking of their basic operations - -* Type checking of all language constructs, except `for`, `with`, and closures +* Type checking of all language constructs, including local and remote calls, except `for`, `with`, and closures * Type checking of all `Kernel` and conversion functions inlined by the compiler -* Detection of clauses and patterns that never match +* [Support for tuples and lists as composite types](https://elixir-lang.org/blog/2024/08/28/typing-lists-and-tuples/) as well as type checking of their basic operations + +* Detection of clauses and patterns that will never match from `case`, `cond`, and `=` + +* Detection of unused clauses from private functions ## ExUnit improvements