Skip to content

Commit a9ed484

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 4904241 commit a9ed484

9 files changed

+96
-26
lines changed

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

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
@@ -172,14 +172,14 @@ value_t fl_invoke_julia_macro(fl_context_t *fl_ctx, value_t *args, uint32_t narg
172172
fl_gc_handle(fl_ctx, &scm);
173173
value_t scmresult;
174174
jl_module_t *defmod = mfunc->def->module;
175-
if (defmod == NULL || defmod == jl_current_module) {
176-
scmresult = fl_cons(fl_ctx, scm, fl_ctx->F);
177-
}
178-
else {
179-
value_t opaque = cvalue(fl_ctx, jl_ast_ctx(fl_ctx)->jvtype, sizeof(void*));
180-
*(jl_value_t**)cv_data((cvalue_t*)ptr(opaque)) = (jl_value_t*)defmod;
181-
scmresult = fl_cons(fl_ctx, scm, opaque);
182-
}
175+
/* if (defmod == NULL || defmod == jl_current_module) { */
176+
/* scmresult = fl_cons(fl_ctx, scm, fl_ctx->F); */
177+
/* } */
178+
/* else { */
179+
value_t opaque = cvalue(fl_ctx, jl_ast_ctx(fl_ctx)->jvtype, sizeof(void*));
180+
*(jl_value_t**)cv_data((cvalue_t*)ptr(opaque)) = (jl_value_t*)defmod;
181+
scmresult = fl_cons(fl_ctx, scm, opaque);
182+
/* } */
183183
fl_free_gc_handles(fl_ctx, 1);
184184

185185
JL_GC_POP();
@@ -667,6 +667,19 @@ static value_t julia_to_scm_(fl_context_t *fl_ctx, jl_value_t *v)
667667
return julia_to_list2(fl_ctx, (jl_value_t*)inert_sym, jl_fieldref(v,0));
668668
if (jl_typeis(v, jl_newvarnode_type))
669669
return julia_to_list2(fl_ctx, (jl_value_t*)newvar_sym, jl_fieldref(v,0));
670+
if (jl_typeis(v, jl_globalref_type)) {
671+
jl_module_t *m = jl_globalref_mod(v);
672+
jl_sym_t *sym = jl_globalref_name(v);
673+
if (m == jl_core_module)
674+
return julia_to_list2(fl_ctx, (jl_value_t*)core_sym,
675+
(jl_value_t*)sym);
676+
value_t args = julia_to_list2(fl_ctx, (jl_value_t*)m, (jl_value_t*)sym);
677+
fl_gc_handle(fl_ctx, &args);
678+
value_t hd = julia_to_scm_(fl_ctx, (jl_value_t*)globalref_sym);
679+
value_t scmv = fl_cons(fl_ctx, hd, args);
680+
fl_free_gc_handles(fl_ctx, 1);
681+
return scmv;
682+
}
670683
if (jl_is_long(v) && fits_fixnum(jl_unbox_long(v)))
671684
return fixnum(jl_unbox_long(v));
672685
if (jl_is_ssavalue(v))

src/builtins.c

+11-2
Original file line numberDiff line numberDiff line change
@@ -676,8 +676,17 @@ JL_CALLABLE(jl_f_setfield)
676676
JL_NARGS(setfield!, 3, 3);
677677
jl_value_t *v = args[0];
678678
jl_value_t *vt = (jl_value_t*)jl_typeof(v);
679-
if (vt == (jl_value_t*)jl_module_type)
680-
jl_error("cannot assign variables in other modules");
679+
if (vt == (jl_value_t*)jl_module_type) {
680+
jl_module_t *m = (jl_module_t*)v;
681+
jl_sym_t *sym = (jl_sym_t*)args[1];
682+
if (!jl_is_symbol(sym))
683+
jl_type_error("setfield!", (jl_value_t*)jl_symbol_type, args[1]);
684+
jl_binding_t *b = jl_get_binding_wr(m, sym);
685+
if (b == NULL)
686+
jl_undefined_var_error(sym);
687+
jl_checked_assignment(b, args[2]);
688+
return args[2];
689+
}
681690
if (!jl_is_datatype(vt))
682691
jl_type_error("setfield!", (jl_value_t*)jl_datatype_type, v);
683692
jl_datatype_t *st = (jl_datatype_t*)vt;

src/interpreter.c

+21-10
Original file line numberDiff line numberDiff line change
@@ -235,17 +235,28 @@ static jl_value_t *eval(jl_value_t *e, jl_value_t **locals, jl_lambda_info_t *la
235235
}
236236
else if (ex->head == method_sym) {
237237
jl_sym_t *fname = (jl_sym_t*)args[0];
238-
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname));
238+
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(fname) ||
239+
jl_is_globalref(fname));
239240

