Soft deprecation of LoadError in runtime

c42f · c42f · commit 5e3e8fb8ccb3 · 2019-05-01T14:13:05.000+10:00
Remove use of LoadError from the runtime now that backtraces are more
reliable.  It's left in boot.jl for now for 1.x compatibility.

Compatibility:

It's difficult to deprecate this in a fully backward compatible way,
because packages commonly test macro expansion using constructs like

    @ test_throws LoadError macroexpand(:(@ some_pkg_macro))

to make this into as minor a change as possible, grandfather special
rules for LoadError into stdlib/Test.
diff --git a/base/boot.jl b/base/boot.jl
@@ -306,6 +306,7 @@ AssertionError() = AssertionError("")
 
 abstract type WrappedException <: Exception end
 
+# Usage of LoadError is deprecated in the runtime. It's left here for compatibility.
 struct LoadError <: WrappedException
     file::AbstractString
     line::Int
diff --git a/base/docs/basedocs.jl b/base/docs/basedocs.jl
@@ -1914,6 +1914,9 @@ AssertionError
 
 An error occurred while [`include`](@ref Base.include)ing, [`require`](@ref Base.require)ing, or [`using`](@ref) a file. The error specifics
 should be available in the `.error` field.
+
+!!! compat "Julia 1.3"
+    LoadError is deprecated because it is no longer emitted by the runtime as of Julia 1.3.
 """
 LoadError
 
diff --git a/src/ast.c b/src/ast.c
@@ -889,11 +889,7 @@ jl_value_t *jl_parse_eval_all(const char *fname,
     ctx->module = old_module;
     jl_ast_ctx_leave(ctx);
     if (err) {
-        if (jl_loaderror_type == NULL)
-            jl_rethrow();
-        else
-            jl_throw(jl_new_struct(jl_loaderror_type, form, result,
-                                   jl_current_exception()));
+        jl_rethrow();
     }
     JL_GC_POP();
     return result;
@@ -1028,32 +1024,14 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
     size_t world = jl_world_counter;
     ptls->world_age = world;
     jl_value_t *result;
-    JL_TRY {
-        margs[0] = jl_toplevel_eval(*ctx, margs[0]);
-        jl_method_instance_t *mfunc = jl_method_lookup(jl_gf_mtable(margs[0]), margs, nargs, 1, world);
-        if (mfunc == NULL) {
-            jl_method_error((jl_function_t*)margs[0], margs, nargs, world);
-            // unreachable
-        }
-        *ctx = mfunc->def.method->module;
-        result = jl_invoke(mfunc, margs, nargs);
-    }
-    JL_CATCH {
-        if (jl_loaderror_type == NULL) {
-            jl_rethrow();
-        }
-        else {
-            jl_value_t *lno = margs[1];
-            jl_value_t *file = jl_fieldref(lno, 1);
-            if (jl_is_symbol(file))
-                margs[0] = jl_cstr_to_string(jl_symbol_name((jl_sym_t*)file));
-            else
-                margs[0] = jl_cstr_to_string("<macrocall>");
-            margs[1] = jl_fieldref(lno, 0); // extract and allocate line number
-            jl_throw(jl_new_struct(jl_loaderror_type, margs[0], margs[1],
-                                   jl_current_exception()));
-        }
+    margs[0] = jl_toplevel_eval(*ctx, margs[0]);
+    jl_method_instance_t *mfunc = jl_method_lookup(jl_gf_mtable(margs[0]), margs, nargs, 1, world);
+    if (mfunc == NULL) {
+        jl_method_error((jl_function_t*)margs[0], margs, nargs, world);
+        // unreachable
     }
+    *ctx = mfunc->def.method->module;
+    result = jl_invoke(mfunc, margs, nargs);
     ptls->world_age = last_age;
     JL_GC_POP();
     return result;
diff --git a/src/init.c b/src/init.c
@@ -902,7 +902,6 @@ void jl_get_builtin_hooks(void)
 #endif
     jl_argumenterror_type  = (jl_datatype_t*)core("ArgumentError");
     jl_methoderror_type    = (jl_datatype_t*)core("MethodError");
-    jl_loaderror_type      = (jl_datatype_t*)core("LoadError");
     jl_initerror_type      = (jl_datatype_t*)core("InitError");
 
     jl_weakref_type = (jl_datatype_t*)core("WeakRef");
diff --git a/src/jltypes.c b/src/jltypes.c
@@ -108,7 +108,6 @@ jl_datatype_t *jl_errorexception_type;
 jl_datatype_t *jl_argumenterror_type;
 jl_datatype_t *jl_typeerror_type;
 jl_datatype_t *jl_methoderror_type;
-jl_datatype_t *jl_loaderror_type;
 jl_datatype_t *jl_initerror_type;
 jl_datatype_t *jl_undefvarerror_type;
 jl_datatype_t *jl_lineinfonode_type;
diff --git a/src/julia.h b/src/julia.h
@@ -586,7 +586,6 @@ extern JL_DLLEXPORT jl_datatype_t *jl_abstractstring_type JL_GLOBALLY_ROOTED;
 extern JL_DLLEXPORT jl_datatype_t *jl_string_type JL_GLOBALLY_ROOTED;
 extern JL_DLLEXPORT jl_datatype_t *jl_errorexception_type JL_GLOBALLY_ROOTED;
 extern JL_DLLEXPORT jl_datatype_t *jl_argumenterror_type JL_GLOBALLY_ROOTED;
-extern JL_DLLEXPORT jl_datatype_t *jl_loaderror_type JL_GLOBALLY_ROOTED;
 extern JL_DLLEXPORT jl_datatype_t *jl_initerror_type JL_GLOBALLY_ROOTED;
 extern JL_DLLEXPORT jl_datatype_t *jl_typeerror_type JL_GLOBALLY_ROOTED;
 extern JL_DLLEXPORT jl_datatype_t *jl_methoderror_type JL_GLOBALLY_ROOTED;
diff --git a/src/rtutils.c b/src/rtutils.c
@@ -932,15 +932,6 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt
         }
         n += jl_printf(out, "]");
     }
-    else if (vt == jl_loaderror_type) {
-        n += jl_printf(out, "LoadError(at ");
-        n += jl_static_show_x(out, *(jl_value_t**)v, depth);
-        // Access the field directly to avoid allocation
-        n += jl_printf(out, " line %" PRIdPTR, ((intptr_t*)v)[1]);
-        n += jl_printf(out, ": ");
-        n += jl_static_show_x(out, ((jl_value_t**)v)[2], depth);
-        n += jl_printf(out, ")");
-    }
     else if (vt == jl_errorexception_type) {
         n += jl_printf(out, "ErrorException(");
         n += jl_static_show_x(out, *(jl_value_t**)v, depth);
diff --git a/stdlib/Printf/test/runtests.jl b/stdlib/Printf/test/runtests.jl
@@ -2,20 +2,6 @@
 
 using Test, Printf
 
-# this macro tests for exceptions thrown at macro expansion
-macro test_me(ty, ex)
-    return quote
-        @test_throws $(esc(ty)) try
-            $(esc(ex))
-        catch err
-            @test err isa LoadError
-            @test err.file === $(string(__source__.file))
-            @test err.line === $(__source__.line)
-            rethrow(err.error)
-        end
-    end
-end
-
 # printf
 # int
 @test (@sprintf "%d" typemax(Int64)) == "9223372036854775807"
@@ -205,14 +191,14 @@ end
 # escape %
 @test (@sprintf "%%") == "%"
 @test (@sprintf "%%s") == "%s"
-@test_me ArgumentError("invalid printf format string: \"%\"") @macroexpand(@sprintf "%") #" (fixes syntax highlighting)
+@test_throws ArgumentError("invalid printf format string: \"%\"") @macroexpand(@sprintf "%") #" (fixes syntax highlighting)
 
 # argument count
-@test_me ArgumentError("@sprintf: wrong number of arguments (0) should be (1)") @macroexpand(@sprintf "%s")
-@test_me ArgumentError("@sprintf: wrong number of arguments (2) should be (1)") @macroexpand(@sprintf "%s" "1" "2")
+@test_throws ArgumentError("@sprintf: wrong number of arguments (0) should be (1)") @macroexpand(@sprintf "%s")
+@test_throws ArgumentError("@sprintf: wrong number of arguments (2) should be (1)") @macroexpand(@sprintf "%s" "1" "2")
 
 # no interpolation
-@test_me ArgumentError("@sprintf: format must be a plain static string (no interpolation or prefix)") @macroexpand(@sprintf "$n")
+@test_throws ArgumentError("@sprintf: format must be a plain static string (no interpolation or prefix)") @macroexpand(@sprintf "$n")
 
 # type width specifier parsing (ignored)
 @test (@sprintf "%llf" 1.2) == "1.200000"
@@ -258,7 +244,7 @@ end
 # invalid format specifiers, not "diouxXDOUeEfFgGaAcCsSpn"
 for c in "bBhHIjJkKlLmMNPqQrRtTvVwWyYzZ"
     fmt_str = string("%", c)
-    @test_me ArgumentError("@sprintf: first argument must be a format string") @macroexpand(@sprintf $fmt_str 1)
+    @test_throws ArgumentError("@sprintf: first argument must be a format string") @macroexpand(@sprintf $fmt_str 1)
 end
 
 # combo
@@ -271,7 +257,7 @@ end
 @test (@sprintf "%s %s %s %d %d %d %f %f %f" Any[10^x+y for x=1:3,y=1:3 ]...) == "11 101 1001 12 102 1002 13.000000 103.000000 1003.000000"
 
 # @printf
-@test_me ArgumentError("@printf: first or second argument must be a format string") @macroexpand(@printf 1)
+@test_throws ArgumentError("@printf: first or second argument must be a format string") @macroexpand(@printf 1)
 
 # Check bug with trailing nul printing BigFloat
 @test (@sprintf("%.330f", BigFloat(1)))[end] != '\0'
@@ -286,10 +272,10 @@ end
 @test (@sprintf("%d\u0f00%d", 1, 2)) == "1\u0f002"
 @test (@sprintf("%d\U0001ffff%d", 1, 2)) == "1\U0001ffff2"
 @test (@sprintf("%d\u2203%d\u0203", 1, 2)) == "1\u22032\u0203"
-@test_me ArgumentError @macroexpand(@sprintf("%y%d", 1, 2))
-@test_me ArgumentError @macroexpand(@sprintf("%\u00d0%d", 1, 2))
-@test_me ArgumentError @macroexpand(@sprintf("%\u0f00%d", 1, 2))
-@test_me ArgumentError @macroexpand(@sprintf("%\U0001ffff%d", 1, 2))
+@test_throws ArgumentError @macroexpand(@sprintf("%y%d", 1, 2))
+@test_throws ArgumentError @macroexpand(@sprintf("%\u00d0%d", 1, 2))
+@test_throws ArgumentError @macroexpand(@sprintf("%\u0f00%d", 1, 2))
+@test_throws ArgumentError @macroexpand(@sprintf("%\U0001ffff%d", 1, 2))
 
 # test at macro execution time
 @test_throws ArgumentError("@sprintf: wrong number of arguments (2) should be (3)") (@sprintf "%d%d%d" 1:2...)
diff --git a/stdlib/Test/src/Test.jl b/stdlib/Test/src/Test.jl
@@ -577,18 +577,24 @@ end
 
 # An internal function, called by the code generated by @test_throws
 # to evaluate and catch the thrown exception - if it exists
-function do_test_throws(result::ExecutionResult, @nospecialize(orig_expr), @nospecialize(extype))
+function do_test_throws(result::ExecutionResult, @nospecialize(orig_expr), @nospecialize(expected_exc))
     if isa(result, Threw)
         # Check that the right type of exception was thrown
         success = false
         exc = result.exception
-        if isa(extype, Type)
-            success = isa(exc, extype)
+        # NB: The use of LoadError in Base is deprecated, but in order to limit
+        # the breakage in package tests we add extra logic here.
+        if isa(expected_exc, Type)
+            success = isa(exc, expected_exc) ||
+                      (expected_exc == LoadError && exc isa Exception)  # deprecated
         else
-            if isa(exc, typeof(extype))
+            if expected_exc isa LoadError && !(exc isa LoadError) && typeof(expected_exc.error) == typeof(exc)
+                expected_exc = expected_exc.error # deprecated
+            end
+            if exc isa typeof(expected_exc)
                 success = true
-                for fld in 1:nfields(extype)
-                    if !isequal(getfield(extype, fld), getfield(exc, fld))
+                for fld in 1:nfields(expected_exc)
+                    if !isequal(getfield(expected_exc, fld), getfield(exc, fld))
                         success = false
                         break
                     end
@@ -598,10 +604,10 @@ function do_test_throws(result::ExecutionResult, @nospecialize(orig_expr), @nosp
         if success
             testres = Pass(:test_throws, nothing, nothing, exc)
         else
-            testres = Fail(:test_throws_wrong, orig_expr, extype, exc, result.source)
+            testres = Fail(:test_throws_wrong, orig_expr, expected_exc, exc, result.source)
         end
     else
-        testres = Fail(:test_throws_nothing, orig_expr, extype, nothing, result.source)
+        testres = Fail(:test_throws_nothing, orig_expr, expected_exc, nothing, result.source)
     end
     record(get_testset(), testres)
 end
diff --git a/stdlib/Test/test/runtests.jl b/stdlib/Test/test/runtests.jl
@@ -915,3 +915,10 @@ end
     end
 end
 
+@testset "Soft deprecation of @test_throws LoadError expr" begin
+    # Undecorated LoadError can stand in for the wrapped error (ie, any Exception)
+    @test_throws LoadError throw(ErrorException("Real error"))
+    # Expected LoadError instances are unwrapped as necessary
+    @test_throws LoadError("file", 111, ErrorException("Real error")) throw(ErrorException("Real error"))
+    @test_throws LoadError("file", 111, ErrorException("Real error")) LoadError("file", 111, throw(ErrorException("Real error")))
+end
diff --git a/test/backtrace.jl b/test/backtrace.jl
@@ -199,7 +199,7 @@ let trace = try
 
             """, "a_filename")
     catch
