Skip to content

Commit

Permalink
fix slurping into function definition (#40738)
Browse files Browse the repository at this point in the history
Discovered while working on #40737. Currently, this throws an
unintuitive error:
```julia
julia> a, f()... = 1, 2, 3
ERROR: syntax: ssavalue with no def
Stacktrace:
 [1] top-level scope
   @ REPL[1]:1
```

Might as well fix this properly - Why not allow function definitions to
slurp a little from time to time? ;)
  • Loading branch information
simeonschaub authored Jul 2, 2021
1 parent 10b010f commit 5c49a0d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,7 @@
(cons R elts)))
((vararg? L)
(if (null? (cdr lhss))
(let ((temp (make-ssavalue)))
(let ((temp (if (eventually-call? (cadr L)) (gensy) (make-ssavalue))))
`(block ,@(reverse stmts)
(= ,temp (tuple ,@rhss))
,@(reverse after)
Expand Down Expand Up @@ -2155,9 +2155,13 @@
((eq? l x) #t)
(else (in-lhs? x (cdr lhss)))))))
;; in-lhs? also checks for invalid syntax, so always call it first
(let* ((xx (if (or (and (not (in-lhs? x lhss)) (symbol? x))
(ssavalue? x))
x (make-ssavalue)))
(let* ((xx (cond ((or (and (not (in-lhs? x lhss)) (symbol? x))
(ssavalue? x))
x)
((and (pair? lhss) (vararg? (last lhss))
(eventually-call? (cadr (last lhss))))
(gensy))
(else (make-ssavalue))))
(ini (if (eq? x xx) '() (list (sink-assignment xx (expand-forms x)))))
(n (length lhss))
;; skip last assignment if it is an all-underscore vararg
Expand Down
12 changes: 12 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2937,3 +2937,15 @@ end
@test eval(Meta.parse("`a\\\r\nb`")) == `ab`
@test eval(Meta.parse("`a\\\rb`")) == `ab`
end

@testset "slurping into function def" begin
x, f()... = [1, 2, 3]
@test x == 1
@test f() == [2, 3]
# test that call to `Base.rest` is outside the definition of `f`
@test f() === f()

x, f()... = 1, 2, 3
@test x == 1
@test f() == (2, 3)
end

5 comments on commit 5c49a0d

@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(ALL, isdaily = true)

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 5c49a0d Jul 2, 2021

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, 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 benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @christopher-dG

@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. cc @maleadt

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 5c49a0d Jul 8, 2021

Choose a reason for hiding this comment

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

Good work @JeffBezanson! Nobody has ever improved such a long list of benchmarks before all at once :)

Please sign in to comment.