From 7a0e916fb48d2489c91574494e7a92e495915455 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Thu, 14 Apr 2016 23:21:06 -0400 Subject: [PATCH] Fix macro hygiene when calling the macro in the same module. * Allow assigning to `GlobalRef`. * Allow defining/extending function with `GlobalRef`. * Convert `GlobalRef` back to flisp. Closes #14893 --- base/docs/Docs.jl | 3 +++ base/linalg/factorization.jl | 4 ++-- base/sparse/cholmod_h.jl | 2 +- base/sparse/umfpack.jl | 2 +- src/ast.c | 29 +++++++++++++++++++++-------- src/builtins.c | 13 +++++++++++-- src/interpreter.c | 20 ++++++++++++++++++-- src/julia-syntax.scm | 10 ++++++++++ src/macroexpand.scm | 2 +- test/core.jl | 29 ++++++++++++++++++++++++++++- 10 files changed, 96 insertions(+), 18 deletions(-) diff --git a/base/docs/Docs.jl b/base/docs/Docs.jl index 76b14015002087..2ddcedfc0b07a8 100644 --- a/base/docs/Docs.jl +++ b/base/docs/Docs.jl @@ -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`. # diff --git a/base/linalg/factorization.jl b/base/linalg/factorization.jl index 045d88212a755a..898964d9da5916 100644 --- a/base/linalg/factorization.jl +++ b/base/linalg/factorization.jl @@ -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) diff --git a/base/sparse/cholmod_h.jl b/base/sparse/cholmod_h.jl index 5f51ec50463ea2..922b3b772b7cc3 100644 --- a/base/sparse/cholmod_h.jl +++ b/base/sparse/cholmod_h.jl @@ -75,5 +75,5 @@ type CHOLMODException <: Exception end macro isok(A) - :($A == TRUE || throw(CHOLMODException(""))) + :($(esc(A)) == TRUE || throw(CHOLMODException(""))) end diff --git a/base/sparse/umfpack.jl b/base/sparse/umfpack.jl index 673617a4fc8e7c..905c3fdf046c85 100644 --- a/base/sparse/umfpack.jl +++ b/base/sparse/umfpack.jl @@ -54,7 +54,7 @@ function umferror(status::Integer) end macro isok(A) - :(umferror($A)) + :(umferror($(esc(A)))) end # check the size of SuiteSparse_long diff --git a/src/ast.c b/src/ast.c index 8bcf0b31a48b46..0e202d1bdb740b 100644 --- a/src/ast.c +++ b/src/ast.c @@ -177,14 +177,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(); @@ -610,6 +610,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)) diff --git a/src/builtins.c b/src/builtins.c index faa6d87df21565..9c0ce69522aae1 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -705,8 +705,17 @@ JL_CALLABLE(jl_f_setfield) JL_NARGS(setfield!, 3, 3); jl_value_t *v = args[0]; jl_value_t *vt = (jl_value_t*)jl_typeof(v); - if (vt == (jl_value_t*)jl_module_type) - jl_error("cannot assign variables in other modules"); + if (vt == (jl_value_t*)jl_module_type) { + jl_module_t *m = (jl_module_t*)v; + jl_sym_t *sym = (jl_sym_t*)args[1]; + if (!jl_is_symbol(sym)) + jl_type_error("setfield!", (jl_value_t*)jl_symbol_type, args[1]); + jl_binding_t *b = jl_get_binding_wr(m, sym); + if (b == NULL) + jl_undefined_var_error(sym); + jl_checked_assignment(b, args[2]); + return args[2]; + } if (!jl_is_datatype(vt)) jl_type_error("setfield!", (jl_value_t*)jl_datatype_type, v); jl_datatype_t *st = (jl_datatype_t*)vt; diff --git a/src/interpreter.c b/src/interpreter.c index 5c89505230ed41..00dc16f05726ce 100644 --- a/src/interpreter.c +++ b/src/interpreter.c @@ -246,9 +246,25 @@ 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]; - assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname)); + assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname) || + jl_is_globalref(fname)); - if (jl_is_symbol(fname)) { + if (jl_is_globalref(fname)) { + // allow defining a function using a existing owning binding + // in order for the following code, which may be the result of + // macro expansion, to work. + // + // global f + // M.f() = ... + jl_module_t *m = (jl_module_t*)jl_data_ptr(fname)[0]; + jl_sym_t *sym = (jl_sym_t*)jl_data_ptr(fname)[1]; + assert(jl_is_symbol(sym)); + jl_binding_t *b = jl_get_binding_for_method_def(m, sym); + jl_value_t *gf = jl_generic_function_def(sym, &b->value, + (jl_value_t*)m, b); + if (jl_expr_nargs(ex) == 1) + return gf; + } else if (jl_is_symbol(fname)) { jl_value_t **bp=NULL; jl_value_t *bp_owner=NULL; jl_binding_t *b=NULL; diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 82ddb610c6c791..dedb8352c1bf8f 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -1800,6 +1800,16 @@ (error (string "invalid assignment location \"" (deparse lhs) "\""))) (else (case (car lhs) + ((globalref) + ;; M.b = + (let* ((a (cadr lhs)) + (b (caddr lhs)) + (rhs (caddr e)) + (rr (if (or (symbol-like? rhs) (atom? rhs)) rhs (make-ssavalue)))) + `(block + ,.(if (eq? rr rhs) '() `((= ,rr ,(expand-forms rhs)))) + (call (core setfield!) ,a (quote ,b) ,rr) + (unnecessary ,rr)))) ((|.|) ;; a.b = (let* ((a (cadr lhs)) diff --git a/src/macroexpand.scm b/src/macroexpand.scm index 991b3da6ab06d7..d91b00847f9086 100644 --- a/src/macroexpand.scm +++ b/src/macroexpand.scm @@ -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)))))) diff --git a/test/core.jl b/test/core.jl index 3c490b387959a2..576844256c32ac 100644 --- a/test/core.jl +++ b/test/core.jl @@ -3490,7 +3490,7 @@ end # issue 13855 macro m13855() - Expr(:localize, :(() -> x)) + Expr(:localize, :(() -> $(esc(:x)))) end @noinline function foo13855(x) @m13855() @@ -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