From 009b2105fbb1e06705081aa1d7db2c07196a346c Mon Sep 17 00:00:00 2001
From: Chris Foster <chris42f@gmail.com>
Date: Tue, 17 Mar 2020 21:39:29 +1000
Subject: [PATCH] Fix functionloc with line nodes in method signatures
This comes in three pieces:
* In the parser, put an addition LineNumberNode at the top of each
function body to track the line of the function definition
* In lowering, remove this LineNumberNode from the body and add it as
the last parameter to method signature in `Expr(:method, name, sig, body)`
* In the runtime, extract the line number node from sig and add it to
the jl_method_t.
Since the first line is now correctly tracked separately from the body,
a special case in coverage codegen was removed. This was required when
line numbers were tracked in a different way - see origin of this in the
use of `toplineno` in bug #17442 and fix #17470.
---
NEWS.md | 2 ++
doc/src/manual/constructors.md | 2 +-
doc/src/manual/types.md | 3 +-
src/codegen.cpp | 12 ++-----
src/julia-parser.scm | 5 +--
src/julia-syntax.scm | 52 ++++++++++++++++++++---------
src/method.c | 13 ++++----
src/toplevel.c | 2 ++
test/reflection.jl | 5 +--
test/show.jl | 3 +-
test/syntax.jl | 6 ++--
test/testhelpers/coverage_file.info | 5 +--
test/testhelpers/coverage_file.jl | 6 +++-
13 files changed, 72 insertions(+), 44 deletions(-)
diff --git a/NEWS.md b/NEWS.md
index c10dca7ad04ce..cb15f98beb1c3 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -63,6 +63,8 @@ Language changes
Now the result is "a\n b", since the space before `b` is no longer considered to occur
at the start of a line. The old behavior is considered a bug ([#35001]).
+* The line number of function definitions is now added by the parser as an
+ additional `LineNumberNode` at the start of each function body ([#35138]).
Multi-threading changes
-----------------------
diff --git a/doc/src/manual/constructors.md b/doc/src/manual/constructors.md
index 2a2c8dd4507bd..5e1541e39fb5d 100644
--- a/doc/src/manual/constructors.md
+++ b/doc/src/manual/constructors.md
@@ -550,7 +550,7 @@ julia> struct SummedArray{T<:Number,S<:Number}
julia> SummedArray(Int32[1; 2; 3], Int32(6))
ERROR: MethodError: no method matching SummedArray(::Array{Int32,1}, ::Int32)
Closest candidates are:
- SummedArray(::Array{T,1}) where T at none:5
+ SummedArray(::Array{T,1}) where T at none:4
```
This constructor will be invoked by the syntax `SummedArray(a)`. The syntax `new{T,S}` allows
diff --git a/doc/src/manual/types.md b/doc/src/manual/types.md
index e1ccdca1f052e..be216b7e9c217 100644
--- a/doc/src/manual/types.md
+++ b/doc/src/manual/types.md
@@ -1207,8 +1207,7 @@ is raised:
julia> supertype(Union{Float64,Int64})
ERROR: MethodError: no method matching supertype(::Type{Union{Float64, Int64}})
Closest candidates are:
- supertype(!Matched::DataType) at operators.jl:42
- supertype(!Matched::UnionAll) at operators.jl:47
+[...]
```
## [Custom pretty-printing](@id man-custom-pretty-printing)
diff --git a/src/codegen.cpp b/src/codegen.cpp
index 7435bb1542c4c..047ef93e0f701 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -5820,10 +5820,13 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
auto coverageVisitStmt = [&] (size_t dbg) {
if (dbg == 0)
return;
+ // Compute inlining stack for current line, inner frame first
while (dbg) {
new_lineinfo.push_back(dbg);
dbg = linetable.at(dbg).inlined_at;
}
+ // Visit frames which differ from previous statement as tracked in
+ // current_lineinfo (tracked outer frame first).
current_lineinfo.resize(new_lineinfo.size(), 0);
for (dbg = 0; dbg < new_lineinfo.size(); dbg++) {
unsigned newdbg = new_lineinfo[new_lineinfo.size() - dbg - 1];
@@ -5906,15 +5909,6 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
BB[label] = bb;
}
- if (do_coverage(mod_is_user_mod)) {
- coverageVisitLine(ctx, ctx.file, toplineno);
- if (linetable.size() >= 2) {
- // avoid double-counting the entry line
- const auto &info = linetable.at(1);
- if (info.file == ctx.file && info.line == toplineno && info.is_user_code == mod_is_user_mod)
- current_lineinfo.push_back(1);
- }
- }
Value *sync_bytes = nullptr;
if (do_malloc_log(true))
sync_bytes = ctx.builder.CreateCall(prepare_call(diff_gc_total_bytes_func), {});
diff --git a/src/julia-parser.scm b/src/julia-parser.scm
index abf519c82985f..379fb63f32aec 100644
--- a/src/julia-parser.scm
+++ b/src/julia-parser.scm
@@ -1419,7 +1419,8 @@
`(const ,assgn))))
((function macro)
- (let* ((paren (eqv? (require-token s) #\())
+ (let* ((loc (line-number-node s))
+ (paren (eqv? (require-token s) #\())
(sig (parse-def s (eq? word 'function) paren)))
(if (and (not paren) (symbol-or-interpolate? sig))
(begin (if (not (eq? (require-token s) 'end))
@@ -1440,7 +1441,7 @@
sig)))
(body (parse-block s)))
(expect-end s word)
- (list word def body)))))
+ (list word def (add-line-number body loc))))))
((abstract)
(if (not (eq? (peek-token s) 'type))
diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
index c644e409723e6..4df69cb5f5a5e 100644
--- a/src/julia-syntax.scm
+++ b/src/julia-syntax.scm
@@ -231,7 +231,7 @@
(let ((type-ex (caddr m)))
(if (eq? (car type-ex) 'block)
;; extract ssavalue labels of sparams from the svec-of-sparams argument to `method`
- (let ((sp-ssavals (cddr (last (last type-ex)))))
+ (let ((sp-ssavals (cddr (cadddr (last type-ex)))))
(map (lambda (a) ;; extract T from (= v (call (core TypeVar) (quote T) ...))
(cadr (caddr (caddr a))))
(filter (lambda (e)
@@ -293,6 +293,24 @@
(define (non-generated-version body)
(generated-part- body #f))
+;; Remove and return the line number for the start of the function definition
+(define (maybe-remove-functionloc! body)
+ (let* ((prologue (extract-method-prologue body))
+ (prologue-lnos (filter linenum? prologue))
+ (functionloc (if (pair? prologue-lnos)
+ (car prologue-lnos)
+ ; Fallback - take first line anywhere in body
+ (let ((lnos (filter linenum? body)))
+ (if (null? lnos) '(line 0 none) (car lnos))))))
+ (if (length> prologue-lnos 1)
+ ; First of two line numbers in prologue is function definition location
+ ; which should be removed from the body.
+ (let loop ((stmts body))
+ (if (eq? functionloc (cadr stmts))
+ (set-cdr! stmts (cddr stmts))
+ (loop (cdr body)))))
+ functionloc))
+
;; construct the (method ...) expression for one primitive method definition,
;; assuming optional and keyword args are already handled
(define (method-def-expr- name sparams argl body (rett '(core Any)))
@@ -328,12 +346,12 @@
(error "function argument and static parameter names must be distinct"))
(if (or (and name (not (sym-ref? name))) (not (valid-name? name)))
(error (string "invalid function name \"" (deparse name) "\"")))
- (let* ((generator (if (expr-contains-p if-generated? body (lambda (x) (not (function-def? x))))
+ (let* ((loc (maybe-remove-functionloc! body))
+ (generator (if (expr-contains-p if-generated? body (lambda (x) (not (function-def? x))))
(let* ((gen (generated-version body))
(nongen (non-generated-version body))
(gname (symbol (string (gensy) "#" (current-julia-module-counter))))
- (gf (make-generator-function gname names anames gen))
- (loc (function-body-lineno body)))
+ (gf (make-generator-function gname names anames gen)))
(set! body (insert-after-meta
nongen
`((meta generated
@@ -343,8 +361,8 @@
,(if (null? sparams)
'nothing
(cons 'list (map car sparams)))
- ,(if (null? loc) 0 (cadr loc))
- (inert ,(if (null? loc) 'none (caddr loc)))
+ ,(cadr loc)
+ (inert ,(caddr loc))
(false))))))
(list gf))
'()))
@@ -356,7 +374,11 @@
(renames (map cons names temps))
(mdef
(if (null? sparams)
- `(method ,name (call (core svec) (call (core svec) ,@(dots->vararg types)) (call (core svec)))
+ `(method ,name
+ (call (core svec)
+ (call (core svec) ,@(dots->vararg types))
+ (call (core svec))
+ (inert ,loc))
,body)
`(method ,name
(block
@@ -377,7 +399,8 @@
(map (lambda (ty)
(replace-vars ty renames))
types)))
- (call (core svec) ,@temps)))
+ (call (core svec) ,@temps)
+ (inert ,loc)))
,body))))
(if (or (symbol? name) (globalref? name))
`(block ,@generator (method ,name) ,mdef (unnecessary ,name)) ;; return the function
@@ -793,10 +816,6 @@
wheres)
,(ctor-body body curlyargs sparams))))))
-(define (function-body-lineno body)
- (let ((lnos (filter linenum? body)))
- (if (null? lnos) '() (car lnos))))
-
;; rewrite calls to `new( ... )` to `new` expressions on the appropriate
;; type, determined by the containing constructor definition.
(define (rewrite-ctor ctor Tname params field-names field-types)
@@ -3021,9 +3040,12 @@ f(x) = yt(x)
(newtypes
(if iskw
`(,(car types) ,(cadr types) ,closure-type ,@(cdddr types))
- `(,closure-type ,@(cdr types)))))
- `(call (core svec) (call (core svec) ,@newtypes)
- (call (core svec) ,@(append (cddr (cadddr te)) type-sp)))))
+ `(,closure-type ,@(cdr types))))
+ (loc (caddddr te)))
+ `(call (core svec)
+ (call (core svec) ,@newtypes)
+ (call (core svec) ,@(append (cddr (cadddr te)) type-sp))
+ ,loc)))
;; collect all toplevel-butfirst expressions inside `e`, and return
;; (ex . stmts), where `ex` is the expression to evaluated and
diff --git a/src/method.c b/src/method.c
index 2508cdf6ea375..1fd2b43e7930b 100644
--- a/src/method.c
+++ b/src/method.c
@@ -504,12 +504,6 @@ static void jl_method_set_source(jl_method_t *m, jl_code_info_t *src)
jl_array_t *stmts = (jl_array_t*)src->code;
size_t i, n = jl_array_len(stmts);
copy = jl_alloc_vec_any(n);
- // set location from first LineInfoNode
- if (jl_array_len(src->linetable) > 0) {
- jl_value_t *ln = jl_array_ptr_ref(src->linetable, 0);
- m->file = (jl_sym_t*)jl_fieldref(ln, 1);
- m->line = jl_unbox_long(jl_fieldref(ln, 2));
- }
for (i = 0; i < n; i++) {
jl_value_t *st = jl_array_ptr_ref(stmts, i);
if (jl_is_expr(st) && ((jl_expr_t*)st)->head == meta_sym) {
@@ -698,9 +692,10 @@ JL_DLLEXPORT void jl_method_def(jl_svec_t *argdata,
jl_code_info_t *f,
jl_module_t *module)
{
- // argdata is svec(svec(types...), svec(typevars...))
+ // argdata is svec(svec(types...), svec(typevars...), functionloc)
jl_svec_t *atypes = (jl_svec_t*)jl_svecref(argdata, 0);
jl_svec_t *tvars = (jl_svec_t*)jl_svecref(argdata, 1);
+ jl_value_t *functionloc = jl_svecref(argdata, 2);
size_t nargs = jl_svec_len(atypes);
int isva = jl_is_vararg_type(jl_svecref(atypes, nargs - 1));
assert(jl_is_svec(atypes));
@@ -755,6 +750,10 @@ JL_DLLEXPORT void jl_method_def(jl_svec_t *argdata,
m->name = name;
m->isva = isva;
m->nargs = nargs;
+ assert(jl_is_linenode(functionloc));
+ jl_value_t *file = jl_linenode_file(functionloc);
+ m->file = jl_is_symbol(file) ? (jl_sym_t*)file : empty_sym;
+ m->line = jl_linenode_line(functionloc);
jl_method_set_source(m, f);
if (jl_has_free_typevars(argtype)) {
diff --git a/src/toplevel.c b/src/toplevel.c
index a245d1af3dec4..939e3748c4d6a 100644
--- a/src/toplevel.c
+++ b/src/toplevel.c
@@ -830,6 +830,8 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
jl_value_t *v = NULL;
int last_lineno = jl_lineno;
const char *last_filename = jl_filename;
+ jl_lineno = 1;
+ jl_filename = "none";
if (jl_options.incremental && jl_generating_output()) {
if (!ptrhash_has(&jl_current_modules, (void*)m)) {
if (m != jl_main_module) { // TODO: this was grand-fathered in
diff --git a/test/reflection.jl b/test/reflection.jl
index b7f4947cefb34..f79bc25a42118 100644
--- a/test/reflection.jl
+++ b/test/reflection.jl
@@ -288,6 +288,7 @@ let rts = return_types(TLayout)
end
# issue #15447
+f15447_line = @__LINE__() + 1
@noinline function f15447(s, a)
if s
return a
@@ -296,7 +297,7 @@ end
return nb
end
end
-@test functionloc(f15447)[2] > 0
+@test functionloc(f15447)[2] == f15447_line
# issue #14346
@noinline function f14346(id, mask, limit)
@@ -304,7 +305,7 @@ end
return true
end
end
-@test functionloc(f14346)[2] == @__LINE__() - 4
+@test functionloc(f14346)[2] == @__LINE__() - 5
# issue #15714
# show variable names for slots and suppress spurious type warnings
diff --git a/test/show.jl b/test/show.jl
index 8df1b87af3fde..ded0d6cdba0c0 100644
--- a/test/show.jl
+++ b/test/show.jl
@@ -922,6 +922,7 @@ end"""
end""")) ==
"""
:(macro m(a, b)
+ #= none:1 =#
#= none:2 =#
quote
#= none:3 =#
@@ -944,7 +945,7 @@ end"""))) ==
"""macro m(a, b)
:(\$a + \$b)
end""")) ==
-":(macro m(a, b)\n #= none:2 =#\n :(\$a + \$b)\n end)"
+":(macro m(a, b)\n #= none:1 =#\n #= none:2 =#\n :(\$a + \$b)\n end)"
@test repr(Expr(:macro, Expr(:call, :m, :x), Expr(:quote, Expr(:call, :+, Expr(:($), :x), 1)))) ==
":(macro m(x)\n :(\$x + 1)\n end)"
diff --git a/test/syntax.jl b/test/syntax.jl
index 56fe174acdbae..e36b195fa1354 100644
--- a/test/syntax.jl
+++ b/test/syntax.jl
@@ -162,11 +162,12 @@ macro test999_str(args...); args; end
@test parseall(":(a = &\nb)") == Expr(:quote, Expr(:(=), :a, Expr(:&, :b)))
@test parseall(":(a = \$\nb)") == Expr(:quote, Expr(:(=), :a, Expr(:$, :b)))
-# issue 11970
+# issue 12027 - short macro name parsing vs _str suffix
@test parseall("""
macro f(args...) end; @f "macro argument"
""") == Expr(:toplevel,
- Expr(:macro, Expr(:call, :f, Expr(:..., :args)), Expr(:block, LineNumberNode(1, :none))),
+ Expr(:macro, Expr(:call, :f, Expr(:..., :args)),
+ Expr(:block, LineNumberNode(1, :none), LineNumberNode(1, :none))),
Expr(:macrocall, Symbol("@f"), LineNumberNode(1, :none), "macro argument"))
# blocks vs. tuples
@@ -1423,6 +1424,7 @@ let ex = Meta.lower(@__MODULE__, Meta.parse("
@test isa(ex, Expr) && ex.head === :error
@test ex.args[1] == """
invalid assignment location "function (s, o...)
+ # none, line 2
# none, line 3
f(a, b) do
# none, line 4
diff --git a/test/testhelpers/coverage_file.info b/test/testhelpers/coverage_file.info
index ee58b7e270bb9..5b7b72789bf73 100644
--- a/test/testhelpers/coverage_file.info
+++ b/test/testhelpers/coverage_file.info
@@ -7,6 +7,7 @@ DA:9,5
DA:11,1
DA:12,1
DA:14,0
-LH:6
-LF:8
+DA:17,1
+LH:7
+LF:9
end_of_record
diff --git a/test/testhelpers/coverage_file.jl b/test/testhelpers/coverage_file.jl
index 56fa2897cd99b..986aaeec9f300 100644
--- a/test/testhelpers/coverage_file.jl
+++ b/test/testhelpers/coverage_file.jl
@@ -14,6 +14,10 @@ function code_coverage_test()
not_reached
end
-exit(code_coverage_test() == [1, 2, 3] ? 0 : 1)
+short_form_func_coverage_test(x) = x*x
+
+success = code_coverage_test() == [1, 2, 3] &&
+ short_form_func_coverage_test(2) == 4
+exit(success ? 0 : 1)
# end of file