Include lines in == Compilation error in file ... == slogans (#14538)

Closes #14535.

Author: josevalim

lib/elixir/lib/code.ex

@@ -2078,7 +2078,7 @@ defmodule Code do
     case :code.ensure_loaded(module) do
       {:error, :nofile} = error ->
         if can_await_module_compilation?() do
-          case Kernel.ErrorHandler.ensure_compiled(module, :module, mode) do
+          case Kernel.ErrorHandler.ensure_compiled(module, :module, mode, nil) do
             :found -> {:module, module}
             :deadlock -> {:error, :unavailable}
             :not_found -> {:error, :nofile}

lib/elixir/lib/kernel/cli.ex

@@ -199,8 +199,9 @@ defmodule Kernel.CLI do
     "    " <> String.replace(string, "\n", "\n    ")
   end
 
-  @elixir_internals [:elixir, :elixir_aliases, :elixir_expand, :elixir_compiler, :elixir_module] ++
-                      [:elixir_clauses, :elixir_lexical, :elixir_def, :elixir_map] ++
+  @elixir_internals [:elixir, :elixir_aliases, :elixir_clauses, :elixir_compiler, :elixir_def] ++
+                      [:elixir_def, :elixir_dispatch, :elixir_expand, :elixir_lexical] ++
+                      [:elixir_map, :elixir_module] ++
                       [:elixir_erl, :elixir_erl_clauses, :elixir_erl_compiler, :elixir_erl_pass] ++
                       [Kernel.ErrorHandler, Module.ParallelChecker]
 

lib/elixir/lib/kernel/error_handler.ex

@@ -9,13 +9,13 @@ defmodule Kernel.ErrorHandler do
 
   @spec undefined_function(module, atom, list) :: term
   def undefined_function(module, fun, args) do
-    ensure_loaded(module) or ensure_compiled(module, :module, :raise)
+    ensure_loaded(module) or ensure_compiled(module, :module, :raise, nil)
     :error_handler.undefined_function(module, fun, args)
   end
 
   @spec undefined_lambda(module, fun, list) :: term
   def undefined_lambda(module, fun, args) do
-    ensure_loaded(module) or ensure_compiled(module, :module, :raise)
+    ensure_loaded(module) or ensure_compiled(module, :module, :raise, nil)
     :error_handler.undefined_lambda(module, fun, args)
   end
 
@@ -27,17 +27,22 @@ defmodule Kernel.ErrorHandler do
     end
   end
 
-  @spec ensure_compiled(module, atom, atom) :: :found | :not_found | :deadlock
+  @spec ensure_compiled(module, atom, atom, nil | integer()) :: :found | :not_found | :deadlock
   # Never wait on nil because it should never be defined.
-  def ensure_compiled(nil, _kind, _deadlock) do
+  def ensure_compiled(nil, _kind, _deadlock, _position) do
     :not_found
   end
 
-  def ensure_compiled(module, kind, deadlock) do
+  def ensure_compiled(module, kind, deadlock, position) do
     {compiler_pid, file_pid} = :erlang.get(:elixir_compiler_info)
     ref = :erlang.make_ref()
     modules = :elixir_module.compiler_modules()
-    send(compiler_pid, {:waiting, kind, self(), ref, file_pid, module, modules, deadlock})
+
+    send(
+      compiler_pid,
+      {:waiting, kind, self(), ref, file_pid, position, module, modules, deadlock}
+    )
+
     :erlang.garbage_collect(self())
 
     receive do

lib/elixir/lib/kernel/parallel_compiler.ex

@@ -82,7 +82,7 @@ defmodule Kernel.ParallelCompiler do
     {compiler_pid, file_pid} = :erlang.get(:elixir_compiler_info)
     defining = :elixir_module.compiler_modules()
     on = Enum.map(refs_tasks, fn {_ref, %{pid: pid}} -> pid end)
-    send(compiler_pid, {:waiting, :pmap, self(), ref, file_pid, on, defining, :raise})
+    send(compiler_pid, {:waiting, :pmap, self(), ref, file_pid, nil, on, defining, :raise})
 
     # Now we allow the tasks to run. This step is not strictly
     # necessary but it makes compilation more deterministic by
@@ -416,7 +416,7 @@ defmodule Kernel.ParallelCompiler do
   defp spawn_workers([{pid, found} | t], spawned, waiting, files, result, warnings, errors, state) do
     {files, waiting} =
       case Map.pop(waiting, pid) do
-        {{kind, ref, file_pid, on, _defining, _deadlock}, waiting} ->
+        {%{kind: kind, ref: ref, file_pid: file_pid, on: on}, waiting} ->
           send(pid, {ref, found})
           {update_timing(files, file_pid, {:waiting, kind, on}), waiting}
 
@@ -607,22 +607,22 @@ defmodule Kernel.ParallelCompiler do
   defp without_definition(waiting_list, files) do
     nilify_empty_or_sort(
       for %{pid: file_pid} <- files,
-          {pid, {_, _, ^file_pid, on, _, _}} <- waiting_list,
+          {pid, %{file_pid: ^file_pid, on: on}} <- waiting_list,
           is_atom(on) and not defining?(on, waiting_list),
           do: {pid, :not_found}
     )
   end
 
   defp deadlocked(waiting_list, type, defining?) do
     nilify_empty_or_sort(
-      for {pid, {_, _, _, on, _, ^type}} <- waiting_list,
+      for {pid, %{on: on, deadlock: ^type}} <- waiting_list,
           is_atom(on) and defining?(on, waiting_list) == defining?,
           do: {pid, :deadlock}
     )
   end
 
   defp defining?(on, waiting_list) do
-    Enum.any?(waiting_list, fn {_, {_, _, _, _, defining, _}} -> on in defining end)
+    Enum.any?(waiting_list, fn {_, %{defining: defining}} -> on in defining end)
   end
 
   defp nilify_empty_or_sort([]), do: nil
@@ -689,11 +689,12 @@ defmodule Kernel.ParallelCompiler do
         )
 
       # If we are simply requiring files, we do not add to waiting.
