From 8a6fd986d5b0fe6066a810c442a7a75b02011797 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 22 Oct 2018 12:48:16 -0400 Subject: [PATCH] fix eval world updates These were scattered about conservatively, and not always in the right places. Here we narrow them to apply more specifically, and remove several that should not be observable. fix #29761 (cherry picked from commit 715e0ebd2bdfeaebbc290606daaee4e25157f880, PR #29765) --- src/toplevel.c | 46 +++++++++++++++++++--------------------------- test/core.jl | 33 +++++++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/toplevel.c b/src/toplevel.c index a57ad3b16ddfb7..fb4efc45b18494 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -220,7 +220,6 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex) if (std_imports) { // add `eval` function defaultdefs = jl_call_scm_on_ast("module-default-defs", (jl_value_t*)ex, newm); - ptls->world_age = jl_world_counter; jl_toplevel_eval_flex(newm, defaultdefs, 0, 1); defaultdefs = NULL; } @@ -241,6 +240,7 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex) jl_rethrow(); } JL_GC_POP(); + newm->primary_world = jl_world_counter; ptls->world_age = last_age; ptls->current_module = last_module; ptls->current_task->current_module = task_last_m; @@ -287,8 +287,9 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex) return (jl_value_t*)newm; } -jl_value_t *jl_eval_dot_expr(jl_module_t *m, jl_value_t *x, jl_value_t *f, int fast) +static jl_value_t *jl_eval_dot_expr(jl_module_t *m, jl_value_t *x, jl_value_t *f, int fast) { + jl_ptls_t ptls = jl_get_ptls_states(); jl_value_t **args; JL_GC_PUSHARGS(args, 3); args[1] = jl_toplevel_eval_flex(m, x, fast, 0); @@ -299,7 +300,10 @@ jl_value_t *jl_eval_dot_expr(jl_module_t *m, jl_value_t *x, jl_value_t *f, int f } else { args[0] = jl_eval_global_var(jl_base_relative_to(m), jl_symbol("getproperty")); + size_t last_age = ptls->world_age; + ptls->world_age = jl_world_counter; args[0] = jl_apply(args, 3); + ptls->world_age = last_age; } JL_GC_POP(); return args[0]; @@ -421,26 +425,21 @@ static void body_attributes(jl_array_t *body, int *has_intrinsics, int *has_defs static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) { static jl_value_t *require_func = NULL; - static size_t require_world = 0; int build_mode = jl_generating_output(); jl_module_t *m = NULL; jl_ptls_t ptls = jl_get_ptls_states(); if (require_func == NULL && jl_base_module != NULL) { require_func = jl_get_global(jl_base_module, jl_symbol("require")); - if (build_mode) - require_world = ptls->world_age; } if (require_func != NULL) { size_t last_age = ptls->world_age; - if (build_mode) - ptls->world_age = require_world; + ptls->world_age = (build_mode ? jl_base_module->primary_world : jl_world_counter); jl_value_t *reqargs[3]; reqargs[0] = require_func; reqargs[1] = (jl_value_t*)mod; reqargs[2] = (jl_value_t*)var; m = (jl_module_t*)jl_apply(reqargs, 3); - if (build_mode) - ptls->world_age = last_age; + ptls->world_age = last_age; } if (m == NULL || !jl_is_module(m)) { jl_errorf("failed to load module %s", jl_symbol_name(var)); @@ -451,7 +450,8 @@ static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) // either: // - sets *name and returns the module to import *name from // - sets *name to NULL and returns a module to import -static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from, jl_array_t *args, jl_sym_t **name, const char *keyword) +static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PROPAGATES_ROOT, + jl_array_t *args, jl_sym_t **name, const char *keyword) JL_GLOBALLY_ROOTED { jl_sym_t *var = (jl_sym_t*)jl_array_ptr_ref(args, 0); size_t i = 1; @@ -630,8 +630,9 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e jl_value_t *lhs = jl_exprarg(ex, 0); jl_value_t *rhs = jl_exprarg(ex, 1); // only handle `a.b` syntax here, so qualified names can be eval'd in pure contexts - if (jl_is_quotenode(rhs) && jl_is_symbol(jl_fieldref(rhs,0))) + if (jl_is_quotenode(rhs) && jl_is_symbol(jl_fieldref(rhs, 0))) { return jl_eval_dot_expr(m, lhs, rhs, fast); + } } if (ptls->in_pure_callback) { @@ -642,8 +643,11 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e jl_code_info_t *thk = NULL; JL_GC_PUSH3(&li, &thk, &ex); + size_t last_age = ptls->world_age; if (!expanded && jl_needs_lowering(e)) { + ptls->world_age = jl_world_counter; ex = (jl_expr_t*)jl_expand(e, m); + ptls->world_age = last_age; } jl_sym_t *head = jl_is_expr(ex) ? ex->head : NULL; @@ -653,8 +657,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e return val; } else if (head == using_sym) { - size_t last_age = ptls->world_age; - ptls->world_age = jl_world_counter; jl_sym_t *name = NULL; jl_module_t *from = eval_import_from(m, ex, "using"); size_t i = 0; @@ -666,7 +668,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e jl_value_t *a = jl_exprarg(ex, i); if (jl_is_expr(a) && ((jl_expr_t*)a)->head == dot_sym) { name = NULL; - ptls->world_age = jl_world_counter; jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "using"); jl_module_t *u = import; if (name != NULL) @@ -688,13 +689,10 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e } } } - ptls->world_age = last_age; JL_GC_POP(); return jl_nothing; } else if (head == import_sym) { - size_t last_age = ptls->world_age; - ptls->world_age = jl_world_counter; jl_sym_t *name = NULL; jl_module_t *from = eval_import_from(m, ex, "import"); size_t i = 0; @@ -706,7 +704,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e jl_value_t *a = jl_exprarg(ex, i); if (jl_is_expr(a) && ((jl_expr_t*)a)->head == dot_sym) { name = NULL; - ptls->world_age = jl_world_counter; jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "import"); if (name == NULL) { import_module(m, import); @@ -716,7 +713,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e } } } - ptls->world_age = last_age; JL_GC_POP(); return jl_nothing; } @@ -752,23 +748,20 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e return jl_nothing; } else if (head == toplevel_sym) { - size_t last_age = ptls->world_age; jl_value_t *res = jl_nothing; int i; for (i = 0; i < jl_array_len(ex->args); i++) { - ptls->world_age = jl_world_counter; // eval each statement in the newest world age res = jl_toplevel_eval_flex(m, jl_array_ptr_ref(ex->args, i), fast, 0); } - ptls->world_age = last_age; JL_GC_POP(); return res; } else if (head == error_sym || head == jl_incomplete_sym) { if (jl_expr_nargs(ex) == 0) jl_errorf("malformed \"%s\" expression", jl_symbol_name(head)); - if (jl_is_string(jl_exprarg(ex,0))) - jl_errorf("syntax: %s", jl_string_data(jl_exprarg(ex,0))); - jl_throw(jl_exprarg(ex,0)); + if (jl_is_string(jl_exprarg(ex, 0))) + jl_errorf("syntax: %s", jl_string_data(jl_exprarg(ex, 0))); + jl_throw(jl_exprarg(ex, 0)); } else if (jl_is_symbol(ex)) { JL_GC_POP(); @@ -781,7 +774,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e int has_intrinsics = 0, has_defs = 0, has_loops = 0; assert(head == thunk_sym); - thk = (jl_code_info_t*)jl_exprarg(ex,0); + thk = (jl_code_info_t*)jl_exprarg(ex, 0); assert(jl_is_code_info(thk)); assert(jl_typeis(thk->code, jl_array_any_type)); body_attributes((jl_array_t*)thk->code, &has_intrinsics, &has_defs, &has_loops); @@ -797,7 +790,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e // worthwhile and also unsound (see #24316). // TODO: This is still not correct since an `eval` can happen elsewhere, but it // helps in common cases. - size_t last_age = ptls->world_age; size_t world = jl_world_counter; ptls->world_age = world; if (!has_defs) { diff --git a/test/core.jl b/test/core.jl index eb568fa7396f60..15a9bd7e607109 100644 --- a/test/core.jl +++ b/test/core.jl @@ -1080,13 +1080,13 @@ let @test getfield(strct, 3) == "bad" mstrct = TestMutable("melm", 1, nothing) - Base.setproperty!(mstrct, :line, 8.0) + @test Base.setproperty!(mstrct, :line, 8.0) === 8 @test mstrct.line === 8 @test_throws TypeError(:setfield!, "", Int, 8.0) setfield!(mstrct, :line, 8.0) @test_throws TypeError(:setfield!, "", Int, 8.0) setfield!(mstrct, 2, 8.0) - setfield!(mstrct, 3, "hi") + @test setfield!(mstrct, 3, "hi") == "hi" @test mstrct.error == "hi" - setfield!(mstrct, 1, "yo") + @test setfield!(mstrct, 1, "yo") == "yo" @test mstrct.file == "yo" @test_throws BoundsError(mstrct, 10) getfield(mstrct, 10) @test_throws BoundsError(mstrct, 0) setfield!(mstrct, 0, "") @@ -1098,7 +1098,7 @@ function Base.getproperty(mstrct::TestMutable, p::Symbol) return (p, getfield(mstrct, :error)) end function Base.setproperty!(mstrct::TestMutable, p::Symbol, v) - setfield!(mstrct, :error, (p, v)) + return setfield!(mstrct, :error, (p, v)) end let @@ -1117,6 +1117,20 @@ let @test mstrct.error === (:error, (:line, 8.0)) end +struct S29761 + x +end +function S29761_world(i) + x = S29761(i) + @eval function Base.getproperty(x::S29761, sym::Symbol) + return sym => getfield(x, sym) + end + # ensure world updates are handled correctly for simple x.y expressions: + return x.x, @eval($x.x), x.x +end +@test S29761_world(1) == (1, :x => 1, 1) + + # allow typevar in Union to match as long as the arguments contain # sufficient information # issue #814 @@ -2264,15 +2278,18 @@ a7652 = A7652(0) t_a7652 = A7652 f7652() = fieldtype(t_a7652, :a) <: Int @test f7652() == (fieldtype(A7652, :a) <: Int) == true + g7652() = fieldtype(DataType, :types) @test g7652() == fieldtype(DataType, :types) == Core.SimpleVector @test fieldtype(t_a7652, 1) == Int + h7652() = setfield!(a7652, 1, 2) -h7652() -@test a7652.a == 2 +@test h7652() === 2 +@test a7652.a === 2 + i7652() = Base.setproperty!(a7652, :a, 3.0) -i7652() -@test a7652.a == 3 +@test i7652() === 3 +@test a7652.a === 3 # issue #7679 @test map(f->f(), Any[ ()->i for i=1:3 ]) == Any[1,2,3]