diff options
author | Michał Muskała <michal@muskala.eu> | 2018-08-07 22:24:49 +0200 |
---|---|---|
committer | Michał Muskała <michal@muskala.eu> | 2018-12-07 17:52:08 +0100 |
commit | 7314ecfb7e70c9bc0e7aa367121e29a0863dacbc (patch) | |
tree | a54501c0451c96934c864d46c86fb09547bb0df5 | |
parent | 2b5511397874b589d216a9a127c1bdbeb9d96632 (diff) | |
download | elixir-7314ecfb7e70c9bc0e7aa367121e29a0863dacbc.tar.gz |
Make sure records use the dsetelement optimisationmm/records
Closes #8059
Additionally some more refactorings of the Record module
-rw-r--r-- | lib/elixir/lib/record.ex | 112 | ||||
-rw-r--r-- | lib/elixir/test/elixir/record_test.exs | 15 |
2 files changed, 72 insertions, 55 deletions
diff --git a/lib/elixir/lib/record.ex b/lib/elixir/lib/record.ex index ba9c2de21..654165b8c 100644 --- a/lib/elixir/lib/record.ex +++ b/lib/elixir/lib/record.ex @@ -379,38 +379,30 @@ defmodule Record do # Gets the index of field. defp index(tag, fields, field) do - if index = find_index(fields, field, 0) do - # Convert to Elixir index - index - 1 - else + find_index(fields, field, 1) || raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(field)}" - end end # Creates a new record with the given default fields and keyword values. defp create(tag, fields, keyword, caller) do - in_match = Macro.Env.in_match?(caller) - keyword = apply_underscore(fields, keyword) - - {match, remaining} = - Enum.map_reduce(fields, keyword, fn {field, default}, each_keyword -> - new_fields = - case Keyword.fetch(each_keyword, field) do - {:ok, value} -> value - :error when in_match -> {:_, [], nil} - :error -> Macro.escape(default) - end - - {new_fields, Keyword.delete(each_keyword, field)} + # Using {} here is safe, since it's not valid AST + default = if Macro.Env.in_match?(caller), do: {:_, [], nil}, else: {} + {default, keyword} = Keyword.pop(keyword, :_, default) + + {elements, remaining} = + Enum.map_reduce(fields, keyword, fn {key, field_default}, remaining -> + case Keyword.pop(remaining, key, default) do + {{}, remaining} -> {Macro.escape(field_default), remaining} + {default, remaining} -> {default, remaining} + end end) case remaining do [] -> - {:{}, [], [tag | match]} + quote(do: {unquote(tag), unquote_splicing(elements)}) - _ -> - keys = for {key, _} <- remaining, do: key - raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(hd(keys))}" + [{key, _} | _] -> + raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(key)}" end end @@ -420,35 +412,64 @@ defmodule Record do raise ArgumentError, "cannot invoke update style macro inside match" end - keyword = apply_underscore(fields, keyword) + if Keyword.has_key?(keyword, :_) do + IO.warn("updating a record with a default (:_) is equivalent to creating a new record") + create(tag, fields, keyword, caller) + else + case build_updates(keyword, fields, [], [], []) do + {updates, [], []} -> + build_update(updates, var) + + {updates, vars, exprs} -> + quote do + {unquote_splicing(:lists.reverse(vars))} = {unquote_splicing(:lists.reverse(exprs))} + unquote(build_update(updates, var)) + end - Enum.reduce(keyword, var, fn {key, value}, acc -> - index = find_index(fields, key, 0) + {:error, key} -> + raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(key)}" + end + end + end - if index do - quote do - :erlang.setelement(unquote(index), unquote(acc), unquote(value)) - end + defp build_update(updates, initial) do + updates + |> Enum.sort(fn {left, _}, {right, _} -> right <= left end) + |> Enum.reduce(initial, fn {key, value}, acc -> + quote(do: :erlang.setelement(unquote(key), unquote(acc), unquote(value))) + end) + end + + defp build_updates([{key, value} | rest], fields, updates, vars, exprs) do + if index = find_index(fields, key, 2) do + if simple_argument?(value) do + build_updates(rest, fields, [{index, value} | updates], vars, exprs) else - raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(key)}" + var = Macro.var(key, __MODULE__) + build_updates(rest, fields, [{index, var} | updates], [var | vars], [value | exprs]) end - end) + else + {:error, key} + end end + defp build_updates([], _fields, updates, vars, exprs), do: {updates, vars, exprs} + + defp simple_argument?({name, _, ctx}) when is_atom(name) and is_atom(ctx), do: true + defp simple_argument?(other), do: Macro.quoted_literal?(other) + # Gets a record key from the given var. defp get(tag, fields, var, key) do - index = find_index(fields, key, 0) + index = + find_index(fields, key, 2) || + raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(key)}" - if index do - quote do - :erlang.element(unquote(index), unquote(var)) - end - else - raise ArgumentError, "record #{inspect(tag)} does not have the key: #{inspect(key)}" + quote do + :erlang.element(unquote(index), unquote(var)) end end - defp find_index([{k, _} | _], k, i), do: i + 2 + defp find_index([{k, _} | _], k, i), do: i defp find_index([{_, _} | t], k, i), do: find_index(t, k, i + 1) defp find_index([], _k, _i), do: nil @@ -480,17 +501,4 @@ defmodule Record do defp join_keyword([], [], acc), do: :lists.reverse(acc) defp join_keyword(rest_fields, _rest_values, acc), do: length(acc) + length(rest_fields) - - defp apply_underscore(fields, keyword) do - case Keyword.fetch(keyword, :_) do - {:ok, default} -> - fields - |> Enum.map(fn {k, _} -> {k, default} end) - |> Keyword.merge(keyword) - |> Keyword.delete(:_) - - :error -> - keyword - end - end end diff --git a/lib/elixir/test/elixir/record_test.exs b/lib/elixir/test/elixir/record_test.exs index 761e8f6a3..084bd06df 100644 --- a/lib/elixir/test/elixir/record_test.exs +++ b/lib/elixir/test/elixir/record_test.exs @@ -7,6 +7,8 @@ defmodule RecordTest do require Record + import ExUnit.CaptureIO + test "extract/2 extracts information from an Erlang file" do assert Record.extract(:file_info, from_lib: "kernel/include/file.hrl") == [ size: :undefined, @@ -127,9 +129,16 @@ defmodule RecordTest do assert match?(user(_: _), user()) refute match?(user(_: "other"), user()) - record = user(user(), _: :_, name: "meg") - assert user(record, :name) == "meg" - assert user(record, :age) == :_ + captured = + capture_io(:stderr, fn -> + expanded = Macro.expand(quote(do: user(user(), _: :_, name: "meg")), __ENV__) + {record, _} = Code.eval_quoted(expanded) + + assert user(record, :name) == "meg" + assert user(record, :age) == :_ + end) + + assert captured =~ "updating a record with a default" end Record.defrecord( |