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
  • Loading branch information
@vtjnash
vtjnash committed Oct 22, 2018
1 parent a928ec5 commit 715e0eb
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 @@ -166,7 +166,6 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)
}
// add `eval` function
form = jl_call_scm_on_ast("module-default-defs", (jl_value_t*)ex, newm);
ptls->world_age = jl_world_counter;
jl_toplevel_eval_flex(newm, form, 0, 1);
form = NULL;
}
Expand All @@ -179,6 +178,7 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex)
ptls->world_age = jl_world_counter;
(void)jl_toplevel_eval_flex(newm, form, 1, 1);
}
newm->primary_world = jl_world_counter;
ptls->world_age = last_age;

#if 0
Expand Down Expand Up @@ -243,8 +243,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 @@ -255,7 +256,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 @@ -377,26 +381,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) JL_GLOBALLY_ROOTED
{
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 @@ -407,7 +406,8 @@ static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_RO
// 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_PROPAGATES_ROOT, jl_array_t *args, jl_sym_t **name, const char *keyword) JL_GLOBALLY_ROOTED
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 @@ -589,8 +589,9 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
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 @@ -601,8 +602,11 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
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 @@ -612,8 +616,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
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 @@ -625,7 +627,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
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 @@ -647,13 +648,10 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
}
}
}
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 @@ -665,7 +663,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
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 @@ -675,7 +672,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
}
}
}
ptls->world_age = last_age;
JL_GC_POP();
return jl_nothing;
}
Expand Down Expand Up @@ -711,23 +707,20 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
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 @@ -740,7 +733,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int

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 @@ -756,7 +749,6 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
// 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 715e0eb

Please sign in to comment.