Skip to content

Commit

Permalink
Track variable version globally (#9536)
Browse files Browse the repository at this point in the history
Instead of incrementing the version for each variable individually we increment it globally for all variables. This means we can uniquely identify each variable assignment with only the version.
  • Loading branch information
ericmj committed Nov 15, 2019
1 parent b844819 commit b3e48bc
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 65 deletions.
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
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

0 comments on commit b3e48bc

Please sign in to comment.