Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebase no load error on macroexpand #38379

Merged
merged 8 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ Language changes
behaviour violated the rule that `isequal(x, y)` implies `hash(x) == hash(y)`.
* `⌿` (U+233F) and `¦` (U+00A6) are now infix operators with times-like and plus-like precedence,
respectively. Previously they were parsed as identifier characters ([#37973]).
* `@macroexpand` and `@macroexpand1` no longer wrap errors with `LoadError`. To reduce breakage,
`@test_throws` has been modified so that affected tests will still pass ([#38379]].

Compiler/Runtime improvements
-----------------------------
Expand Down
3 changes: 3 additions & 0 deletions base/docs/basedocs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2240,6 +2240,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.7"
LoadErrors are no longer emitted by `@macroexpand` and `@macroexpand1` as of Julia 1.7.
"""
LoadError

Expand Down
30 changes: 15 additions & 15 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ struct macroctx_stack {

static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mod);
static value_t julia_to_scm(fl_context_t *fl_ctx, jl_value_t *v);
static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel, size_t world);
static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel, size_t world, int throw_load_error);

static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
Expand Down Expand Up @@ -968,7 +968,7 @@ int jl_has_meta(jl_array_t *body, jl_sym_t *sym) JL_NOTSAFEPOINT
return 0;
}

static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule, jl_module_t **ctx, size_t world)
static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule, jl_module_t **ctx, size_t world, int throw_load_error)
{
jl_ptls_t ptls = jl_get_ptls_states();
JL_TIMING(MACRO_INVOCATION);
Expand Down Expand Up @@ -1003,7 +1003,7 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
result = jl_invoke(margs[0], &margs[1], nargs - 1, mfunc);
}
JL_CATCH {
if (jl_loaderror_type == NULL) {
if ((jl_loaderror_type == NULL) || !throw_load_error) {
jl_rethrow();
}
else {
Expand All @@ -1023,7 +1023,7 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
return result;
}

static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel, size_t world)
static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel, size_t world, int throw_load_error)
{
if (!expr || !jl_is_expr(expr))
return expr;
Expand All @@ -1037,7 +1037,7 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
if (e->head == quote_sym && jl_expr_nargs(e) == 1) {
expr = jl_call_scm_on_ast("julia-bq-macro", jl_exprarg(e, 0), inmodule);
JL_GC_PUSH1(&expr);
expr = jl_expand_macros(expr, inmodule, macroctx, onelevel, world);
expr = jl_expand_macros(expr, inmodule, macroctx, onelevel, world, throw_load_error);
JL_GC_POP();
return expr;
}
Expand All @@ -1047,7 +1047,7 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
JL_TYPECHK(hygienic-scope, module, (jl_value_t*)newctx.m);
newctx.parent = macroctx;
jl_value_t *a = jl_exprarg(e, 0);
jl_value_t *a2 = jl_expand_macros(a, inmodule, &newctx, onelevel, world);
jl_value_t *a2 = jl_expand_macros(a, inmodule, &newctx, onelevel, world, throw_load_error);
if (a != a2)
jl_array_ptr_set(e->args, 0, a2);
return expr;
Expand All @@ -1056,7 +1056,7 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
struct macroctx_stack newctx;
newctx.m = macroctx ? macroctx->m : inmodule;
newctx.parent = macroctx;
jl_value_t *result = jl_invoke_julia_macro(e->args, inmodule, &newctx.m, world);
jl_value_t *result = jl_invoke_julia_macro(e->args, inmodule, &newctx.m, world, throw_load_error);
jl_value_t *wrap = NULL;
JL_GC_PUSH3(&result, &wrap, &newctx.m);
// copy and wrap the result in `(hygienic-scope ,result ,newctx)
Expand All @@ -1066,7 +1066,7 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
wrap = (jl_value_t*)jl_exprn(hygienicscope_sym, 2);
result = jl_copy_ast(result);
if (!onelevel)
result = jl_expand_macros(result, inmodule, wrap ? &newctx : macroctx, onelevel, world);
result = jl_expand_macros(result, inmodule, wrap ? &newctx : macroctx, onelevel, world, throw_load_error);
if (wrap) {
jl_exprargset(wrap, 0, result);
jl_exprargset(wrap, 1, newctx.m);
Expand All @@ -1088,7 +1088,7 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
for (j = 2; j < nm; j++) {
jl_exprargset(mc2, j+1, jl_exprarg(mc, j));
}
jl_value_t *ret = jl_expand_macros((jl_value_t*)mc2, inmodule, macroctx, onelevel, world);
jl_value_t *ret = jl_expand_macros((jl_value_t*)mc2, inmodule, macroctx, onelevel, world, throw_load_error);
JL_GC_POP();
return ret;
}
Expand All @@ -1099,7 +1099,7 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
size_t i;
for (i = 0; i < jl_array_len(e->args); i++) {
jl_value_t *a = jl_array_ptr_ref(e->args, i);
jl_value_t *a2 = jl_expand_macros(a, inmodule, macroctx, onelevel, world);
jl_value_t *a2 = jl_expand_macros(a, inmodule, macroctx, onelevel, world, throw_load_error);
if (a != a2)
jl_array_ptr_set(e->args, i, a2);
}
Expand All @@ -1111,7 +1111,7 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand(jl_value_t *expr, jl_module_t *inmodule)
JL_TIMING(LOWERING);
JL_GC_PUSH1(&expr);
expr = jl_copy_ast(expr);
expr = jl_expand_macros(expr, inmodule, NULL, 0, jl_world_counter);
expr = jl_expand_macros(expr, inmodule, NULL, 0, jl_world_counter, 0);
expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
JL_GC_POP();
return expr;
Expand All @@ -1122,7 +1122,7 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand1(jl_value_t *expr, jl_module_t *inmodule
JL_TIMING(LOWERING);
JL_GC_PUSH1(&expr);
expr = jl_copy_ast(expr);
expr = jl_expand_macros(expr, inmodule, NULL, 1, jl_world_counter);
expr = jl_expand_macros(expr, inmodule, NULL, 1, jl_world_counter, 0);
expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
JL_GC_POP();
return expr;
Expand All @@ -1148,7 +1148,7 @@ JL_DLLEXPORT jl_value_t *jl_expand_in_world(jl_value_t *expr, jl_module_t *inmod
JL_TIMING(LOWERING);
JL_GC_PUSH1(&expr);
expr = jl_copy_ast(expr);
expr = jl_expand_macros(expr, inmodule, NULL, 0, world);
expr = jl_expand_macros(expr, inmodule, NULL, 0, world, 1);
expr = jl_call_scm_on_ast_and_loc("jl-expand-to-thunk", expr, inmodule, file, line);
JL_GC_POP();
return expr;
Expand All @@ -1161,7 +1161,7 @@ JL_DLLEXPORT jl_value_t *jl_expand_with_loc_warn(jl_value_t *expr, jl_module_t *
JL_TIMING(LOWERING);
JL_GC_PUSH1(&expr);
expr = jl_copy_ast(expr);
expr = jl_expand_macros(expr, inmodule, NULL, 0, ~(size_t)0);
expr = jl_expand_macros(expr, inmodule, NULL, 0, ~(size_t)0, 1);
jl_ast_context_t *ctx = jl_ast_ctx_enter();
fl_context_t *fl_ctx = &ctx->fl;
JL_AST_PRESERVE_PUSH(ctx, old_roots, inmodule);
Expand All @@ -1182,7 +1182,7 @@ JL_DLLEXPORT jl_value_t *jl_expand_stmt_with_loc(jl_value_t *expr, jl_module_t *
JL_TIMING(LOWERING);
JL_GC_PUSH1(&expr);
expr = jl_copy_ast(expr);
expr = jl_expand_macros(expr, inmodule, NULL, 0, ~(size_t)0);
expr = jl_expand_macros(expr, inmodule, NULL, 0, ~(size_t)0, 1);
expr = jl_call_scm_on_ast_and_loc("jl-expand-to-thunk-stmt", expr, inmodule, file, line);
JL_GC_POP();
return expr;
Expand Down
14 changes: 13 additions & 1 deletion stdlib/Test/src/Test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -600,16 +600,28 @@ macro test_throws(extype, ex)
:(do_test_throws($result, $orig_ex, $(esc(extype))))
end

const MACROEXPAND_LIKE = Symbol.(("@macroexpand", "@macroexpand1"))

# 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, orig_expr, extype)
if isa(result, Threw)
# Check that the right type of exception was thrown
success = false
exc = result.exception
# NB: Throwing LoadError from macroexpands is deprecated, but in order to limit
# the breakage in package tests we add extra logic here.
from_macroexpand =
orig_expr isa Expr &&
orig_expr.head === :macrocall &&
orig_expr.args[1] in MACROEXPAND_LIKE
if isa(extype, Type)
success = isa(exc, extype)
success = isa(exc, extype) ||
(from_macroexpand && extype == LoadError && exc isa Exception) # deprecated
else
if from_macroexpand && extype isa LoadError && !(exc isa LoadError) && typeof(extype.error) == typeof(exc)
extype = extype.error # deprecated
end
if isa(exc, typeof(extype))
success = true
for fld in 1:nfields(extype)
Expand Down
15 changes: 15 additions & 0 deletions stdlib/Test/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1039,3 +1039,18 @@ end
@test occursin(expected, result)
end
end

macro test_macro_throw_1()
throw(ErrorException("Real error"))
end
macro test_macro_throw_2()
LoadError("file", 111, throw(ErrorException("Real error")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks weird — it throws ErrorException so that LoadError is doing nothing. I see it's carried over from 3ee9d8f, but it was probably a mistake there too.

IIRC the intention here was to actually throw LoadError explicitly, as the occasional non-core library does that to mimic Base.

end

@testset "Soft deprecation of @test_throws LoadError @macroexpand" begin
# Undecorated LoadError can stand in for the wrapped error (ie, any Exception)
@test_throws LoadError @macroexpand @test_macro_throw_1
# Expected LoadError instances are unwrapped as necessary
@test_throws LoadError("file", 111, ErrorException("Real error")) @macroexpand @test_macro_throw_1
@test_throws LoadError("file", 111, ErrorException("Real error")) @macroexpand @test_macro_throw_2
end
6 changes: 2 additions & 4 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5647,11 +5647,9 @@ f_isdefined_unionvar(y, t) = (t > 0 && (x = (t == 1 ? 1 : y)); @isdefined x)
@test !f_isdefined_unionvar(1, 0)
f_isdefined_splat(x...) = @isdefined x
@test f_isdefined_splat(1, 2, 3)
let err = try; @macroexpand @isdefined :x; false; catch ex; ex; end,
let e = 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
e::MethodError
@test e.f === getfield(@__MODULE__, Symbol("@isdefined"))
@test e.args === (__source__, @__MODULE__, :(:x))
end
Expand Down
9 changes: 1 addition & 8 deletions test/docs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -773,14 +773,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
err::UndefVarError
@test err.var == Symbol("@x")
end

Expand Down
6 changes: 0 additions & 6 deletions test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -473,12 +473,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
Expand Down
13 changes: 0 additions & 13 deletions test/simdloop.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,6 @@ import Base.SimdLoop.SimdError

# Test that @simd rejects inner loop body with invalid control flow statements
# issue #8613
macro test_throws(ty, ex)
return quote
Test.@test_throws $(esc(ty)) try
$(esc(ex))
catch err
@test err isa LoadError
@test err.file === $(string(__source__.file))
@test err.line === $(__source__.line + 1)
rethrow(err.error)
end
end
end

@test_throws SimdError("break is not allowed inside a @simd loop body") @macroexpand begin
@simd for x = 1:10
x == 1 && break
Expand Down
7 changes: 2 additions & 5 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -615,15 +615,12 @@ end
@test A15838.@f() === nothing
@test A15838.@f(1) === :b
let ex = :(A15838.@f(1, 2)), __source__ = LineNumberNode(@__LINE__, Symbol(@__FILE__))
nometh = try
e = try
macroexpand(@__MODULE__, ex)
false
catch ex
ex
end::LoadError
@test nometh.file === string(__source__.file)
@test nometh.line === __source__.line
e = nometh.error::MethodError
end::MethodError
@test e.f === getfield(A15838, Symbol("@f"))
@test e.args === (__source__, @__MODULE__, 1, 2)
end
Expand Down
3 changes: 1 addition & 2 deletions test/threads_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,7 @@ end
try
@macroexpand @threads(for i = 1:10, j = 1:10; end)
catch ex
@test ex isa LoadError
@test ex.error isa ArgumentError
@test ex isa ArgumentError
end

@testset "@spawn interpolation" begin
Expand Down