Skip to content

Commit 7a0e916

Browse files
committed
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
1 parent 6f8e94e commit 7a0e916

10 files changed

+96
-18
lines changed

base/docs/Docs.jl

+3
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,9 @@ function docm(meta, ex, define = true)
606606
# Don't try to redefine expressions. This is only needed for `Base` img gen since
607607
# otherwise calling `loaddocs` would redefine all documented functions and types.
608608
def = define ? x : nothing
609+
if isa(x, GlobalRef) && (x::GlobalRef).mod == current_module()
610+
x = (x::GlobalRef).name
611+
end
609612

610613
# Keywords using the `@kw_str` macro in `base/docs/basedocs.jl`.
611614
#

base/linalg/factorization.jl

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ transpose(F::Factorization) = error("transpose not implemented for $(typeof(F))"
99
ctranspose(F::Factorization) = error("ctranspose not implemented for $(typeof(F))")
1010

1111
macro assertposdef(A, info)
12-
:(($info)==0 ? $A : throw(PosDefException($info)))
12+
:($(esc(info)) == 0 ? $(esc(A)) : throw(PosDefException($(esc(info)))))
1313
end
1414

1515
macro assertnonsingular(A, info)
16-
:(($info)==0 ? $A : throw(SingularException($info)))
16+
:($(esc(info)) == 0 ? $(esc(A)) : throw(SingularException($(esc(info)))))
1717
end
1818

1919
function logdet(F::Factorization)

base/sparse/cholmod_h.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,5 @@ type CHOLMODException <: Exception
7575
end
7676

7777
macro isok(A)
78-
:($A == TRUE || throw(CHOLMODException("")))
78+
:($(esc(A)) == TRUE || throw(CHOLMODException("")))
7979
end

base/sparse/umfpack.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function umferror(status::Integer)
5454
end
5555

5656
macro isok(A)
57-
:(umferror($A))
57+
:(umferror($(esc(A))))
5858
end
5959

6060
# check the size of SuiteSparse_long

src/ast.c

+21-8
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,14 @@ value_t fl_invoke_julia_macro(fl_context_t *fl_ctx, value_t *args, uint32_t narg
177177
fl_gc_handle(fl_ctx, &scm);
178178
value_t scmresult;
179179
jl_module_t *defmod = mfunc->def->module;
180-
if (defmod == NULL || defmod == ptls->current_module) {
181-
scmresult = fl_cons(fl_ctx, scm, fl_ctx->F);
182-
}
183-
else {
184-
value_t opaque = cvalue(fl_ctx, jl_ast_ctx(fl_ctx)->jvtype, sizeof(void*));
185-
*(jl_value_t**)cv_data((cvalue_t*)ptr(opaque)) = (jl_value_t*)defmod;
186-
scmresult = fl_cons(fl_ctx, scm, opaque);
187-
}
180+
/* if (defmod == NULL || defmod == ptls->current_module) { */
181+
/* scmresult = fl_cons(fl_ctx, scm, fl_ctx->F); */
182+
/* } */
183+
/* else { */
184+
value_t opaque = cvalue(fl_ctx, jl_ast_ctx(fl_ctx)->jvtype, sizeof(void*));
185+
*(jl_value_t**)cv_data((cvalue_t*)ptr(opaque)) = (jl_value_t*)defmod;
186+
scmresult = fl_cons(fl_ctx, scm, opaque);
187+
/* } */
188188
fl_free_gc_handles(fl_ctx, 1);
189189

190190
JL_GC_POP();
@@ -610,6 +610,19 @@ static value_t julia_to_scm_(fl_context_t *fl_ctx, jl_value_t *v)
610610
return julia_to_list2(fl_ctx, (jl_value_t*)inert_sym, jl_fieldref(v,0));
611611
if (jl_typeis(v, jl_newvarnode_type))
612612
return julia_to_list2(fl_ctx, (jl_value_t*)newvar_sym, jl_fieldref(v,0));
613+
if (jl_typeis(v, jl_globalref_type)) {
614+
jl_module_t *m = jl_globalref_mod(v);
615+
jl_sym_t *sym = jl_globalref_name(v);
616+
if (m == jl_core_module)
617+
return julia_to_list2(fl_ctx, (jl_value_t*)core_sym,
618+
(jl_value_t*)sym);
619+
value_t args = julia_to_list2(fl_ctx, (jl_value_t*)m, (jl_value_t*)sym);
620+
fl_gc_handle(fl_ctx, &args);
621+
value_t hd = julia_to_scm_(fl_ctx, (jl_value_t*)globalref_sym);
622+
value_t scmv = fl_cons(fl_ctx, hd, args);
623+
fl_free_gc_handles(fl_ctx, 1);
624+
return scmv;
625+
}
613626
if (jl_is_long(v) && fits_fixnum(jl_unbox_long(v)))
614627
return fixnum(jl_unbox_long(v));
615628
if (jl_is_ssavalue(v))

src/builtins.c

+11-2
Original file line numberDiff line numberDiff line change
@@ -705,8 +705,17 @@ JL_CALLABLE(jl_f_setfield)
705705
JL_NARGS(setfield!, 3, 3);
706706
jl_value_t *v = args[0];
707707
jl_value_t *vt = (jl_value_t*)jl_typeof(v);
708-
if (vt == (jl_value_t*)jl_module_type)
709-
jl_error("cannot assign variables in other modules");
708+
if (vt == (jl_value_t*)jl_module_type) {
709+
jl_module_t *m = (jl_module_t*)v;
710+
jl_sym_t *sym = (jl_sym_t*)args[1];
711+
if (!jl_is_symbol(sym))
712+
jl_type_error("setfield!", (jl_value_t*)jl_symbol_type, args[1]);
713+
jl_binding_t *b = jl_get_binding_wr(m, sym);
714+
if (b == NULL)
715+
jl_undefined_var_error(sym);
716+
jl_checked_assignment(b, args[2]);
717+
return args[2];
718+
}
710719
if (!jl_is_datatype(vt))
711720
jl_type_error("setfield!", (jl_value_t*)jl_datatype_type, v);
712721
jl_datatype_t *st = (jl_datatype_t*)vt;

src/interpreter.c

+18-2
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,25 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
246246
}
247247
else if (ex->head == method_sym) {
248248
jl_sym_t *fname = (jl_sym_t*)args[0];
249-
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname));
249+
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname) ||
250+
jl_is_globalref(fname));
250251

