Ensure defrecord and defstruct executes its args

Author: José Valim

lib/elixir/lib/file.ex

@@ -1,10 +1,5 @@
 require Record
 
-record = Record.extract(:file_info, from_lib: "kernel/include/file.hrl")
-keys   = :lists.map(&elem(&1, 0), record)
-vals   = :lists.map(&{&1, [], nil}, keys)
-pairs  = :lists.zip(keys, vals)
-
 defmodule File.Stat do
   @moduledoc """
   A struct responsible to hold file information.
@@ -43,7 +38,12 @@ defmodule File.Stat do
   `:universal`, or `:posix`. Default is `:local`.
   """
 
-  defstruct unquote(keys)
+  record = Record.extract(:file_info, from_lib: "kernel/include/file.hrl")
+  keys   = :lists.map(&elem(&1, 0), record)
+  vals   = :lists.map(&{&1, [], nil}, keys)
+  pairs  = :lists.zip(keys, vals)
+
+  defstruct keys
 
   @doc """
   Converts a `File.Stat` struct to a `:file_info` record.

lib/elixir/lib/inspect/algebra.ex

@@ -116,16 +116,16 @@ defmodule Inspect.Algebra do
 
   require Record
 
-  @opaque doc_cons :: {:doc_cons, t, t}
+  @typep doc_cons :: {:doc_cons, t, t}
   Record.defrecordp :doc_cons, left: :doc_nil, right: :doc_nil
 
-  @opaque doc_nest :: {:doc_nest, non_neg_integer, t}
+  @typep doc_nest :: {:doc_nest, non_neg_integer, t}
   Record.defrecordp :doc_nest, indent: 1, doc: :doc_nil
 
-  @opaque doc_break :: {:doc_break, binary}
+  @typep doc_break :: {:doc_break, binary}
   Record.defrecordp :doc_break, str: " "
 
-  @opaque doc_group :: {:doc_group, t}
+  @typep doc_group :: {:doc_group, t}
   Record.defrecordp :doc_group, doc: :doc_nil
 
   defmacrop is_doc(doc) do

lib/elixir/lib/kernel.ex

@@ -3299,21 +3299,34 @@ defmodule Kernel do
   In case a struct does not declare a field type, it defaults to `term`.
   """
   defmacro defstruct(kv) do
-    kv = Macro.escape(kv, unquote: true)
-    quote bind_quoted: [kv: kv] do
-      # Expand possible macros that return KVs.
-      kv = Macro.expand(kv, __ENV__)
-      {fields, types} = Record.Backend.split_fields_and_types(:defstruct, kv)
-
-      if :code.ensure_loaded(Kernel.Typespec) == {:module, Kernel.Typespec} and
-         not Kernel.Typespec.defines_type?(__MODULE__, :t, 0) do
-        @type t :: %{unquote_splicing(types), __struct__: __MODULE__}
+    {fields, types} = Record.Backend.split_fields_and_types(:defstruct, kv)
+
+    fields =
+      quote bind_quoted: [fields: fields] do
+        fields = Enum.map(fields, fn
+          { key, _ } = pair when is_atom(key) -> pair
+          key when is_atom(key) -> { key, nil }
+          other -> raise ArgumentError, message: "struct fields must be atoms, got: #{inspect other}"
+        end)
+
+        def __struct__() do
+          %{unquote_splicing(Macro.escape(fields)), __struct__: __MODULE__}
+        end
       end
 
-      def __struct__() do
-        %{unquote_splicing(fields), __struct__: __MODULE__}
+    types =
+      case bootstraped?(Kernel.Typespec) do
+        true ->
+          quote do
+            unless Kernel.Typespec.defines_type?(__MODULE__, :t, 0) do
+              @type t :: %{unquote_splicing(types), __struct__: __MODULE__}
+            end
+          end
+        false ->
+          nil
       end
-    end
+
+    [types, fields]
   end
 
   @doc ~S"""

lib/elixir/lib/record.ex

@@ -155,14 +155,9 @@ defmodule Record do
 
   """
   defmacro defrecord(name, tag \\ nil, kv) do
-    kv = Macro.escape(kv, unquote: true)
-
     quote bind_quoted: [name: name, tag: tag, kv: kv] do
       tag = tag || name
-      kv  = Macro.expand(kv, __ENV__)
-
-      {fields, _types} = Record.Backend.split_fields_and_types(:defrecord, kv)
-      fields = Macro.escape(fields)
+      fields = Macro.escape(kv)
 
       defmacro(unquote(name)(args \\ [])) do
         Record.Backend.access(unquote(tag), unquote(fields), args, __CALLER__)
