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

Fix macro hygiene when calling the macro in the same module. #15850

Merged
merged 3 commits into from
Dec 8, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions base/docs/Docs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,9 @@ function docm(meta, ex, define = true)
# Don't try to redefine expressions. This is only needed for `Base` img gen since
# otherwise calling `loaddocs` would redefine all documented functions and types.
def = define ? x : nothing
if isa(x, GlobalRef) && (x::GlobalRef).mod == current_module()
x = (x::GlobalRef).name
end

# Keywords using the `@kw_str` macro in `base/docs/basedocs.jl`.
#
Expand Down
4 changes: 2 additions & 2 deletions base/linalg/factorization.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ transpose(F::Factorization) = error("transpose not implemented for $(typeof(F))"
ctranspose(F::Factorization) = error("ctranspose not implemented for $(typeof(F))")

macro assertposdef(A, info)
:(($info)==0 ? $A : throw(PosDefException($info)))
:($(esc(info)) == 0 ? $(esc(A)) : throw(PosDefException($(esc(info)))))
end

macro assertnonsingular(A, info)
:(($info)==0 ? $A : throw(SingularException($info)))
:($(esc(info)) == 0 ? $(esc(A)) : throw(SingularException($(esc(info)))))
end

function logdet(F::Factorization)
Expand Down
2 changes: 1 addition & 1 deletion base/sparse/cholmod_h.jl
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,5 @@ type CHOLMODException <: Exception
end

macro isok(A)
:($A == TRUE || throw(CHOLMODException("")))
:($(esc(A)) == TRUE || throw(CHOLMODException("")))
end
2 changes: 1 addition & 1 deletion base/sparse/umfpack.jl
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function umferror(status::Integer)
end

macro isok(A)
:(umferror($A))
:(umferror($(esc(A))))
end

# check the size of SuiteSparse_long
Expand Down
46 changes: 32 additions & 14 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,18 @@ value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t na
jl_ptls_t ptls = jl_get_ptls_states();
// tells whether a var is defined in and *by* the current module
argcount(fl_ctx, "defined-julia-global", nargs, 1);
(void)tosymbol(fl_ctx, args[0], "defined-julia-global");
if (ptls->current_module == NULL)
jl_module_t *mod = ptls->current_module;
jl_sym_t *sym = (jl_sym_t*)scm_to_julia(fl_ctx, args[0], 0);
if (jl_is_globalref(sym)) {
mod = jl_globalref_mod(sym);
sym = jl_globalref_name(sym);
}
if (!jl_is_symbol(sym))
type_error(fl_ctx, "defined-julia-global", "symbol", args[0]);
if (!mod)
return fl_ctx->F;
jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0]));
jl_binding_t *b =
(jl_binding_t*)ptrhash_get(&ptls->current_module->bindings, var);
return (b != HT_NOTFOUND && b->owner==ptls->current_module) ? fl_ctx->T : fl_ctx->F;
jl_binding_t *b = (jl_binding_t*)ptrhash_get(&mod->bindings, sym);
return (b != HT_NOTFOUND && b->owner == mod) ? fl_ctx->T : fl_ctx->F;
}

