Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track variable version globally #9536

Merged
merged 1 commit into from Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/elixir/lib/macro/env.ex
Expand Up @@ -71,7 +71,7 @@ defmodule Macro.Env do
{%{optional(variable) => {var_version, var_type}},
%{optional(variable) => {var_version, var_type}} | false}
@typep unused_vars ::
%{optional({variable, var_version}) => non_neg_integer | false}
{%{optional({variable, var_version}) => non_neg_integer | false}, non_neg_integer}
@typep prematch_vars ::
%{optional(variable) => {var_version, var_type}} | :warn | :raise | :pin | :apply
@typep tracers :: [module]
Expand Down Expand Up @@ -121,7 +121,7 @@ defmodule Macro.Env do
prematch_vars: :warn,
requires: [],
tracers: [],
unused_vars: %{},
unused_vars: {%{}, 0},
vars: []
}
end
Expand Down
2 changes: 1 addition & 1 deletion lib/elixir/lib/module/types/helpers.ex
Expand Up @@ -16,7 +16,7 @@ defmodule Module.Types.Helpers do
@doc """
Returns unique identifier for the current assignment of the variable.
"""
def var_name({name, meta, _context}), do: {name, Keyword.fetch!(meta, :version)}
def var_name({_name, meta, _context}), do: Keyword.fetch!(meta, :version)