@@ -178,14 +173,9 @@ defmodule Record do
   Same as `defrecord/3` but generates private macros.
   """
   defmacro defrecordp(name, tag \\ nil, kv) do
-    kv = Macro.escape(kv, unquote: true)
-
     quote bind_quoted: [name: name, tag: tag, kv: kv] do
       tag = tag || name
-      kv  = Macro.expand(kv, __ENV__)
-
-      {fields, _types} = Record.Backend.split_fields_and_types(:defrecordp, kv)
-      fields = Macro.escape(fields)
+      fields = Macro.escape(kv)
 
       defmacrop(unquote(name)(args \\ [])) do
         Record.Backend.access(unquote(tag), unquote(fields), args, __CALLER__)

lib/elixir/lib/record/backend.ex

@@ -7,30 +7,26 @@ defmodule Record.Backend do
 
   This logic is shared by records and structs.
   """
-  def split_fields_and_types(tag, kv) when is_list(kv) do
-    split_fields_and_types(tag, kv, [], [])
-  end
-
-  def split_fields_and_types(tag, other) do
-    raise ArgumentError, message: "#{tag} fields must be a keyword list, got: #{Macro.to_string other}"
+  def split_fields_and_types(tag, kv) do
+    if Keyword.keyword?(kv) do
+      split_fields_and_types(tag, kv, [], [])
+    else
+      {kv, []}
+    end
   end
 
   defp split_fields_and_types(tag, [{field, {:::, _, [default, type]}}|t], fields, types) do
     split_fields_and_types(tag, t, [{field, default}|fields], [{field, type}|types])
   end
 
-  defp split_fields_and_types(tag, [{field, default}|t], fields, types) when is_atom(field) do
+  defp split_fields_and_types(tag, [{field, default}|t], fields, types) do
     split_fields_and_types(tag, t, [{field, default}|fields], [{field, quote(do: term)}|types])
   end
 
-  defp split_fields_and_types(tag, [field|t], fields, types) when is_atom(field) do
+  defp split_fields_and_types(tag, [field|t], fields, types) do
     split_fields_and_types(tag, t, [{field, nil}|fields], [{field, quote(do: term)}|types])
   end
 
-  defp split_fields_and_types(tag, [other|_], _fields, _types) do
-    raise ArgumentError, message: "#{tag} fields must be atoms, got: #{Macro.to_string other}"
-  end
-
   defp split_fields_and_types(_tag, [], fields, types) do
     {:lists.reverse(fields), :lists.reverse(types)}
   end

lib/elixir/src/elixir_map.erl

@@ -89,42 +89,33 @@ translate_struct(Meta, Name, {'%{}', MapMeta, Args}, S) ->
 
 %% Helpers
 
-load_struct(Meta, Module, S) ->
+load_struct(Meta, Name, S) ->
   Local =
-    elixir_module:is_open(Module) andalso
+    elixir_module:is_open(Name) andalso
     (case lists:keyfind(struct, 1, Meta) of
       {struct, context} -> true;
-      _ -> wait_for_struct(Module)
+      _ -> wait_for_struct(Name)
     end),
 
-  case Local of
-    true ->
-      try
-        (elixir_locals:local_for(Module, '__struct__', 0, def))()
-      catch
-        error:undef  -> get_struct(Meta, Module, S);
-        error:badarg -> get_struct(Meta, Module, S)
-      end;
-    false ->
-      get_struct(Meta, Module, S)
-  end.
-
-get_struct(Meta, Module, S) ->
-  case code:ensure_loaded(Module) of
-    {module, Module} ->
-      case erlang:function_exported(Module, '__struct__', 0) of
-        true -> Module:'__struct__'();
-        false -> raise_struct(Meta, Module, S)
-      end;
-    {error, _} ->
-      raise_struct(Meta, Module, S)
+  try
+    case Local of
+      true ->
+        try
+          (elixir_locals:local_for(Name, '__struct__', 0, def))()
+        catch
+          error:undef  -> Name:'__struct__'();
+          error:badarg -> Name:'__struct__'()
+        end;
+      false ->
+        Name:'__struct__'()
+    end
+  catch
+    error:undef ->
+      Inspected = elixir_aliases:inspect(Name),
+      compile_error(Meta, S#elixir_scope.file, "~ts.__struct__/0 is undefined, "
+        "cannot expand struct ~ts", [Inspected, Inspected])
   end.
 
-raise_struct(Meta, Module, S) ->
-  Inspected = elixir_aliases:inspect(Module),
-  compile_error(Meta, S#elixir_scope.file, "~ts.__struct__/0 is undefined, "
-    "cannot expand struct ~ts", [Inspected, Inspected]).
-
 wait_for_struct(Module) ->
   case erlang:get(elixir_compiler_pid) of
     undefined ->

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -203,10 +203,10 @@ defmodule Kernel.ErrorsTest do
 
   test :struct_fields_on_defstruct do
     assert_compile_fail ArgumentError,
-      "defstruct fields must be a keyword list, got: my_fields",
+      "struct fields must be atoms, got: 1",
       '''
       defmodule TZ do
-        defstruct my_fields
+        defstruct [1, 2, 3]
       end
       '''
   end

lib/elixir/test/elixir/map_test.exs

@@ -129,7 +129,7 @@ defmodule MapTest do
       defstruct []
     end
 
-    defstruct name: "josé", nested: %NestedUser{}
+    defstruct name: "josé", nested: struct(NestedUser)
 
     def new do
       %LocalUser{}