-        stacktrace(Base.catch_stack()[end-1][2]) # Ignore LoadError
+        stacktrace(catch_backtrace())
     end
     @test trace[1].func == Symbol("top-level scope")
     @test trace[1].file == :a_filename
@@ -213,7 +213,7 @@ let trace = try
 
             """, "a_filename")
     catch
-        stacktrace(Base.catch_stack()[end-1][2]) # Ignore LoadError
+        stacktrace(catch_backtrace())
     end
     @test trace[1].func == Symbol("top-level scope")
     @test trace[1].file == :a_filename
diff --git a/test/core.jl b/test/core.jl
@@ -985,8 +985,7 @@ let didthrow =
             """)
         false
     catch ex
-        @test isa(ex, LoadError)
-        @test isa(ex.error, InitError)
+        @test ex isa InitError
         true
     end
     @test didthrow
@@ -1071,18 +1070,16 @@ let
     @test_throws BoundsError(z, 0) getfield(z, 0)
     @test_throws BoundsError(z, 3) getfield(z, 3)
 
-    strct = LoadError("yofile", 0, "bad")
-    @test nfields(strct) == 3 # sanity test
+    strct = InitError(:yomod, "bad")
+    @test nfields(strct) == 2 # sanity test
     @test_throws BoundsError(strct, 10) getfield(strct, 10)
