Skip to content

Commit

Permalink
lowering: preserve line numbers over julia-expand-macroscope pass (#4…
Browse files Browse the repository at this point in the history
…4995)

This is to preserve the line number of the macro caller in the output,
in case we don't have context from eval on where it occured. But we
make slightly more changes than strictly necessary to prepare for
future improvements in this area.
  • Loading branch information
vtjnash authored May 25, 2023
1 parent 94bc2f3 commit 01ddf80
Show file tree
Hide file tree
Showing 17 changed files with 195 additions and 85 deletions.
3 changes: 1 addition & 2 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -533,11 +533,10 @@ import Core: CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, Return
end # module IR

# docsystem basics
const unescape = Symbol("hygienic-scope")
macro doc(x...)
docex = atdoc(__source__, __module__, x...)
isa(docex, Expr) && docex.head === :escape && return docex
return Expr(:escape, Expr(unescape, docex, typeof(atdoc).name.module))
return Expr(:escape, Expr(:var"hygienic-scope", docex, typeof(atdoc).name.module, __source__))
end
macro __doc__(x)
return Expr(:escape, Expr(:block, Expr(:meta, :doc), x))
Expand Down
32 changes: 24 additions & 8 deletions base/docs/Docs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,26 @@ catdoc(xs...) = vcat(xs...)
const keywords = Dict{Symbol, DocStr}()

function unblock(@nospecialize ex)
while isexpr(ex, :var"hygienic-scope")
isexpr(ex.args[1], :escape) || break
ex = ex.args[1].args[1]
end
isexpr(ex, :block) || return ex
exs = filter(ex -> !(isa(ex, LineNumberNode) || isexpr(ex, :line)), ex.args)
length(exs) == 1 || return ex
return unblock(exs[1])
end

# peek through ex to figure out what kind of expression it may eventually act like
# but ignoring scopes and line numbers
function unescape(@nospecialize ex)
ex = unblock(ex)
while isexpr(ex, :escape) || isexpr(ex, :var"hygienic-scope")
ex = unblock(ex.args[1])
end
return ex
end

uncurly(@nospecialize ex) = isexpr(ex, :curly) ? ex.args[1] : ex

namify(@nospecialize x) = astname(x, isexpr(x, :macro))::Union{Symbol,Expr,GlobalRef}
Expand Down Expand Up @@ -351,18 +365,19 @@ function metadata(__source__, __module__, expr, ismodule)
fields = P[]
last_docstr = nothing
for each in (expr.args[3]::Expr).args
if isa(each, Symbol) || isexpr(each, :(::))
eachex = unescape(each)
if isa(eachex, Symbol) || isexpr(eachex, :(::))
# a field declaration
if last_docstr !== nothing
push!(fields, P(namify(each::Union{Symbol,Expr}), last_docstr))
push!(fields, P(namify(eachex::Union{Symbol,Expr}), last_docstr))
last_docstr = nothing
end
elseif isexpr(each, :function) || isexpr(each, :(=))
elseif isexpr(eachex, :function) || isexpr(eachex, :(=))
break
elseif isa(each, String) || isexpr(each, :string) || isexpr(each, :call) ||
(isexpr(each, :macrocall) && each.args[1] === Symbol("@doc_str"))
elseif isa(eachex, String) || isexpr(eachex, :string) || isexpr(eachex, :call) ||
(isexpr(eachex, :macrocall) && eachex.args[1] === Symbol("@doc_str"))
# forms that might be doc strings
last_docstr = each::Union{String,Expr}
last_docstr = each
end
end
dict = :($(Dict{Symbol,Any})($([(:($(P)($(quot(f)), $d)))::Expr for (f, d) in fields]...)))
Expand Down Expand Up @@ -627,8 +642,9 @@ function loaddocs(docs::Vector{Core.SimpleVector})
for (mod, ex, str, file, line) in docs
data = Dict{Symbol,Any}(:path => string(file), :linenumber => line)
doc = docstr(str, data)
docstring = docm(LineNumberNode(line, file), mod, doc, ex, false) # expand the real @doc macro now
Core.eval(mod, Expr(Core.unescape, docstring, Docs))
lno = LineNumberNode(line, file)
docstring = docm(lno, mod, doc, ex, false) # expand the real @doc macro now
Core.eval(mod, Expr(:var"hygienic-scope", docstring, Docs, lno))
end
empty!(docs)
nothing
Expand Down
2 changes: 1 addition & 1 deletion base/osutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ macro static(ex)
@label loop
hd = ex.head
if hd (:if, :elseif, :&&, :||)
cond = Core.eval(__module__, ex.args[1])
cond = Core.eval(__module__, ex.args[1])::Bool
if xor(cond, hd === :||)
return esc(ex.args[2])
elseif length(ex.args) == 3
Expand Down
2 changes: 1 addition & 1 deletion base/threadcall.jl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ macro threadcall(f, rettype, argtypes, argvals...)
push!(body, :(return Int(Core.sizeof($rettype))))