-      {:waiting, _kind, child, ref, _file_pid, _on, _defining, _deadlock} when output == :require ->
+      {:waiting, _kind, child, ref, _file_pid, _position, _on, _defining, _deadlock}
+      when output == :require ->
         send(child, {ref, :not_found})
         spawn_workers(queue, spawned, waiting, files, result, warnings, errors, state)
 
-      {:waiting, kind, child_pid, ref, file_pid, on, defining, deadlock} ->
+      {:waiting, kind, child_pid, ref, file_pid, position, on, defining, deadlock} ->
         # If we already got what we were waiting for, do not put it on waiting.
         # If we're waiting on ourselves, send :found so that we can crash with
         # a better error.
@@ -706,7 +707,17 @@ defmodule Kernel.ParallelCompiler do
             send(child_pid, {ref, reply})
             {waiting, files, result}
           else
-            waiting = Map.put(waiting, child_pid, {kind, ref, file_pid, on, defining, deadlock})
+            waiting =
+              Map.put(waiting, child_pid, %{
+                kind: kind,
+                ref: ref,
+                file_pid: file_pid,
+                position: position,
+                on: on,
+                defining: defining,
+                deadlock: deadlock
+              })
+
             files = update_timing(files, file_pid, :compiling)
             result = Map.put(result, {kind, on}, [child_pid | available_or_pending])
             {waiting, files, result}
@@ -760,7 +771,7 @@ defmodule Kernel.ParallelCompiler do
         terminate(new_files)
 
         return_error(warnings, errors, state, fn ->
-          print_error(file, kind, reason, stack)
+          print_error(file, nil, kind, reason, stack)
           [to_error(file, kind, reason, stack)]
         end)
 
@@ -774,7 +785,7 @@ defmodule Kernel.ParallelCompiler do
           terminate(files)
 
           return_error(warnings, errors, state, fn ->
-            print_error(file.file, :exit, reason, [])
+            print_error(file.file, nil, :exit, reason, [])
             [to_error(file.file, :exit, reason, [])]
           end)
         else
@@ -949,11 +960,11 @@ defmodule Kernel.ParallelCompiler do
         {:current_stacktrace, stacktrace} = Process.info(pid, :current_stacktrace)
         Process.exit(pid, :kill)
 
-        {kind, _, _, on, _, _} = Map.fetch!(waiting, pid)
+        %{kind: kind, on: on, position: position} = Map.fetch!(waiting, pid)
         description = "deadlocked waiting on #{kind} #{inspect(on)}"
         error = CompileError.exception(description: description, file: nil, line: nil)
