Condititionally compile and lazily compute __STACKTRACE__ (elixir-lan…

…g#7648)

Erlang/OTP 20 warns if the stacktrace is read outside of a
catch/rescue. This commit mirrors this behaviour by consistently
warning on `System.stacktrace/0` being used outside of a
catch/rescue.

Erlang/OTP 21 warns whenever System.stacktrace/:erlang.get_stacktrace
are used. Therefore we need to promote the usage of `__STACKTRACE__`
and make sure to conditionally compile it according to the OTP version.
This requires changes to the Exception normalization mechanism
so we compute `__STACKTRACE__` only when strictly required.

In future Elixir releases, `System.stacktrace/0` will warn when
used even inside catch/rescue.

CHANGELOG.md

@@ -62,7 +62,6 @@
   * [Code] Deprecate `Code.load_file/2` in favor of `Code.compile_file/2`
   * [Code] Deprecate `Code.loaded_files/0` in favor of `Code.required_files/0`
   * [Code] Deprecate `Code.unload_files/1` in favor of `Code.unrequire_files/1`
-  * [Exception] Deprecate `Exception.normalize/2` and `Exception.format/2` as a stacktrace is now explicitly required
 
 ### 4. Hard-deprecations
 

lib/eex/test/eex_test.exs

@@ -4,8 +4,7 @@ require EEx
 
 defmodule EExTest.Compiled do
   def before_compile do
-    fill_in_stacktrace()
-    {__ENV__.line, hd(tl(System.stacktrace()))}
+    {__ENV__.line, hd(tl(get_stacktrace()))}
   end
 
   EEx.function_from_string(:def, :string_sample, "<%= a + b %>", [:a, :b])
@@ -19,21 +18,19 @@ defmodule EExTest.Compiled do
   def file_sample(arg), do: private_file_sample(arg)
 
   def after_compile do
-    fill_in_stacktrace()
-    {__ENV__.line, hd(tl(System.stacktrace()))}
+    {__ENV__.line, hd(tl(get_stacktrace()))}
   end
 
   @file "unknown"
   def unknown do
-    fill_in_stacktrace()
-    {__ENV__.line, hd(tl(System.stacktrace()))}
+    {__ENV__.line, hd(tl(get_stacktrace()))}
   end
 
-  defp fill_in_stacktrace do
+  defp get_stacktrace do
     try do
       :erlang.error("failed")
-    catch
-      :error, _ -> System.stacktrace()
+    rescue
+      _ -> __STACKTRACE__
     end
   end
 end
@@ -447,13 +444,13 @@ defmodule EExTest do
       file = to_charlist(Path.relative_to_cwd(__ENV__.file))
 
       assert EExTest.Compiled.before_compile() ==
-               {8, {EExTest.Compiled, :before_compile, 0, [file: file, line: 7]}}
+               {7, {EExTest.Compiled, :before_compile, 0, [file: file, line: 7]}}
 
       assert EExTest.Compiled.after_compile() ==
-               {23, {EExTest.Compiled, :after_compile, 0, [file: file, line: 22]}}
+               {21, {EExTest.Compiled, :after_compile, 0, [file: file, line: 21]}}
 
       assert EExTest.Compiled.unknown() ==
-               {29, {EExTest.Compiled, :unknown, 0, [file: 'unknown', line: 28]}}
+               {26, {EExTest.Compiled, :unknown, 0, [file: 'unknown', line: 26]}}
     end
   end
 

lib/elixir/lib/access.ex

@@ -238,10 +238,8 @@ defmodule Access do
 
   defmacrop raise_undefined_behaviour(exception, module, top) do
     quote do
-      stacktrace = System.stacktrace()
-
       exception =
-        case stacktrace do
+        case __STACKTRACE__ do
           [unquote(top) | _] ->
             reason = "#{inspect(unquote(module))} does not implement the Access behaviour"
             %{unquote(exception) | reason: reason}
@@ -250,7 +248,7 @@ defmodule Access do
             unquote(exception)
         end
 
-      reraise exception, stacktrace
+      reraise exception, __STACKTRACE__
     end
   end
 

lib/elixir/lib/exception.ex

@@ -2,10 +2,8 @@ defmodule Exception do
   @moduledoc """
   Functions to format throw/catch/exit and exceptions.
 
-  Note that stacktraces in Elixir are updated on throw,
-  errors and exits. For example, at any given moment,
-  `System.stacktrace/0` will return the stacktrace for the
-  last throw/error/exit that occurred in the current process.
+  Note that stacktraces in Elixir are only available inside
+  catch and rescue by using the `__STACKTRACE__/0` variable.
 
   Do not rely on the particular format returned by the `format*`
   functions in this module. They may be changed in future releases
@@ -74,11 +72,6 @@ defmodule Exception do
     end
   end
 
-  @deprecated "Use normalize/3 with an explicit stacktrace instead"
-  def normalize(kind, payload) do
-    normalize(kind, payload, nil)
-  end
-
   @doc """
   Normalizes an exception, converting Erlang exceptions
   to Elixir exceptions.
@@ -87,44 +80,29 @@ defmodule Exception do
   normalizes only `:error`, returning the untouched payload
   for others.
 
-  The third argument, a stacktrace, is optional. If it is
-  not supplied `System.stacktrace/0` will sometimes be used
-  to get additional information for the `kind` `:error`. If
-  the stacktrace is unknown and `System.stacktrace/0` would
-  not return the stacktrace corresponding to the exception
-  an empty stacktrace, `[]`, must be used.
+  The third argument is the stacktrace which is used to enrich
+  a normalized error with more information. It is only used when
+  the kind is an error.
   """
   @spec normalize(:error, any, stacktrace) :: t
   @spec normalize(non_error_kind, payload, stacktrace) :: payload when payload: var
-  def normalize(kind, payload, stacktrace)
-
-  def normalize(:error, exception, stacktrace) do
-    if exception?(exception) do
-      exception
-    else
-      ErlangError.normalize(exception, stacktrace)
-    end
-  end
-
-  def normalize(_kind, payload, _stacktrace) do
-    payload
-  end
+  def normalize(kind, payload, stacktrace \\ [])
+  def normalize(:error, %_{__exception__: true} = payload, _stacktrace), do: payload
+  def normalize(:error, payload, stacktrace), do: ErlangError.normalize(payload, stacktrace)
+  def normalize(_kind, payload, _stacktrace), do: payload
 
   @doc """
   Normalizes and formats any throw/error/exit.
 
   The message is formatted and displayed in the same
   format as used by Elixir's CLI.
 
-  The third argument, a stacktrace, is optional. If it is
-  not supplied `System.stacktrace/0` will sometimes be used
-  to get additional information for the `kind` `:error`. If
-  the stacktrace is unknown and `System.stacktrace/0` would
-  not return the stacktrace corresponding to the exception
-  an empty stacktrace, `[]`, must be used.
+  The third argument is the stacktrace which is used to enrich
+  a normalized error with more information. It is only used when
+  the kind is an error.
   """
-  @spec format_banner(kind, any, stacktrace | nil) :: String.t()
-  def format_banner(kind, exception, stacktrace \\ nil)
+  @spec format_banner(kind, any, stacktrace) :: String.t()
+  def format_banner(kind, exception, stacktrace \\ [])
 
   def format_banner(:error, exception, stacktrace) do
     exception = normalize(:error, exception, stacktrace)
@@ -143,11 +121,6 @@ defmodule Exception do
     "** (EXIT from #{inspect(pid)}) " <> format_exit(reason, <<"\n    ">>)
   end
 
-  @deprecated "Use format/3 with an explicit stacktrace instead"
-  def format(kind, payload) do
-    format(kind, payload, nil)
-  end
-
   @doc """
   Normalizes and formats throw/errors/exits and stacktraces.
 
@@ -157,15 +130,14 @@ defmodule Exception do
   Note that `{:EXIT, pid}` do not generate a stacktrace though
   (as they are retrieved as messages without stacktraces).
   """
-  @spec format(kind, any, stacktrace | nil) :: String.t()
-  def format(kind, payload, stacktrace)
+  @spec format(kind, any, stacktrace) :: String.t()
+  def format(kind, payload, stacktrace \\ [])
 
   def format({:EXIT, _} = kind, any, _) do
     format_banner(kind, any)
   end
 
   def format(kind, payload, stacktrace) do
-    stacktrace = stacktrace || System.stacktrace()
     message = format_banner(kind, payload, stacktrace)
 
     case stacktrace do
@@ -1200,7 +1172,7 @@ defmodule ErlangError do
 
   def normalize({:badkey, key}, stacktrace) do
     term =
-      case ensure_stacktrace(stacktrace) do
+      case stacktrace do
         [{Map, :get_and_update!, [map, _, _], _} | _] -> map
         [{Map, :update!, [map, _, _], _} | _] -> map
         [{:maps, :update, [_, _, map], _} | _] -> map
@@ -1228,13 +1200,12 @@ defmodule ErlangError do
   end
 
   def normalize(:undef, stacktrace) do
-    stacktrace = ensure_stacktrace(stacktrace)
     {mod, fun, arity} = from_stacktrace(stacktrace)
     %UndefinedFunctionError{module: mod, function: fun, arity: arity}
   end
 
   def normalize(:function_clause, stacktrace) do
-    {mod, fun, arity} = from_stacktrace(ensure_stacktrace(stacktrace))
+    {mod, fun, arity} = from_stacktrace(stacktrace)
     %FunctionClauseError{module: mod, function: fun, arity: arity}
   end
 
@@ -1246,18 +1217,6 @@ defmodule ErlangError do
     %ErlangError{original: other}
   end
 
-  defp ensure_stacktrace(nil) do
-    try do
-      :erlang.get_stacktrace()
-    rescue
-      _ -> []
-    end
-  end
-
-  defp ensure_stacktrace(stacktrace) do
-    stacktrace
-  end
-
   defp from_stacktrace([{module, function, args, _} | _]) when is_list(args) do
     {module, function, length(args)}
   end

lib/elixir/lib/kernel.ex

@@ -1759,23 +1759,16 @@ defmodule Kernel do
 
   Works like `raise/1` but does not generate a new stacktrace.
 
-  Notice that `System.stacktrace/0` returns the stacktrace
-  of the last exception. That said, it is common to assign
-  the stacktrace as the first expression inside a `rescue`
-  clause as any other exception potentially raised (and
-  rescued) between the rescue clause and the raise call
-  may change the `System.stacktrace/0` value.
+  Notice that `__STACKTRACE__` can be used inside catch/rescue
+  to retrieve the current stacktrace.
 
   ## Examples
 
       try do
         raise "oops"
       rescue
         exception ->
-          stacktrace = System.stacktrace
-          if Exception.message(exception) == "oops" do
-            reraise exception, stacktrace
-          end
+          reraise exception, __STACKTRACE__
       end
 
   """
@@ -1818,8 +1811,7 @@ defmodule Kernel do
         raise "oops"
       rescue
         exception ->
-          stacktrace = System.stacktrace
-          reraise WrapperError, [exception: exception], stacktrace
+          reraise WrapperError, [exception: exception], __STACKTRACE__
       end
 
   """

lib/elixir/lib/kernel/special_forms.ex

@@ -1709,7 +1709,9 @@ defmodule Kernel.SpecialForms do
   pattern matching (similar to the `case` special form).
 
   Note that calls inside `try/1` are not tail recursive since the VM
-  needs to keep the stacktrace in case an exception happens.
+  needs to keep the stacktrace in case an exception happens. To
+  retrieve the stacktrace, access `__STACKTRACE__/0` inside the `rescue`
+  or `catch` clause.
 
   ## `rescue` clauses
 

lib/elixir/lib/system.ex

@@ -445,14 +445,19 @@ defmodule System do
   end
 
   @doc """
-  Last exception stacktrace.
+  Deprecated mechanism to retrieve the last exception stacktrace.
+
+  Accessing the stacktrace outside of a rescue/catch is deprecated.
+  If you want to support only Elixir v1.7+, you must access
+  `__STACKTRACE__/0` inside a rescue/catch. If you want to support
+  earlier Elixir versions, move `System.stacktrace/0` inside a rescue/catch.
 
   Note that the Erlang VM (and therefore this function) does not
   return the current stacktrace but rather the stacktrace of the
   latest exception.
-
-  Inlined by the compiler.
   """
+  # TODO: Fully deprecate it on Elixir v1.9.
+  # It is currently partially deprecated in elixir_dispatch.erl
   def stacktrace do
     :erlang.get_stacktrace()
   end

lib/elixir/src/elixir_dispatch.erl

@@ -329,11 +329,24 @@ elixir_imported_macros() ->
 
 %% Inline common cases.
 check_deprecation(Meta, ?kernel, to_char_list, 1, E) ->
-  elixir_errors:warn(?line(Meta), ?key(E, file), "Use Kernel.to_charlist/1 instead");
+  Message = "Kernel.to_char_list/1 is deprecated. Use Kernel.to_charlist/1 instead",
+  elixir_errors:warn(?line(Meta), ?key(E, file), Message);
 check_deprecation(_, ?kernel, _, _, _) ->
   ok;
 check_deprecation(_, erlang, _, _, _) ->
   ok;
+check_deprecation(Meta, 'Elixir.System', stacktrace, 0, #{contextual_vars := Vars} = E) ->
+  case lists:member('__STACKTRACE__', Vars) of
+    true ->
+      ok;
+    false ->
+      Message =
+        "System.stacktrace/0 outside of rescue/catch clauses is deprecated. "
+          "If you want to support only Elixir v1.7+, you must access __STACKTRACE__ "
+          "inside a rescue/catch. If you want to support earlier Elixir versions, "
+          "move System.stacktrace/0 inside a rescue/catch",
+      elixir_errors:warn(?line(Meta), ?key(E, file), Message)
+  end;
 check_deprecation(Meta, Receiver, Name, Arity, E) ->
   case (get(elixir_compiler_dest) == undefined) andalso is_module_loaded(Receiver) andalso
         erlang:function_exported(Receiver, '__info__', 1) andalso get_deprecations(Receiver) of

lib/elixir/src/elixir_erl_for.erl

@@ -177,15 +177,30 @@ build_into(Ann, Clauses, Expr, Into, Uniq, S) ->
         [Done],
         [],
         [{call, Ann, Fun, [Done, {atom, Ann, done}]}]}],
-      [{clause, Ann,
+      [stacktrace_clause(Ann, Fun, Acc, Kind, Reason, Stack)],
+      []},
+
+  {{block, Ann, [MatchExpr, TryExpr]}, SD}.
+
+stacktrace_clause(Ann, Fun, Acc, Kind, Reason, Stack) ->
+  Release = erlang:system_info(otp_release),
+
+  if
+    Release == "19"; Release == "20" ->
+      {clause, Ann,
         [{tuple, Ann, [Kind, Reason, {var, Ann, '_'}]}],
         [],
         [{match, Ann, Stack, elixir_erl:remote(Ann, erlang, get_stacktrace, [])},
          {call, Ann, Fun, [Acc, {atom, Ann, halt}]},
-         elixir_erl:remote(Ann, erlang, raise, [Kind, Reason, Stack])]}],
-      []},
+         elixir_erl:remote(Ann, erlang, raise, [Kind, Reason, Stack])]};
 
-  {{block, Ann, [MatchExpr, TryExpr]}, SD}.
+    true ->
+      {clause, Ann,
+        [{tuple, Ann, [Kind, Reason, Stack]}],
+        [],
+        [{call, Ann, Fun, [Acc, {atom, Ann, halt}]},
+         elixir_erl:remote(Ann, erlang, raise, [Kind, Reason, Stack])]}
+  end.
 
 %% Helpers