value_t fl_current_julia_module(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
Expand Down Expand Up @@ -177,14 +182,14 @@ value_t fl_invoke_julia_macro(fl_context_t *fl_ctx, value_t *args, uint32_t narg
fl_gc_handle(fl_ctx, &scm);
value_t scmresult;
jl_module_t *defmod = mfunc->def->module;
if (defmod == NULL || defmod == ptls->current_module) {
scmresult = fl_cons(fl_ctx, scm, fl_ctx->F);
}
else {
value_t opaque = cvalue(fl_ctx, jl_ast_ctx(fl_ctx)->jvtype, sizeof(void*));
*(jl_value_t**)cv_data((cvalue_t*)ptr(opaque)) = (jl_value_t*)defmod;
scmresult = fl_cons(fl_ctx, scm, opaque);
}
/* if (defmod == NULL || defmod == ptls->current_module) { */
/* scmresult = fl_cons(fl_ctx, scm, fl_ctx->F); */
/* } */
/* else { */
value_t opaque = cvalue(fl_ctx, jl_ast_ctx(fl_ctx)->jvtype, sizeof(void*));
*(jl_value_t**)cv_data((cvalue_t*)ptr(opaque)) = (jl_value_t*)defmod;
scmresult = fl_cons(fl_ctx, scm, opaque);
/* } */
fl_free_gc_handles(fl_ctx, 1);

JL_GC_POP();
Expand Down Expand Up @@ -610,6 +615,19 @@ static value_t julia_to_scm_(fl_context_t *fl_ctx, jl_value_t *v)
return julia_to_list2(fl_ctx, (jl_value_t*)inert_sym, jl_fieldref(v,0));
if (jl_typeis(v, jl_newvarnode_type))
return julia_to_list2(fl_ctx, (jl_value_t*)newvar_sym, jl_fieldref(v,0));
if (jl_typeis(v, jl_globalref_type)) {
jl_module_t *m = jl_globalref_mod(v);
jl_sym_t *sym = jl_globalref_name(v);
if (m == jl_core_module)
return julia_to_list2(fl_ctx, (jl_value_t*)core_sym,
(jl_value_t*)sym);
value_t args = julia_to_list2(fl_ctx, (jl_value_t*)m, (jl_value_t*)sym);
fl_gc_handle(fl_ctx, &args);
value_t hd = julia_to_scm_(fl_ctx, (jl_value_t*)globalref_sym);
value_t scmv = fl_cons(fl_ctx, hd, args);
fl_free_gc_handles(fl_ctx, 1);
return scmv;
}
if (jl_is_long(v) && fits_fixnum(jl_unbox_long(v)))
return fixnum(jl_unbox_long(v));
if (jl_is_ssavalue(v))
Expand Down
20 changes: 16 additions & 4 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3497,13 +3497,20 @@ static jl_cgval_t emit_expr(jl_value_t *expr, jl_codectx_t *ctx)

Value *bp = NULL, *name, *bp_owner = V_null;
jl_binding_t *bnd = NULL;
if (jl_is_symbol(mn)) {
bool issym = jl_is_symbol(mn);
bool isglobalref = !issym && jl_is_globalref(mn);
if (issym || isglobalref) {
jl_module_t *mod = ctx->module;
if (isglobalref) {
mod = jl_globalref_mod(mn);
mn = (jl_value_t*)jl_globalref_name(mn);
}
if (jl_symbol_name((jl_sym_t*)mn)[0] == '@')
jl_errorf("macro definition not allowed inside a local scope");
name = literal_pointer_val(mn);
bnd = jl_get_binding_for_method_def(ctx->module, (jl_sym_t*)mn);
bnd = jl_get_binding_for_method_def(mod, (jl_sym_t*)mn);
bp = julia_binding_gv(bnd);
bp_owner = literal_pointer_val((jl_value_t*)ctx->module);
bp_owner = literal_pointer_val((jl_value_t*)mod);
}
else if (jl_is_slot(mn)) {
int sl = jl_slot_number(mn)-1;
Expand All @@ -3527,10 +3534,15 @@ static jl_cgval_t emit_expr(jl_value_t *expr, jl_codectx_t *ctx)
}
else if (head == const_sym) {
jl_sym_t *sym = (jl_sym_t*)args[0];
jl_module_t *mod = ctx->module;
if (jl_is_globalref(sym)) {
mod = jl_globalref_mod(sym);
sym = jl_globalref_name(sym);
}
if (jl_is_symbol(sym)) {
JL_FEAT_REQUIRE(ctx, runtime);
jl_binding_t *bnd = NULL;
(void)global_binding_pointer(ctx->module, sym, &bnd, true, ctx); assert(bnd);
(void)global_binding_pointer(mod, sym, &bnd, true, ctx); assert(bnd);
builder.CreateCall(prepare_call(jldeclareconst_func),
literal_pointer_val(bnd));
}
Expand Down
41 changes: 34 additions & 7 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
}
else if (ex->head == method_sym) {
jl_sym_t *fname = (jl_sym_t*)args[0];
if (jl_is_globalref(fname)) {
modu = jl_globalref_mod(fname);
fname = jl_globalref_name(fname);
}
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname));

if (jl_is_symbol(fname)) {
Expand All @@ -271,18 +275,29 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
return jl_nothing;
}
else if (ex->head == const_sym) {
jl_value_t *sym = args[0];
jl_sym_t *sym = (jl_sym_t*)args[0];
if (jl_is_globalref(sym)) {
modu = jl_globalref_mod(sym);
sym = jl_globalref_name(sym);
}
assert(jl_is_symbol(sym));
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)sym);
jl_binding_t *b = jl_get_binding_wr(modu, sym);
jl_declare_constant(b);
return (jl_value_t*)jl_nothing;
}
else if (ex->head == global_sym) {
// create uninitialized mutable binding for "global x" decl
// TODO: handle type decls
for (size_t i=0; i < jl_array_len(ex->args); i++) {
assert(jl_is_symbol(args[i]));
jl_get_binding_wr(modu, (jl_sym_t*)args[i]);
size_t i, l = jl_array_len(ex->args);
for (i = 0; i < l; i++) {
jl_sym_t *gsym = (jl_sym_t*)args[i];
jl_module_t *gmodu = modu;
if (jl_is_globalref(gsym)) {
gmodu = jl_globalref_mod(gsym);
gsym = jl_globalref_name(gsym);
}
assert(jl_is_symbol(gsym));
jl_get_binding_wr(gmodu, gsym);
}
return (jl_value_t*)jl_nothing;
}
Expand All @@ -296,6 +311,10 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
jl_datatype_t *dt = NULL;
JL_GC_PUSH4(&para, &super, &temp, &dt);
assert(jl_is_svec(para));
if (jl_is_globalref(name)) {
modu = jl_globalref_mod(name);
name = (jl_value_t*)jl_globalref_name(name);
}
assert(jl_is_symbol(name));
dt = jl_new_abstracttype(name, NULL, (jl_svec_t*)para);
jl_binding_t *b = jl_get_binding_wr(modu, (jl_sym_t*)name);
Expand Down Expand Up @@ -328,6 +347,10 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
jl_value_t *super = NULL, *para = NULL, *vnb = NULL, *temp = NULL;
jl_datatype_t *dt = NULL;
JL_GC_PUSH4(&para, &super, &temp, &dt);
if (jl_is_globalref(name)) {
modu = jl_globalref_mod(name);
name = (jl_value_t*)jl_globalref_name(name);
}
assert(jl_is_symbol(name));
para = eval(args[1], s);
assert(jl_is_svec(para));
Expand Down Expand Up @@ -367,13 +390,17 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
if (inside_typedef)
jl_error("cannot eval a new data type definition while defining another type");
jl_value_t *name = args[0];
assert(jl_is_symbol(name));
jl_value_t *para = eval(args[1], s);
assert(jl_is_svec(para));
jl_value_t *temp = NULL;
jl_value_t *super = NULL;
jl_datatype_t *dt = NULL;
JL_GC_PUSH4(&para, &super, &temp, &dt);
if (jl_is_globalref(name)) {
modu = jl_globalref_mod(name);
name = (jl_value_t*)jl_globalref_name(name);
}
assert(jl_is_symbol(name));
assert(jl_is_svec(para));
temp = eval(args[2], s); // field names
#ifndef NDEBUG
size_t i, l = jl_svec_len(para);
Expand Down
8 changes: 8 additions & 0 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1800,6 +1800,14 @@
(error (string "invalid assignment location \"" (deparse lhs) "\"")))
(else
(case (car lhs)
((globalref)
;; M.b =
(let* ((rhs (caddr e))
(rr (if (or (symbol-like? rhs) (atom? rhs)) rhs (make-ssavalue))))
`(block
,.(if (eq? rr rhs) '() `((= ,rr ,(expand-forms rhs))))
(= ,lhs ,rr)
(unnecessary ,rr))))
((|.|)
;; a.b =
(let* ((a (cadr lhs))
Expand Down
2 changes: 1 addition & 1 deletion src/macroexpand.scm
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@
(error (cadr form)))
(let ((form (car form))
(m (cdr form)))
;; m is the macro's def module, or #f if def env === use env
;; m is the macro's def module
(rename-symbolic-labels
(julia-expand-macros
(resolve-expansion-vars form m))))))
Expand Down
29 changes: 28 additions & 1 deletion test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3490,7 +3490,7 @@ end

# issue 13855
macro m13855()
Expr(:localize, :(() -> x))
Expr(:localize, :(() -> $(esc(:x))))
end
@noinline function foo13855(x)
@m13855()
Expand Down Expand Up @@ -4794,3 +4794,30 @@ end
@test let_Box4()() == 44
@test let_Box5()() == 46
@test let_noBox()() == 21

module TestModuleAssignment
using Base.Test
@eval $(GlobalRef(TestModuleAssignment, :x)) = 1
@test x == 1
@eval $(GlobalRef(TestModuleAssignment, :x)) = 2
@test x == 2
end

# issue #14893
module M14893
x = 14893
macro m14893()
:x
end
function f14893()
x = 1
@m14893
end
end
function f14893()
x = 2
M14893.@m14893
end

@test f14893() == 14893
@test M14893.f14893() == 14893