-        print_error(file, :error, error, stacktrace)
-        {Path.relative_to_cwd(file), on, description, stacktrace}
+        print_error(file, position, :error, error, stacktrace)
+        {Path.relative_to_cwd(file), position, on, description, stacktrace}
       end
 
     IO.puts(:stderr, """
@@ -967,24 +978,25 @@ defmodule Kernel.ParallelCompiler do
       |> Enum.map(&(&1 |> elem(0) |> String.length()))
       |> Enum.max()
 
-    for {file, mod, _, _} <- deadlock do
+    for {file, _, mod, _, _} <- deadlock do
       IO.puts(:stderr, ["  ", String.pad_leading(file, max), " => " | inspect(mod)])
     end
 
     IO.puts(
       :stderr,
       "\nEnsure there are no compile-time dependencies between those files " <>
-        "and that the modules they reference exist and are correctly named\n"
+        "(such as structs or macros) and that the modules they reference exist " <>
+        "and are correctly named\n"
     )
 
-    for {file, _, description, stacktrace} <- deadlock do
+    for {file, position, _, description, stacktrace} <- deadlock do
       file = Path.absname(file)
 
       %{
         severity: :error,
         file: file,
         source: file,
-        position: nil,
+        position: position,
         message: description,
         stacktrace: stacktrace,
         span: nil
@@ -1004,9 +1016,11 @@ defmodule Kernel.ParallelCompiler do
     :ok
   end
 
-  defp print_error(file, kind, reason, stack) do
+  defp print_error(file, position, kind, reason, stack) do
+    position = if is_integer(position), do: ":#{position}", else: ""
+
     IO.write(:stderr, [
-      "\n== Compilation error in file #{Path.relative_to_cwd(file)} ==\n",
+      "\n== Compilation error in file #{Path.relative_to_cwd(file)}#{position} ==\n",
       Kernel.CLI.format_error(kind, reason, stack)
     ])
   end

lib/elixir/src/elixir.erl

@@ -209,16 +209,8 @@ env_for_eval(#{lexical_tracker := Pid} = Env) ->
   end;
 env_for_eval(Opts) when is_list(Opts) ->
   Env = elixir_env:new(),
-
-  Line = case lists:keyfind(line, 1, Opts) of
-    {line, LineOpt} when is_integer(LineOpt) -> LineOpt;
-    false -> ?key(Env, line)
-  end,
-
-  File = case lists:keyfind(file, 1, Opts) of
-    {file, FileOpt} when is_binary(FileOpt) -> FileOpt;
-    false -> ?key(Env, file)
-  end,
+  Line = elixir_utils:get_line(Opts, Env),
+  File = elixir_utils:get_file(Opts, Env),
 
   Module = case lists:keyfind(module, 1, Opts) of
     {module, ModuleOpt} when is_atom(ModuleOpt) -> ModuleOpt;

lib/elixir/src/elixir_aliases.erl

@@ -148,7 +148,7 @@ ensure_loaded(Meta, Module, E) ->
       ok;
 
     _ ->
-      case wait_for_module(Module) of
+      case wait_for_module(Module, Meta, E) of
         found ->
           ok;
 
@@ -163,10 +163,10 @@ ensure_loaded(Meta, Module, E) ->
       end
   end.
 
-wait_for_module(Module) ->
+wait_for_module(Module, Meta, E) ->
   case erlang:get(elixir_compiler_info) of
     undefined -> not_found;
-    _ -> 'Elixir.Kernel.ErrorHandler':ensure_compiled(Module, module, hard)
+    _ -> 'Elixir.Kernel.ErrorHandler':ensure_compiled(Module, module, hard, elixir_utils:get_line(Meta, E))
   end.
 
 %% Receives a list of atoms, binaries or lists

lib/elixir/src/elixir_map.erl

@@ -137,7 +137,7 @@ load_struct_info(Meta, Name, Assocs, E) ->
 
 maybe_load_struct_info(Meta, Name, Assocs, Trace, E) ->
   try
-    case is_open(Name, E) andalso lookup_struct_info_from_data_tables(Name) of
+    case is_open(Name, Meta, E) andalso lookup_struct_info_from_data_tables(Name) of
       %% If I am accessing myself and there is no attribute,
       %% don't invoke the fallback to avoid calling loaded code.
       false when ?key(E, module) =:= Name -> nil;
@@ -185,7 +185,7 @@ load_struct(Meta, Name, Assocs, E) ->
 
 maybe_load_struct(Meta, Name, Assocs, E) ->
   try
-    case is_open(Name, E) andalso elixir_def:external_for(Meta, Name, '__struct__', 1, [def]) of
+    case is_open(Name, Meta, E) andalso elixir_def:external_for(Meta, Name, '__struct__', 1, [def]) of
       %% If I am accessing myself and there is no __struct__ function,
       %% don't invoke the fallback to avoid calling loaded code.
       false when ?key(E, module) =:= Name ->
@@ -232,17 +232,17 @@ assert_struct_assocs(Meta, Assocs, E) ->
   [function_error(Meta, E, ?MODULE, {invalid_key_for_struct, K})
    || {K, _} <- Assocs, not is_atom(K)].
 
-is_open(Name, E) ->
-  in_context(Name, E) orelse ((code:ensure_loaded(Name) /= {module, Name}) andalso wait_for_struct(Name)).
+is_open(Name, Meta, E) ->
+  in_context(Name, E) orelse ((code:ensure_loaded(Name) /= {module, Name}) andalso wait_for_struct(Name, Meta, E)).
 
 in_context(Name, E) ->
   %% We also include the current module because it won't be present
   %% in context module in case the module name is defined dynamically.
   lists:member(Name, [?key(E, module) | ?key(E, context_modules)]).
 
-wait_for_struct(Module) ->
+wait_for_struct(Module, Meta, E) ->
   (erlang:get(elixir_compiler_info) /= undefined) andalso
-    ('Elixir.Kernel.ErrorHandler':ensure_compiled(Module, struct, hard) =:= found).
+    ('Elixir.Kernel.ErrorHandler':ensure_compiled(Module, struct, hard, elixir_utils:get_line(Meta, E)) =:= found).
 
 detail_undef(Name, E) ->
   case in_context(Name, E) andalso (?key(E, function) == nil) of

lib/elixir/src/elixir_utils.erl

@@ -5,7 +5,7 @@
 %% Convenience functions used throughout elixir source code
 %% for ast manipulation and querying.
 -module(elixir_utils).
--export([get_line/1, generated/1,
+-export([get_line/1, get_line/2, get_file/2, generated/1,
   split_last/1, split_opts/1, noop/0, var_context/2,
   characters_to_list/1, characters_to_binary/1, relative_to_cwd/1,
   macro_name/1, returns_boolean/1, caller/4, meta_keep/1,
@@ -176,6 +176,18 @@ get_line(Opts) when is_list(Opts) ->
     _ -> 0
   end.
 
+get_line(Meta, Env) when is_list(Meta) ->
+  case lists:keyfind(line, 1, Meta) of
+    {line, LineOpt} when is_integer(LineOpt) -> LineOpt;
+    false -> ?key(Env, line)
+  end.
+
+get_file(Meta, Env) when is_list(Meta) ->
+  case lists:keyfind(file, 1, Meta) of
+    {file, FileOpt} when is_binary(FileOpt) -> FileOpt;
+    false -> ?key(Env, file)
+  end.
+
 generated([{generated, true} | _] = Meta) -> Meta;
 generated(Meta) -> [{generated, true} | Meta].
 

lib/elixir/test/elixir/kernel/parallel_compiler_test.exs

@@ -380,7 +380,7 @@ defmodule Kernel.ParallelCompilerTest do
           """,
           bar: """
           defmodule BarDeadlock do
-            FooDeadlock.__info__(:module)
+            %FooDeadlock{}
           end
           """
         )