-    @test_throws ErrorException("setfield! immutable struct of type LoadError cannot be changed") setfield!(strct, 0, "")
-    @test_throws ErrorException("setfield! immutable struct of type LoadError cannot be changed") setfield!(strct, 4, "")
-    @test_throws ErrorException("setfield! immutable struct of type LoadError cannot be changed") setfield!(strct, :line, 0)
-    @test strct.file == "yofile"
-    @test strct.line === 0
+    @test_throws ErrorException("setfield! immutable struct of type InitError cannot be changed") setfield!(strct, 0, "")
+    @test_throws ErrorException("setfield! immutable struct of type InitError cannot be changed") setfield!(strct, 4, "")
+    @test_throws ErrorException("setfield! immutable struct of type InitError cannot be changed") setfield!(strct, :mod, :yomod2)
+    @test strct.mod == :yomod
     @test strct.error == "bad"
-    @test getfield(strct, 1) == "yofile"
-    @test getfield(strct, 2) === 0
-    @test getfield(strct, 3) == "bad"
+    @test getfield(strct, 1) == :yomod
+    @test getfield(strct, 2) == "bad"
 
     mstrct = TestMutable("melm", 1, nothing)
     @test Base.setproperty!(mstrct, :line, 8.0) === 8
@@ -2034,8 +2031,8 @@ test5536(a::Union{Real, AbstractArray}) = "Non-splatting"
     include_string(@__MODULE__, "1 + #= #= blah =# =# 2") ==
     include_string(@__MODULE__, "1 + #= #= #= nested =# =# =# 2") ==
     include_string(@__MODULE__, "1 + #= \0 =# 2")