251-
if (jl_is_symbol(fname)) {
252+
if (jl_is_globalref(fname)) {
253+
// allow defining a function using a existing owning binding
254+
// in order for the following code, which may be the result of
255+
// macro expansion, to work.
256+
//
257+
// global f
258+
// M.f() = ...
259+
jl_module_t *m = (jl_module_t*)jl_data_ptr(fname)[0];
260+
jl_sym_t *sym = (jl_sym_t*)jl_data_ptr(fname)[1];
261+
assert(jl_is_symbol(sym));
262+
jl_binding_t *b = jl_get_binding_for_method_def(m, sym);
263+
jl_value_t *gf = jl_generic_function_def(sym, &b->value,
264+
(jl_value_t*)m, b);
265+
if (jl_expr_nargs(ex) == 1)
266+
return gf;
267+
} else if (jl_is_symbol(fname)) {
252268
jl_value_t **bp=NULL;
253269
jl_value_t *bp_owner=NULL;
254270
jl_binding_t *b=NULL;

src/julia-syntax.scm

+10
Original file line numberDiff line numberDiff line change
@@ -1800,6 +1800,16 @@
18001800
(error (string "invalid assignment location \"" (deparse lhs) "\"")))
18011801
(else
18021802
(case (car lhs)
1803+
((globalref)
1804+
;; M.b =
1805+
(let* ((a (cadr lhs))
1806+
(b (caddr lhs))
1807+
(rhs (caddr e))
1808+
(rr (if (or (symbol-like? rhs) (atom? rhs)) rhs (make-ssavalue))))
1809+
`(block
1810+
,.(if (eq? rr rhs) '() `((= ,rr ,(expand-forms rhs))))
1811+
(call (core setfield!) ,a (quote ,b) ,rr)
1812+
(unnecessary ,rr))))
18031813
((|.|)
18041814
;; a.b =
18051815
(let* ((a (cadr lhs))

src/macroexpand.scm

+1-1
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@
423423
(error (cadr form)))
424424
(let ((form (car form))
425425
(m (cdr form)))
426-
;; m is the macro's def module, or #f if def env === use env
426+
;; m is the macro's def module
427427
(rename-symbolic-labels
428428
(julia-expand-macros
429429
(resolve-expansion-vars form m))))))

test/core.jl

+28-1
Original file line numberDiff line numberDiff line change
@@ -3490,7 +3490,7 @@ end
34903490

34913491
# issue 13855
34923492
macro m13855()
3493-
Expr(:localize, :(() -> x))
3493+
Expr(:localize, :(() -> $(esc(:x))))
34943494
end
34953495
@noinline function foo13855(x)
34963496
@m13855()
@@ -4794,3 +4794,30 @@ end
47944794
@test let_Box4()() == 44
47954795
@test let_Box5()() == 46
47964796
@test let_noBox()() == 21
4797+
4798+
module TestModuleAssignment
4799+
using Base.Test
4800+
@eval $(GlobalRef(TestModuleAssignment, :x)) = 1
4801+
@test x == 1
4802+
@eval $(GlobalRef(TestModuleAssignment, :x)) = 2
4803+
@test x == 2
4804+
end
4805+
4806+
# issue #14893
4807+
module M14893
4808+
x = 14893
4809+
macro m14893()
4810+
:x
4811+
end
4812+
function f14893()
4813+
x = 1
4814+
@m14893
4815+
end
4816+
end
4817+
function f14893()
4818+
x = 2
4819+
M14893.@m14893
4820+
end
4821+
4822+
@test f14893() == 14893
4823+
@test M14893.f14893() == 14893

0 commit comments

Comments
 (0)