@@ -390,7 +390,7 @@ defmodule Kernel.ParallelCompilerTest do
           fixtures = [foo, bar]
           assert {:error, [bar_error, foo_error], @no_warnings} = compile(fixtures)
 
-          assert %{file: ^bar, position: nil, message: "deadlocked waiting on module FooDeadlock"} =
+          assert %{file: ^bar, position: 2, message: "deadlocked waiting on struct FooDeadlock"} =
                    bar_error
 
           assert %{file: ^foo, position: nil, message: "deadlocked waiting on module BarDeadlock"} =
@@ -402,8 +402,8 @@ defmodule Kernel.ParallelCompilerTest do
       assert msg =~ "parallel_deadlock/bar.ex => FooDeadlock"
       assert msg =~ ~r"== Compilation error in file .+parallel_deadlock/foo\.ex =="
       assert msg =~ "** (CompileError) deadlocked waiting on module BarDeadlock"
-      assert msg =~ ~r"== Compilation error in file .+parallel_deadlock/bar\.ex =="
-      assert msg =~ "** (CompileError) deadlocked waiting on module FooDeadlock"
+      assert msg =~ ~r"== Compilation error in file .+parallel_deadlock/bar\.ex:2 =="
+      assert msg =~ "** (CompileError) deadlocked waiting on struct FooDeadlock"
     end
 
     test "does not deadlock from Code.ensure_compiled" do