-@test_throws LoadError include_string(@__MODULE__, "#=")
-@test_throws LoadError include_string(@__MODULE__, "#= #= #= =# =# =")
+@test_throws ErrorException("syntax: incomplete: unterminated multi-line comment #= ... =#") include_string(@__MODULE__, "#=")
+@test_throws ErrorException("syntax: incomplete: unterminated multi-line comment #= ... =#") include_string(@__MODULE__, "#= #= #= =# =# =")
 
 # issue #6142
 import Base: +
@@ -3869,12 +3866,9 @@ end
 @test @m8846(a, 1) === (:a, 1)
 let nometh = try; @eval @m8846(a, b, c); false; catch ex; ex; end
     __source__ = LineNumberNode(@__LINE__() -  1, Symbol(@__FILE__))
-    nometh::LoadError
-    @test nometh.file === string(__source__.file)
-    @test nometh.line === __source__.line
-    e = nometh.error::MethodError
-    @test e.f === getfield(@__MODULE__, Symbol("@m8846"))
-    @test e.args === (__source__, @__MODULE__, :a, :b, :c)
+    @test nometh isa MethodError
+    @test nometh.f === getfield(@__MODULE__, Symbol("@m8846"))
+    @test nometh.args === (__source__, @__MODULE__, :a, :b, :c)
  end
 
 # a simple case of parametric dispatch with unions