@doc """
Push expression to stack.
Expand Down
22 changes: 12 additions & 10 deletions lib/elixir/src/elixir_env.erl
Expand Up @@ -25,7 +25,7 @@ new() ->
context_modules => [], %% modules defined in the current context
vars => [], %% a set of defined variables
current_vars => {#{}, false}, %% a tuple with maps of read and optional write current vars
unused_vars => #{}, %% a map of unused vars
unused_vars => {#{}, 0}, %% a map of unused vars and a version counter for vars
prematch_vars => warn, %% controls behaviour outside and inside matches
lexical_tracker => nil, %% lexical tracker PID
contextual_vars => [], %% available contextual variables
Expand All @@ -42,8 +42,10 @@ linify(#{} = Env) ->
Env.

with_vars(Env, Vars) ->
Read = maps:from_list([{Var, 0} || Var <- Vars]),
Env#{vars := Vars, current_vars := {Read, false}, unused_vars := #{}}.
NumVars = length(Vars),
VarVersions = lists:zip(Vars, lists:seq(0, NumVars - 1)),
Read = maps:from_list(VarVersions),
Env#{vars := Vars, current_vars := {Read, false}, unused_vars := {#{}, NumVars}}.

env_to_scope(#{context := Context}) ->
#elixir_erl{context=Context}.
Expand All @@ -56,7 +58,7 @@ env_to_scope_with_vars(Env, Vars) ->
}.

reset_vars(Env) ->
Env#{vars := [], current_vars := {#{}, false}, unused_vars := #{}}.
Env#{vars := [], current_vars := {#{}, false}, unused_vars := {#{}, 0}}.

%% SCOPE MERGING

Expand All @@ -83,17 +85,17 @@ merge_vars(V1, V2) ->

%% UNUSED VARS

reset_unused_vars(E) ->
E#{unused_vars := #{}}.
reset_unused_vars(#{unused_vars := {_Unused, Version}} = E) ->
E#{unused_vars := {#{}, Version}}.

check_unused_vars(#{unused_vars := Unused} = E) ->
check_unused_vars(#{unused_vars := {Unused, _Version}} = E) ->
[elixir_errors:form_warn([{line, Line}], E, ?MODULE, {unused_var, Name}) ||
{{{Name, _}, _}, Line} <- maps:to_list(Unused), Line /= false, not_underscored(Name)],
E.

merge_and_check_unused_vars(E, #{unused_vars := ClauseUnused}) ->
#{current_vars := {Read, _}, unused_vars := Unused} = E,
E#{unused_vars := merge_and_check_unused_vars(Read, Unused, ClauseUnused, E)}.
merge_and_check_unused_vars(E, #{unused_vars := {ClauseUnused, Version}}) ->
#{current_vars := {Read, _}, unused_vars := {Unused, _Version}} = E,
E#{unused_vars := {merge_and_check_unused_vars(Read, Unused, ClauseUnused, E), Version}}.

merge_and_check_unused_vars(Current, Unused, ClauseUnused, E) ->
maps:fold(fun({Var, Count} = Key, ClauseValue, Acc) ->
Expand Down
38 changes: 19 additions & 19 deletions lib/elixir/src/elixir_expand.erl
Expand Up @@ -338,51 +338,51 @@ expand({'_', Meta, Kind}, E) when is_atom(Kind) ->
form_error(Meta, E, ?MODULE, unbound_underscore);

expand({Name, Meta, Kind}, #{context := match} = E) when is_atom(Name), is_atom(Kind) ->
#{current_vars := {ReadCurrent, WriteCurrent}, unused_vars := Unused, prematch_vars := Prematch} = E,
#{current_vars := {ReadCurrent, WriteCurrent}, unused_vars := {Unused, Version}, prematch_vars := Prematch} = E,
Pair = {Name, elixir_utils:var_context(Meta, Kind)},
PrematchVersion = var_version(Prematch, Pair),

case ReadCurrent of
%% Variable is being overridden now
#{Pair := PrematchVersion} ->
NewUnused = var_unused(Pair, Meta, PrematchVersion + 1, Unused),
NewReadCurrent = ReadCurrent#{Pair => PrematchVersion + 1},
NewWriteCurrent = (WriteCurrent /= false) andalso WriteCurrent#{Pair => PrematchVersion + 1},
Var = {Name, [{version, PrematchVersion + 1} | Meta], Kind},
{Var, E#{current_vars := {NewReadCurrent, NewWriteCurrent}, unused_vars := NewUnused}};
NewUnused = var_unused(Pair, Meta, Version, Unused),
NewReadCurrent = ReadCurrent#{Pair => Version},
NewWriteCurrent = (WriteCurrent /= false) andalso WriteCurrent#{Pair => Version},
Var = {Name, [{version, Version} | Meta], Kind},
{Var, E#{current_vars := {NewReadCurrent, NewWriteCurrent}, unused_vars := {NewUnused, Version + 1}}};

%% Variable was already overriden
#{Pair := CurrentVersion} ->
maybe_warn_underscored_var_repeat(Meta, Name, Kind, E),
NewUnused = Unused#{{Pair, CurrentVersion} => false},
Var = {Name, [{version, CurrentVersion} | Meta], Kind},
{Var, E#{unused_vars := NewUnused}};
{Var, E#{unused_vars := {NewUnused, Version}}};

%% Variable defined for the first time
_ ->
NewVars = ordsets:add_element(Pair, ?key(E, vars)),
NewUnused = var_unused(Pair, Meta, 0, Unused),
NewReadCurrent = ReadCurrent#{Pair => 0},
NewWriteCurrent = (WriteCurrent /= false) andalso WriteCurrent#{Pair => 0},
Var = {Name, [{version, 0} | Meta], Kind},
{Var, E#{vars := NewVars, current_vars := {NewReadCurrent, NewWriteCurrent}, unused_vars := NewUnused}}
NewUnused = var_unused(Pair, Meta, Version, Unused),
NewReadCurrent = ReadCurrent#{Pair => Version},
NewWriteCurrent = (WriteCurrent /= false) andalso WriteCurrent#{Pair => Version},
Var = {Name, [{version, Version} | Meta], Kind},
{Var, E#{vars := NewVars, current_vars := {NewReadCurrent, NewWriteCurrent}, unused_vars := {NewUnused, Version + 1}}}
end;

expand({Name, Meta, Kind}, E) when is_atom(Name), is_atom(Kind) ->
#{current_vars := {ReadCurrent, _WriteCurrent}, unused_vars := Unused} = E,
#{current_vars := {ReadCurrent, _WriteCurrent}, unused_vars := {Unused, Version}} = E,
Pair = {Name, elixir_utils:var_context(Meta, Kind)},

case ReadCurrent of
#{Pair := Version} ->
#{Pair := PairVersion} ->
maybe_warn_underscored_var_access(Meta, Name, Kind, E),
UnusedKey = {Pair, Version},
Var = {Name, [{version, Version} | Meta], Kind},
UnusedKey = {Pair, PairVersion},
Var = {Name, [{version, PairVersion} | Meta], Kind},

case Unused of
#{UnusedKey := false} ->
{Var, E};
_ ->
{Var, E#{unused_vars := Unused#{UnusedKey => false}}}
{Var, E#{unused_vars := {Unused#{UnusedKey => false}, Version}}}
end;

_ ->
Expand Down Expand Up @@ -499,13 +499,13 @@ expand(Other, E) ->

%% Helpers

escape_env_entries(Meta, #{current_vars := {Read, Write}, unused_vars := Unused} = Env0) ->
escape_env_entries(Meta, #{current_vars := {Read, Write}, unused_vars := {Unused, Version}} = Env0) ->
Env1 = case Env0 of
#{function := nil} -> Env0;
_ -> Env0#{lexical_tracker := nil}
end,
Current = {maybe_escape_map(Read), maybe_escape_map(Write)},
Env2 = Env1#{current_vars := Current, unused_vars := maybe_escape_map(Unused)},
Env2 = Env1#{current_vars := Current, unused_vars := {maybe_escape_map(Unused), Version}},
Env3 = elixir_env:linify({?line(Meta), Env2}),
Env3.

Expand Down
4 changes: 2 additions & 2 deletions lib/elixir/src/elixir_module.erl
Expand Up @@ -72,8 +72,8 @@ compile(Module, Block, Vars, #{line := Line, current_vars := {Read, _}} = Env) w
%% point, the lexical tracker process is long gone.
MaybeLexEnv =
case ?key(Env, function) of
nil -> Env#{module := Module, current_vars := {Read, false}, unused_vars := #{}};
_ -> Env#{lexical_tracker := nil, function := nil, module := Module, current_vars := {Read, false}, unused_vars := #{}}
nil -> Env#{module := Module, current_vars := {Read, false}, unused_vars := {#{}, 0}};
_ -> Env#{lexical_tracker := nil, function := nil, module := Module, current_vars := {Read, false}, unused_vars := {#{}, 0}}
end,

case MaybeLexEnv of
Expand Down
71 changes: 70 additions & 1 deletion lib/elixir/test/elixir/kernel/expansion_test.exs
Expand Up @@ -8,6 +8,75 @@ end
defmodule Kernel.ExpansionTest do
use ExUnit.Case, async: true

defmacrop var_ver(var, version) do
quote do
{unquote(var), [version: unquote(version)], __MODULE__}
end
end

test "tracks variable version" do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if there are other tests I should include.

assert {:__block__, _, [{:=, _, [var_ver(:x, 0), 0]}, {:=, _, [_, var_ver(:x, 0)]}]} =
expand_with_version(
quote do
x = 0
_ = x
end
)

assert {:__block__, _,
[
{:=, _, [var_ver(:x, 0), 0]},
{:=, _, [_, var_ver(:x, 0)]},
{:=, _, [var_ver(:x, 1), 1]},
{:=, _, [_, var_ver(:x, 1)]}
]} =
expand_with_version(
quote do
x = 0
_ = x
x = 1
_ = x
end
)

assert {:__block__, _,
[
{:=, _, [var_ver(:x, 0), 0]},
{:fn, _, [{:->, _, [[var_ver(:x, 1)], {:=, _, [var_ver(:x, 2), 2]}]}]},
{:=, _, [_, var_ver(:x, 0)]},
{:=, _, [var_ver(:x, 3), 3]}
]} =
expand_with_version(
quote do
x = 0
fn x -> x = 2 end
_ = x
x = 3
end
)

assert {:__block__, _,
[
{:=, _, [var_ver(:x, 0), 0]},
{:case, _, [:foo, [do: [{:->, _, [[var_ver(:x, 1)], var_ver(:x, 1)]}]]]},
{:=, _, [_, var_ver(:x, 0)]},
{:=, _, [var_ver(:x, 2), 2]}
]} =
expand_with_version(
quote do
x = 0
case(:foo, do: (x -> x))
_ = x
x = 2
end
)
end

defp expand_with_version(expr) do
env = :elixir_env.reset_vars(__ENV__)
elem(:elixir_expand.expand(expr, env), 0)
end

describe "__block__" do
test "expands to nil when empty" do
assert expand(quote(do: unquote(:__block__)())) == nil
Expand Down Expand Up @@ -2430,7 +2499,7 @@ defmodule Kernel.ExpansionTest do
expand(quote(do: unquote({1, 2, 3})))
end

assert_raise CompileError, ~r"invalid quoted expression: #Function<", fn ->
assert_raise CompileError, ~r"invalid quoted expression: #Function\<", fn ->
expand(quote(do: unquote({:sample, fn -> nil end})))
end

Expand Down
8 changes: 4 additions & 4 deletions lib/elixir/test/elixir/module/types/infer_test.exs
Expand Up @@ -133,7 +133,7 @@ defmodule Module.Types.InferTest do

test "vars" do
assert {{:var, 0}, var_context} = new_var({:foo, [version: 0], nil}, new_context())
assert {{:var, 1}, var_context} = new_var({:bar, [version: 0], nil}, var_context)
assert {{:var, 1}, var_context} = new_var({:bar, [version: 1], nil}, var_context)

assert {:ok, {:var, 0}, context} = unify({:var, 0}, :integer, var_context)
assert Types.lift_type({:var, 0}, context) == :integer
Expand Down Expand Up @@ -168,7 +168,7 @@ defmodule Module.Types.InferTest do

test "vars inside tuples" do
assert {{:var, 0}, var_context} = new_var({:foo, [version: 0], nil}, new_context())
assert {{:var, 1}, var_context} = new_var({:bar, [version: 0], nil}, var_context)
assert {{:var, 1}, var_context} = new_var({:bar, [version: 1], nil}, var_context)

assert {:ok, {:tuple, [{:var, 0}]}, context} =
unify({:tuple, [{:var, 0}]}, {:tuple, [:integer]}, var_context)
Expand Down Expand Up @@ -196,8 +196,8 @@ defmodule Module.Types.InferTest do

test "recursive type" do
assert {{:var, 0}, var_context} = new_var({:foo, [version: 0], nil}, new_context())
assert {{:var, 1}, var_context} = new_var({:bar, [version: 0], nil}, var_context)
assert {{:var, 2}, var_context} = new_var({:baz, [version: 0], nil}, var_context)
assert {{:var, 1}, var_context} = new_var({:bar, [version: 1], nil}, var_context)
assert {{:var, 2}, var_context} = new_var({:baz, [version: 2], nil}, var_context)

assert {:ok, {:var, _}, context} = unify({:var, 0}, {:var, 1}, var_context)
assert {:ok, {:var, _}, context} = unify({:var, 1}, {:var, 0}, context)
Expand Down
44 changes: 18 additions & 26 deletions lib/elixir/test/elixir/module/types/pattern_test.exs
Expand Up @@ -8,20 +8,23 @@ defmodule Module.Types.PatternTest do

defmacrop quoted_pattern(patterns) do
quote do
patterns = unquote(Macro.escape(expand_head(patterns)))
{patterns, true} = unquote(Macro.escape(expand_head(patterns, true)))

of_pattern(patterns, new_stack(), new_context())
|> lift_result()
end
end

defmacrop quoted_guard(guards, context) do
defmacrop quoted_guard(vars, guards) do
quote do
of_guard(unquote(Macro.escape(expand_guards(guards))), new_stack(), unquote(context))
{vars, guards} = unquote(Macro.escape(expand_head(vars, guards)))
context = Enum.reduce(vars, new_context(), &(new_var(&1, &2) |> elem(1)))

of_guard(guards, new_stack(), context)
end
end

defp expand_head(patterns) do
defp expand_head(patterns, guards) do
{_, vars} =
Macro.prewalk(patterns, [], fn
{:_, _, context} = var, vars when is_atom(context) ->
Expand All @@ -39,23 +42,12 @@ defmodule Module.Types.PatternTest do

fun =
quote do
fn unquote(patterns) -> unquote(vars) end
end

{ast, _env} = :elixir_expand.expand(fun, __ENV__)
{:fn, _, [{:->, _, [[patterns], _]}]} = ast
patterns
end

defp expand_guards(guards) do
fun =
quote do
fn var!(x) when unquote(guards) -> var!(x) end
fn unquote(patterns) when unquote(guards) -> unquote(vars) end
end

{ast, _env} = :elixir_expand.expand(fun, __ENV__)
{:fn, _, [{:->, _, [[{:when, _, [_, guards]}], _]}]} = ast
guards
{:fn, _, [{:->, _, [[{:when, _, [patterns, guards]}], _]}]} = ast
{patterns, guards}
end

defp new_context() do
Expand Down Expand Up @@ -83,10 +75,12 @@ defmodule Module.Types.PatternTest do

describe "of_pattern/3" do
test "error location" do
line = __ENV__.line + 3

assert {:error, {{:unable_unify, :integer, :binary, expr, traces}, location}} =
quoted_pattern(<<foo::integer, foo::binary>>)

assert location == [{"types_test.ex", 87, {TypesTest, :test, 0}}]
assert location == [{"types_test.ex", line, {TypesTest, :test, 0}}]

assert {:<<>>, _,
[
Expand All @@ -97,10 +91,10 @@ defmodule Module.Types.PatternTest do
assert [
{{:foo, _, nil},
{:type, :binary, {:"::", _, [{:foo, _, nil}, {:binary, _, _}]},
{"types_test.ex", 87}}},
{"types_test.ex", ^line}}},
{{:foo, _, nil},
{:type, :integer, {:"::", _, [{:foo, _, nil}, {:integer, _, _}]},
{"types_test.ex", 87}}}
{"types_test.ex", ^line}}}
] = traces
end

Expand Down Expand Up @@ -225,15 +219,13 @@ defmodule Module.Types.PatternTest do
end

test "of_guard/2" do
assert {{:var, 0}, var_context} = new_var({:x, [version: 0], nil}, new_context())

assert {:ok, :boolean, context} = quoted_guard(is_tuple(x), var_context)
assert {:ok, :boolean, context} = quoted_guard([x], is_tuple(x))
assert Types.lift_type({:var, 0}, context) == :tuple

assert {:ok, :dynamic, context} = quoted_guard(elem(x, 0), var_context)
assert {:ok, :dynamic, context} = quoted_guard([x], elem(x, 0))
assert Types.lift_type({:var, 0}, context) == :tuple

assert {:error, {_, {:unable_unify, :tuple, :boolean, _, _}, _}} =
quoted_guard(is_tuple(x) and is_boolean(x), var_context)
quoted_guard([x], is_tuple(x) and is_boolean(x))
end
end