# return code to generate wrapper function and send work request thread queue
wrapper = Expr(Symbol("hygienic-scope"), wrapper, @__MODULE__)
wrapper = Expr(:var"hygienic-scope", wrapper, @__MODULE__, __source__)
return :(let fun_ptr = @cfunction($wrapper, Int, (Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cvoid}))
# use cglobal to look up the function on the calling thread
do_threadcall(fun_ptr, cglobal($f), $rettype, Any[$(argtypes...)], Any[$(argvals...)])
Expand Down
2 changes: 1 addition & 1 deletion base/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ macro kwdef(expr)
kwdefs = nothing
end
return quote
Base.@__doc__ $(esc(expr))
$(esc(:($Base.@__doc__ $expr)))
$kwdefs
end
end
Expand Down
87 changes: 76 additions & 11 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,8 @@ static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mo
}
JL_CATCH {
// if expression cannot be converted, replace with error expr
//jl_(jl_current_exception());
//jlbacktrace();
jl_expr_t *ex = jl_exprn(jl_error_sym, 1);
v = (jl_value_t*)ex;
jl_array_ptr_set(ex->args, 0, jl_cstr_to_string("invalid AST"));
Expand Down Expand Up @@ -1000,7 +1002,59 @@ int jl_has_meta(jl_array_t *body, jl_sym_t *sym) JL_NOTSAFEPOINT
return 0;
}

static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule, jl_module_t **ctx, size_t world, int throw_load_error)
// Utility function to return whether `e` is any of the special AST types or
// will always evaluate to itself exactly unchanged. This corresponds to
// `is_self_quoting` in Core.Compiler utilities.
int jl_is_ast_node(jl_value_t *e) JL_NOTSAFEPOINT
{
return jl_is_newvarnode(e)
|| jl_is_code_info(e)
|| jl_is_linenode(e)
|| jl_is_gotonode(e)
|| jl_is_gotoifnot(e)
|| jl_is_returnnode(e)
|| jl_is_ssavalue(e)
|| jl_is_slotnumber(e)
|| jl_is_argument(e)
|| jl_is_quotenode(e)
|| jl_is_globalref(e)
|| jl_is_symbol(e)
|| jl_is_pinode(e)
|| jl_is_phinode(e)
|| jl_is_phicnode(e)
|| jl_is_upsilonnode(e)
|| jl_is_expr(e);
}

static int is_self_quoting_expr(jl_expr_t *e) JL_NOTSAFEPOINT
{
return (e->head == jl_inert_sym ||
e->head == jl_core_sym ||
e->head == jl_line_sym ||
e->head == jl_lineinfo_sym ||
e->head == jl_meta_sym ||
e->head == jl_boundscheck_sym ||
e->head == jl_inline_sym ||
e->head == jl_noinline_sym);
}

// any AST, except those that cannot contain symbols
// and have no side effects
int need_esc_node(jl_value_t *e) JL_NOTSAFEPOINT
{
if (jl_is_linenode(e)
|| jl_is_ssavalue(e)
|| jl_is_slotnumber(e)
|| jl_is_argument(e)
|| jl_is_quotenode(e))
return 0;
if (jl_is_expr(e))
return !is_self_quoting_expr((jl_expr_t*)e);
// note: jl_is_globalref(e) is not included here, since we care a little about about having a line number for it
return jl_is_ast_node(e);
}

