Skip to content

Commit

Permalink
fix eval world updates
Browse files Browse the repository at this point in the history 
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 715e0eb, PR #29765)
  • Loading branch information
@vtjnash @KristofferC
vtjnash authored and KristofferC committed Feb 20, 2020
1 parent 8a50882 commit 8a6fd98
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 35 deletions.
46 changes: 19 additions & 27 deletions src/toplevel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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];
Expand Down Expand Up @@ -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));
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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) {
Expand Down
33 changes: 25 additions & 8 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit 8a6fd98

Please sign in to comment.