@@ -5505,11 +5499,8 @@ f_isdefined_splat(x...) = @isdefined x
 @test f_isdefined_splat(1, 2, 3)
 let err = try; @macroexpand @isdefined :x; false; catch ex; ex; end,
     __source__ = LineNumberNode(@__LINE__() - 1, Symbol(@__FILE__))
-    @test err.file === string(__source__.file)
-    @test err.line === __source__.line
-    e = err.error::MethodError
-    @test e.f === getfield(@__MODULE__, Symbol("@isdefined"))
-    @test e.args === (__source__, @__MODULE__, :(:x))
+    @test err.f === getfield(@__MODULE__, Symbol("@isdefined"))
+    @test err.args === (__source__, @__MODULE__, :(:x))
 end
 f_isdefined_cl_1(y) = (local x; for i = 1:y; x = 2; end; () -> x; @isdefined x)
 f_isdefined_cl_2(y) = (local x; for i = 1:y; x = 2; end; () -> @isdefined x)
diff --git a/test/docs.jl b/test/docs.jl
@@ -722,18 +722,7 @@ end
 )
 
 # Issue #13905.
-let err = try; @macroexpand(@doc "" f() = @x); false; catch ex; ex; end
-    __source__ = LineNumberNode(@__LINE__() -  1, Symbol(@__FILE__))
-    err::LoadError
-    @test err.file === string(__source__.file)
-    @test err.line === __source__.line
-    err = err.error::LoadError
-    @test err.file === string(__source__.file)
-    @test err.line === __source__.line
-    err = err.error::UndefVarError
-    @test err.var == Symbol("@x")
- end
-
+@test_throws UndefVarError(Symbol("@x")) @macroexpand(@doc "" f() = @x)
 
 # Undocumented DataType Summaries.
 
@@ -1089,7 +1078,7 @@ macro mdoc22098 end
 @test docstrings_equal(@doc(:@mdoc22098), doc"an empty macro")
 
 # issue #24468
-let ex = try
+let trace = try
     include_string(@__MODULE__, """
 
     \"\"\"
@@ -1098,10 +1087,12 @@ let ex = try
     function hello(param::Vector{In64_nOt_DeFiNeD__})
     end
     """)
-catch e
-    e
+catch
+    stacktrace(catch_backtrace())
 end
-    @test ex.line == 2
+    @test trace[1].func == Symbol("top-level scope")
+    @test trace[1].file == :string
+    @test trace[1].line == 2
 end
 
 struct t_docs_abc end
diff --git a/test/errorshow.jl b/test/errorshow.jl
@@ -401,12 +401,6 @@ let
     @test (@macroexpand @fastmath +      ) == :(Base.FastMath.add_fast)
     @test (@macroexpand @fastmath min(1) ) == :(Base.FastMath.min_fast(1))
     let err = try; @macroexpand @doc "" f() = @x; catch ex; ex; end
-        file, line = @__FILE__, @__LINE__() - 1
-        err = err::LoadError
-        @test err.file == file && err.line == line
-        err = err.error::LoadError
-        @test err.file == file && err.line == line
-        err = err.error::UndefVarError
         @test err == UndefVarError(Symbol("@x"))
     end
     @test (@macroexpand @seven_dollar $bar) == 7
diff --git a/test/precompile.jl b/test/precompile.jl
diff --git a/test/simdloop.jl b/test/simdloop.jl
diff --git a/test/syntax.jl b/test/syntax.jl