static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule, jl_module_t **ctx, jl_value_t **lineinfo, size_t world, int throw_load_error)
{
jl_task_t *ct = jl_current_task;
JL_TIMING(MACRO_INVOCATION, MACRO_INVOCATION);
Expand All @@ -1012,10 +1066,9 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
margs[0] = jl_array_ptr_ref(args, 0);
// __source__ argument
jl_value_t *lno = jl_array_ptr_ref(args, 1);
if (!jl_is_linenode(lno))
lno = jl_new_struct(jl_linenumbernode_type, jl_box_long(0), jl_nothing);
margs[1] = lno;
if (!jl_is_linenode(lno)) {
margs[1] = jl_new_struct(jl_linenumbernode_type, jl_box_long(0), jl_nothing);
}
margs[2] = (jl_value_t*)inmodule;
for (i = 3; i < nargs; i++)
margs[i] = jl_array_ptr_ref(args, i - 1);
Expand Down Expand Up @@ -1054,6 +1107,7 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
}
}
ct->world_age = last_age;
*lineinfo = margs[1];
JL_GC_POP();
return result;
}
Expand All @@ -1076,36 +1130,47 @@ static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, str
JL_GC_POP();
return expr;
}
if (e->head == jl_hygienicscope_sym && jl_expr_nargs(e) == 2) {
if (e->head == jl_hygienicscope_sym && jl_expr_nargs(e) >= 2) {
struct macroctx_stack newctx;
newctx.m = (jl_module_t*)jl_exprarg(e, 1);
JL_TYPECHK(hygienic-scope, module, (jl_value_t*)newctx.m);
newctx.parent = macroctx;
jl_value_t *a = jl_exprarg(e, 0);
jl_value_t *a2 = jl_expand_macros(a, inmodule, &newctx, onelevel, world, throw_load_error);
if (a != a2)
if (jl_is_expr(a2) && ((jl_expr_t*)a2)->head == jl_escape_sym && !need_esc_node(jl_exprarg(a2, 0)))
expr = jl_exprarg(a2, 0);
else if (!need_esc_node(a2))
expr = a2;
else if (a != a2)
jl_array_ptr_set(e->args, 0, a2);
return expr;
}
if (e->head == jl_macrocall_sym) {
struct macroctx_stack newctx;
newctx.m = macroctx ? macroctx->m : inmodule;
newctx.parent = macroctx;
jl_value_t *result = jl_invoke_julia_macro(e->args, inmodule, &newctx.m, world, throw_load_error);
jl_value_t *lineinfo = NULL;
jl_value_t *result = jl_invoke_julia_macro(e->args, inmodule, &newctx.m, &lineinfo, world, throw_load_error);
if (!need_esc_node(result))
return result;
jl_value_t *wrap = NULL;
JL_GC_PUSH3(&result, &wrap, &newctx.m);
JL_GC_PUSH4(&result, &wrap, &newctx.m, &lineinfo);
// copy and wrap the result in `(hygienic-scope ,result ,newctx)
if (jl_is_expr(result) && ((jl_expr_t*)result)->head == jl_escape_sym)
result = jl_exprarg(result, 0);
else
wrap = (jl_value_t*)jl_exprn(jl_hygienicscope_sym, 2);
wrap = (jl_value_t*)jl_exprn(jl_hygienicscope_sym, 3);
result = jl_copy_ast(result);
if (!onelevel)
result = jl_expand_macros(result, inmodule, wrap ? &newctx : macroctx, onelevel, world, throw_load_error);
if (wrap) {
if (wrap && need_esc_node(result)) {
jl_exprargset(wrap, 0, result);
jl_exprargset(wrap, 1, newctx.m);
result = wrap;
jl_exprargset(wrap, 2, lineinfo);
if (jl_is_expr(result) && ((jl_expr_t*)result)->head == jl_escape_sym)
result = jl_exprarg(result, 0);
else
result = wrap;
}
JL_GC_POP();
return result;
Expand Down
9 changes: 5 additions & 4 deletions src/ast.scm
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,13 @@
(define (eq-sym? a b)
(or (eq? a b) (and (ssavalue? a) (ssavalue? b) (eqv? (cdr a) (cdr b)))))

(define (blockify e)
(define (blockify e (lno #f))
(set! lno (if lno (list lno) '()))
(if (and (pair? e) (eq? (car e) 'block))
(if (null? (cdr e))
`(block (null))
e)
`(block ,e)))
`(block ,@lno (null))
(if (null? lno) e `(block ,@lno ,@(cdr e))))
`(block ,@lno ,e)))

(define (make-var-info name) (list name '(core Any) 0))
(define vinfo:name car)
Expand Down
28 changes: 24 additions & 4 deletions src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,38 @@

;; lowering entry points

; find the first line number in this expression, before we might eliminate them
(define (first-lineno blk)
(cond ((not (pair? blk)) #f)
((eq? (car blk) 'line) blk)
((and (eq? (car blk) 'hygienic-scope) (pair? (cdddr blk)) (pair? (cadddr blk)) (eq? (car (cadddr blk)) 'line))
(cadddr blk))
((memq (car blk) '(escape hygienic-scope))
(first-lineno (cadr blk)))
((memq (car blk) '(toplevel block))
(let loop ((xs (cdr blk)))
(and (pair? xs)
(let ((elt (first-lineno (car xs))))
(or elt (loop (cdr xs)))))))
(else #f)))

;; return a lambda expression representing a thunk for a top-level expression
;; note: expansion of stuff inside module is delayed, so the contents obey
;; toplevel expansion order (don't expand until stuff before is evaluated).
(define (expand-toplevel-expr-- e file line)
(let ((ex0 (julia-expand-macroscope e)))
(let ((lno (first-lineno e))
(ex0 (julia-expand-macroscope e)))
(if (and lno (or (not (length= lno 3)) (not (atom? (caddr lno))))) (set! lno #f))
(if (toplevel-only-expr? ex0)
ex0
(let* ((ex (julia-expand0 ex0 file line))
(if (and (pair? e) (memq (car ex0) '(error incomplete)))
ex0
(if lno `(toplevel ,lno ,ex0) ex0))
(let* ((linenode (if (and lno (or (= line 0) (eq? file 'none))) lno `(line ,line ,file)))
(ex (julia-expand0 ex0 linenode))
(th (julia-expand1
`(lambda () ()
(scope-block
,(blockify ex)))
,(blockify ex lno)))
file line)))
(if (and (null? (cdadr (caddr th)))
(and (length= (lam:body th) 2)
Expand Down
6 changes: 3 additions & 3 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -5090,8 +5090,8 @@ f(x) = yt(x)

(define *current-desugar-loc* #f)

(define (julia-expand0 ex file line)
(with-bindings ((*current-desugar-loc* `(line ,line ,file)))
(define (julia-expand0 ex lno)
(with-bindings ((*current-desugar-loc* lno))
(trycatch (expand-forms ex)
(lambda (e)
(if (and (pair? e) (eq? (car e) 'error))
Expand All @@ -5106,4 +5106,4 @@ f(x) = yt(x)
(define (julia-expand ex (file 'none) (line 0))
(julia-expand1
(julia-expand0
(julia-expand-macroscope ex) file line) file line))
(julia-expand-macroscope ex) `(line ,line ,file)) file line))
11 changes: 7 additions & 4 deletions src/macroexpand.scm
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@
((hygienic-scope) ; TODO: move this lowering to resolve-scopes, instead of reimplementing it here badly
(let ((parent-scope (cons (list env m) parent-scope))
(body (cadr e))
(m (caddr e)))
(m (caddr e))
(lno (cdddr e)))
(resolve-expansion-vars-with-new-env body env m parent-scope inarg #t)))
((tuple)
(cons (car e)
Expand Down Expand Up @@ -574,7 +575,8 @@
((eq? (car e) 'module) e)
((eq? (car e) 'hygienic-scope)
(let ((form (cadr e)) ;; form is the expression returned from expand-macros
(modu (caddr e))) ;; m is the macro's def module
(modu (caddr e)) ;; m is the macro's def module
(lno (cdddr e))) ;; lno is (optionally) the line number node
(resolve-expansion-vars form modu)))
(else
(map julia-expand-macroscopes- e))))
Expand All @@ -585,8 +587,9 @@
((eq? (car e) 'hygienic-scope)
(let ((parent-scope (list relabels parent-scope))
(body (cadr e))
(m (caddr e)))
`(hygienic-scope ,(rename-symbolic-labels- (cadr e) (table) parent-scope) ,m)))
(m (caddr e))
(lno (cdddr e)))
`(hygienic-scope ,(rename-symbolic-labels- (cadr e) (table) parent-scope) ,m ,@lno)))
((and (eq? (car e) 'escape) (not (null? parent-scope)))
`(escape ,(apply rename-symbolic-labels- (cadr e) parent-scope)))
((or (eq? (car e) 'symbolicgoto) (eq? (car e) 'symboliclabel))
Expand Down
Loading

2 comments on commit 01ddf80

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

Please sign in to comment.