Skip to content

Commit

Permalink
replace getfield calls with GlobalRef in more cases. ref #10403
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Jul 21, 2015
1 parent 0c9d74b commit 2502f27
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 19 deletions.
1 change: 1 addition & 0 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ function _iisconst(s::Symbol)
end
_iisconst(s::SymbolNode) = _iisconst(s.name)
_iisconst(s::TopNode) = isconst(_topmod(), s.name)
_iisconst(s::GlobalRef) = isconst(s.mod, s.name)
_iisconst(x::Expr) = false
_iisconst(x::ANY) = true

Expand Down
33 changes: 27 additions & 6 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -985,25 +985,31 @@ int jl_in_vinfo_array(jl_array_t *a, jl_sym_t *v)
return 0;
}

static int in_sym_array(jl_array_t *a, jl_value_t *v)
int jl_in_sym_array(jl_array_t *a, jl_sym_t *v)
{
size_t i, l=jl_array_len(a);
for(i=0; i<l; i++) {
if (jl_cellref(a,i) == v)
if (jl_cellref(a,i) == (jl_value_t*)v)
return 1;
}
return 0;
}

int jl_local_in_ast(jl_expr_t *ast, jl_sym_t *sym)
{
return jl_in_vinfo_array(jl_lam_vinfo(ast), sym) ||
jl_in_vinfo_array(jl_lam_capt(ast), sym) ||
jl_in_sym_array(jl_lam_staticparams(ast), sym);
}

JL_CALLABLE(jl_f_get_field);

static jl_value_t *resolve_globals(jl_value_t *expr, jl_lambda_info_t *lam)
{
if (jl_is_symbol(expr)) {
if (lam->module == NULL)
return expr;
int is_local = jl_in_vinfo_array(jl_lam_vinfo((jl_expr_t*)lam->ast), (jl_sym_t*)expr) ||
jl_in_vinfo_array(jl_lam_capt((jl_expr_t*)lam->ast), (jl_sym_t*)expr) ||
in_sym_array(jl_lam_staticparams((jl_expr_t*)lam->ast), expr);
if (!is_local)
if (!jl_local_in_ast((jl_expr_t*)lam->ast, (jl_sym_t*)expr))
return jl_module_globalref(lam->module, (jl_sym_t*)expr);
}
else if (jl_is_lambda_info(expr)) {
Expand All @@ -1020,6 +1026,21 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_lambda_info_t *lam)
e->head == line_sym || e->head == meta_sym) {
}
else {
if (e->head == call_sym && jl_expr_nargs(e) == 3 && jl_is_quotenode(jl_exprarg(e,2)) &&
lam->module != NULL) {
// replace getfield(module_expr, :sym) with GlobalRef
jl_value_t *s = jl_fieldref(jl_exprarg(e,2),0);
if (jl_is_symbol(s)) {
jl_value_t *f = jl_static_eval(jl_exprarg(e,0), NULL, lam->module,
NULL, (jl_expr_t*)lam->ast, 0, 0);
if (f && jl_is_func(f) && ((jl_function_t*)f)->fptr == &jl_f_get_field) {
jl_value_t *m = jl_static_eval(jl_exprarg(e,1), NULL, lam->module,
NULL, (jl_expr_t*)lam->ast, 0, 0);
if (m && jl_is_module(m))
return jl_module_globalref((jl_module_t*)m, (jl_sym_t*)s);
}
}
}
size_t i = 0;
if (e->head == method_sym || e->head == abstracttype_sym || e->head == compositetype_sym ||
e->head == bitstype_sym || e->head == macro_sym || e->head == module_sym)
Expand Down
12 changes: 1 addition & 11 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1256,16 +1256,6 @@ extern "C" void jl_write_malloc_log(void)

// --- constant determination ---

static bool in_vinfo(jl_sym_t *s, jl_array_t *vi)
{
size_t i, l = jl_array_len(vi);
for(i=0; i < l; i++) {
if (s == (jl_sym_t*)jl_cellref(jl_cellref(vi, i), 0))
return true;
}
return false;
}

// try to statically evaluate, NULL if not possible
extern "C"
jl_value_t *jl_static_eval(jl_value_t *ex, void *ctx_, jl_module_t *mod,
Expand All @@ -1281,7 +1271,7 @@ jl_value_t *jl_static_eval(jl_value_t *ex, void *ctx_, jl_module_t *mod,
isglob = is_global(sym, ctx);
}
else if (ast) {
isglob = !in_vinfo(sym, jl_lam_vinfo(ast)) && !in_vinfo(sym, jl_lam_capt(ast));
isglob = !jl_local_in_ast(ast, sym);
}
if (isglob) {
size_t i;
Expand Down
2 changes: 0 additions & 2 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1454,8 +1454,6 @@ jl_function_t *jl_get_specialization(jl_function_t *f, jl_tupletype_t *types)

void jl_trampoline_compile_function(jl_function_t *f, int always_infer, jl_tupletype_t *sig);

int jl_in_vinfo_array(jl_array_t *a, jl_sym_t *v);

static void parameters_to_closureenv(jl_value_t *ast, jl_svec_t *tvars)
{
jl_array_t *closed = jl_lam_capt((jl_expr_t*)ast);
Expand Down
2 changes: 2 additions & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,8 @@ jl_array_t *jl_lam_staticparams(jl_expr_t *l);
jl_sym_t *jl_lam_argname(jl_lambda_info_t *li, int i);
int jl_lam_vars_captured(jl_expr_t *ast);
jl_expr_t *jl_lam_body(jl_expr_t *l);
int jl_in_vinfo_array(jl_array_t *a, jl_sym_t *v);
int jl_local_in_ast(jl_expr_t *ast, jl_sym_t *sym);
DLLEXPORT jl_value_t *jl_ast_rettype(jl_lambda_info_t *li, jl_value_t *ast);
jl_sym_t *jl_decl_var(jl_value_t *ex);
DLLEXPORT int jl_is_rest_arg(jl_value_t *ex);
Expand Down

5 comments on commit 2502f27

@staticfloat
Copy link
Member

Choose a reason for hiding this comment

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

Is it intended that this change disables the ability to have functions that shadow Base functions in a module? E.g. if Homebrew.jl were to define an info() function that shadows Base.info()? See JuliaPackaging/Homebrew.jl#90 for more.

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

No, that was caused by an earlier change (#4345). However I'm considering allowing shadowing using Base (see #12183).

@kmsquire
Copy link
Member

Choose a reason for hiding this comment

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

git bisect showed this commit to be the cause.

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh dear. That was not the intention.

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be fixed now.

Please sign in to comment.