240-
if (jl_is_symbol(fname)) {
241-
jl_value_t **bp=NULL;
242-
jl_value_t *bp_owner=NULL;
243-
jl_binding_t *b=NULL;
244-
if (bp == NULL) {
245-
b = jl_get_binding_for_method_def(jl_current_module, fname);
246-
bp = &b->value;
247-
bp_owner = (jl_value_t*)jl_current_module;
248-
}
241+
if (jl_is_globalref(fname)) {
242+
// allow defining a function using a existing owning binding
243+
// in order for the following code, which may be the result of
244+
// macro expansion, to work.
245+
//
246+
// global f
247+
// M.f() = ...
248+
jl_module_t *m = (jl_module_t*)jl_data_ptr(fname)[0];
249+
jl_sym_t *sym = (jl_sym_t*)jl_data_ptr(fname)[1];
250+
assert(jl_is_symbol(sym));
251+
jl_binding_t *b = jl_get_binding_for_method_def(m, sym);
252+
jl_value_t *gf = jl_generic_function_def(sym, &b->value,
253+
(jl_value_t*)m, b);
254+
if (jl_expr_nargs(ex) == 1)
255+
return gf;
256+
} else if (jl_is_symbol(fname)) {
257+
jl_binding_t *b = jl_get_binding_for_method_def(jl_current_module, fname);
258+
jl_value_t **bp = &b->value;
259+
jl_value_t *bp_owner = (jl_value_t*)jl_current_module;
249260
jl_value_t *gf = jl_generic_function_def(fname, bp, bp_owner, b);
250261
if (jl_expr_nargs(ex) == 1)
251262
return gf;

src/julia-syntax.scm

+10
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,16 @@
16071607
(error (string "invalid assignment location \"" (deparse lhs) "\"")))
16081608
(else
16091609
(case (car lhs)
1610+
((globalref)
1611+
;; M.b =
1612+
(let* ((a (cadr lhs))
1613+
(b (caddr lhs))
1614+
(rhs (caddr e))
1615+
(rr (if (or (symbol-like? rhs) (atom? rhs)) rhs (make-ssavalue))))
1616+
`(block
1617+
,.(if (eq? rr rhs) '() `((= ,rr ,(expand-forms rhs))))
1618+
(call (core setfield!) ,a (quote ,b) ,rr)
1619+
(unnecessary ,rr))))
16101620
((|.|)
16111621
;; a.b =
16121622
(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
@@ -3398,7 +3398,7 @@ end
33983398

33993399
# issue 13855
34003400
macro m13855()
3401-
Expr(:localize, :(() -> x))
3401+
Expr(:localize, :(() -> $(esc(:x))))
34023402
end
34033403
@noinline function foo13855(x)
34043404
@m13855()
@@ -4203,3 +4203,30 @@ end
42034203
g1090{T}(x::T)::T = x+1.0
42044204
@test g1090(1) === 2
42054205
@test g1090(Float32(3)) === Float32(4)
4206+
4207+
module TestModuleAssignment
4208+
using Base.Test
4209+
@eval $(GlobalRef(TestModuleAssignment, :x)) = 1
4210+
@test x == 1
4211+
@eval $(GlobalRef(TestModuleAssignment, :x)) = 2
4212+
@test x == 2
4213+
end
4214+
4215+
# issue #14893
4216+
module M14893
4217+
x = 14893
4218+
macro m14893()
4219+
:x
4220+
end
4221+
function f14893()
4222+
x = 1
4223+
@m14893
4224+
end
4225+
end
4226+
function f14893()
4227+
x = 2
4228+
M14893.@m14893
4229+
end
4230+
4231+
@test f14893() == 14893
4232+
@test M14893.f14893() == 14893

0 commit comments

Comments
 (0)