Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow slurping in any position #42902

Merged
merged 19 commits into from
Apr 8, 2022
Merged

allow slurping in any position #42902

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fc075da
RFC: allow slurping in any position
simeonschaub Nov 2, 2021
d4cefef
don't force tail to be static
simeonschaub Jan 10, 2022
4fce905
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Jan 10, 2022
a1ada06
fix tests, add some new ones
simeonschaub Jan 11, 2022
d984b60
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Jan 18, 2022
5ab22cd
add some docs
simeonschaub Jan 20, 2022
9d1e5b7
add NEWS entry
simeonschaub Jan 20, 2022
6c2eedb
fix docs
simeonschaub Jan 20, 2022
29e002e
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Jan 20, 2022
0b438c4
fix whitespace
simeonschaub Jan 20, 2022
3ec586a
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Jan 28, 2022
b14182b
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Feb 12, 2022
a2f1cad
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Apr 6, 2022
0d9d04b
update compat annotations
simeonschaub Apr 6, 2022
34a24c3
remove nonsense assert
simeonschaub Apr 6, 2022
73c8155
fix bad rebase n NEWS [ci skip]
simeonschaub Apr 6, 2022
ae13239
retrigger CI
simeonschaub Apr 6, 2022
e3c1066
explain `destructure-`
simeonschaub Apr 8, 2022
d20ba83
Merge remote-tracking branch 'origin/master' into sds/extended_slurp
simeonschaub Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2672,3 +2672,33 @@ function intersect(v::AbstractVector, r::AbstractRange)
return vectorfilter(T, _shrink_filter!(seen), common)
end
intersect(r::AbstractRange, v::AbstractVector) = intersect(v, r)


_collect_n(itr, ::Val{0}) = error()
_collect_n(itr, ::Val{0}, st) = ((), st)
function _collect_n(itr, ::Val{N}, st...) where {N}
tmp = iterate(itr, st...)
if tmp === nothing
error("Iterator does not contain enough elements for the given variables.")
end
first, st′ = tmp
tail, st′′ = _collect_n(itr, Val(N-1), st′)
return (first, tail...), st′′
end

function split_rest(itr, ::Val{N}, st...) where {N}
if IteratorSize(itr) == IsInfinite()
error("Can't split an infinite iterator in the middle.")
end
last_n, st′ = _collect_n(itr, Val(N), st...)
front = Vector{@default_eltype(itr)}()
while true
tmp = iterate(itr, st′)
tmp === nothing && break
xᵢ, st′ = tmp
push!(front, first(last_n))
last_n = (tail(last_n)..., xᵢ)
end
simeonschaub marked this conversation as resolved.
Show resolved Hide resolved
return front, last_n
end

3 changes: 3 additions & 0 deletions base/tuple.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ rest(a::Array, i::Int=1) = a[i:end]
rest(a::Core.SimpleVector, i::Int=1) = a[i:end]
rest(itr, state...) = Iterators.rest(itr, state...)

split_rest(t::Tuple, ::Val{N}) where {N} = t[1:end-N], t[end-N+1:end]
split_rest(t::Tuple, ::Val{N}, st) where {N} = t[st:end-N], t[end-N+1:end]

# Use dispatch to avoid a branch in first
first(::Tuple{}) = throw(ArgumentError("tuple must be non-empty"))
first(t::Tuple) = t[1]
Expand Down
145 changes: 97 additions & 48 deletions src/julia-syntax.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1459,15 +1459,59 @@
after
(cons R elts)))
((vararg? L)
(if (any vararg? (cdr lhss))
(error "multiple \"...\" on lhs of assignment"))
(if (null? (cdr lhss))
(let ((temp (if (eventually-call? (cadr L)) (gensy) (make-ssavalue))))
`(block ,@(reverse stmts)
(= ,temp (tuple ,@rhss))
,@(reverse after)
(= ,(cadr L) ,temp)
(unnecessary (tuple ,@(reverse elts) (... ,temp)))))
(error (string "invalid \"...\" on non-final assignment location \""
(cadr L) "\""))))
(let ((lhss- (reverse lhss))
(rhss- (reverse rhss))
(lhs-tail '())
(rhs-tail '()))
(define (extract-tail)
(if (not (or (null? lhss-) (null? rhss-)
(vararg? (car lhss-)) (vararg? (car rhss-))))
(begin
(set! lhs-tail (cons (car lhss-) lhs-tail))
(set! rhs-tail (cons (car rhss-) rhs-tail))
(set! lhss- (cdr lhss-))
(set! rhss- (cdr rhss-))
(extract-tail))))
(extract-tail)
(let* ((temp (if (any (lambda (x)
(or (eventually-call? x)
(and (vararg? x) (eventually-call? (cadr x)))))
lhss-)
(gensy)
(make-ssavalue)))
(assigns (make-assignment temp `(tuple ,@(reverse rhss-))))
(assigns (if (symbol? temp)
`((local-def ,temp) ,assigns)
(list assigns)))
(n (length lhss-))
(st (gensy))
(end (list after))
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, would be clearer to make this a mutable variable than a 1-element list.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, it is also passed to destructure- 😭

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, agree it's a little bit awkward, but couldn't think of something more elegant

(assigns (if (and (length= lhss- 1) (vararg? (car lhss-)))
(begin
(set-car! end
(cons `(= ,(cadar lhss-) ,temp) (car end)))
assigns)
(append (if (> n 0)
`(,@assigns (local ,st))
assigns)
(destructure- 1 (reverse lhss-) temp
n st end)))))
(loop lhs-tail
(append (map (lambda (x) (if (vararg? x) (cadr x) x)) lhss-) assigned)
rhs-tail
(append (reverse assigns) stmts)
(car end)
(cons `(... ,temp) elts))))))

((vararg? R)
(let ((temp (make-ssavalue)))
`(block ,@(reverse stmts)
Expand Down Expand Up @@ -2136,6 +2180,50 @@
(cdar lhss))
(unnecessary ,xx))))

(define (destructure- i lhss xx n st end)
simeonschaub marked this conversation as resolved.
Show resolved Hide resolved
(if (null? lhss)
'()
(let* ((lhs (car lhss))
(lhs- (cond ((or (symbol? lhs) (ssavalue? lhs))
lhs)
((vararg? lhs)
(let ((lhs- (cadr lhs)))
(if (or (symbol? lhs-) (ssavalue? lhs-))
lhs
`(|...| ,(if (eventually-call? lhs-)
(gensy)
(make-ssavalue))))))
;; can't use ssavalues if it's a function definition
((eventually-call? lhs) (gensy))
(else (make-ssavalue)))))
(if (and (vararg? lhs) (any vararg? (cdr lhss)))
(error "multiple \"...\" on lhs of assignment"))
(if (not (eq? lhs lhs-))
(if (vararg? lhs)
(set-car! end (cons (expand-forms `(= ,(cadr lhs) ,(cadr lhs-))) (car end)))
(set-car! end (cons (expand-forms `(= ,lhs ,lhs-)) (car end)))))
(if (vararg? lhs-)
(if (= i n)
(if (underscore-symbol? (cadr lhs-))
'()
(list (expand-forms
`(= ,(cadr lhs-) (call (top rest) ,xx ,@(if (eq? i 1) '() `(,st)))))))
(let ((tail (if (eventually-call? lhs) (gensy) (make-ssavalue))))
(cons (expand-forms
(lower-tuple-assignment
(list (cadr lhs-) tail)
`(call (top split_rest) ,xx (call (top Val) ,(- n i))
,@(if (eq? i 1) '() `(,st)))))
(destructure- 1 (cdr lhss) tail (- n i) st end))))
(cons (expand-forms
(lower-tuple-assignment
(if (= i n)
(list lhs-)
(list lhs- st))
`(call (top indexed_iterate)
,xx ,i ,@(if (eq? i 1) '() `(,st)))))
(destructure- (+ i 1) (cdr lhss) xx n st end))))))

(define (expand-tuple-destruct lhss x)
(define (sides-match? l r)
;; l and r either have equal lengths, or r has a trailing ...
Expand All @@ -2152,70 +2240,31 @@
(tuple-to-assignments lhss x))
;; (a, b, ...) = other
(begin
;; like memq, but if last element of lhss is (... sym),
;; check against sym instead
;; like memq, but if lhs is (... sym), check against sym instead
(define (in-lhs? x lhss)
(if (null? lhss)
#f
(let ((l (car lhss)))
(cond ((and (pair? l) (eq? (car l) '|...|))
(if (null? (cdr lhss))
(eq? (cadr l) x)
(error (string "invalid \"...\" on non-final assignment location \""
(cadr l) "\""))))
(eq? (cadr l) x))
((eq? l x) #t)
(else (in-lhs? x (cdr lhss)))))))
;; in-lhs? also checks for invalid syntax, so always call it first
(let* ((xx (cond ((or (and (not (in-lhs? x lhss)) (symbol? x))
(ssavalue? x))
(let* ((xx (cond ((or (ssavalue? x)
(and (symbol? x) (not (in-lhs? x lhss))))
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
(n (if (> n 0)
(let ((l (last lhss)))
(if (and (vararg? l) (underscore-symbol? (cadr l)))
(- n 1)
n))
n))
(st (gensy))
(end '()))
(end (list (list))))
`(block
,@(if (> n 0) `((local ,st)) '())
,@ini
,@(map (lambda (i lhs)
(let ((lhs- (cond ((or (symbol? lhs) (ssavalue? lhs))
lhs)
((vararg? lhs)
(let ((lhs- (cadr lhs)))
(if (or (symbol? lhs-) (ssavalue? lhs-))
lhs
`(|...| ,(if (eventually-call? lhs-)
(gensy)
(make-ssavalue))))))
;; can't use ssavalues if it's a function definition
((eventually-call? lhs) (gensy))
(else (make-ssavalue)))))
(if (not (eq? lhs lhs-))
(if (vararg? lhs)
(set! end (cons (expand-forms `(= ,(cadr lhs) ,(cadr lhs-))) end))
(set! end (cons (expand-forms `(= ,lhs ,lhs-)) end))))
(expand-forms
(if (vararg? lhs-)
`(= ,(cadr lhs-) (call (top rest) ,xx ,@(if (eq? i 0) '() `(,st))))
(lower-tuple-assignment
(if (= i (- n 1))
(list lhs-)
(list lhs- st))
`(call (top indexed_iterate)
,xx ,(+ i 1) ,@(if (eq? i 0) '() `(,st))))))))
(iota n)
lhss)
,@(reverse end)
,@(destructure- 1 lhss xx n st end)
,@(reverse (car end))
(unnecessary ,xx))))))

;; move an assignment into the last statement of a block to keep more statements at top level
Expand Down
42 changes: 40 additions & 2 deletions test/syntax.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2618,8 +2618,6 @@ end
@test x == 1 && y == 2
@test z == (3:5,)

@test Meta.isexpr(Meta.@lower(begin a, b..., c = 1:3 end), :error)
@test Meta.isexpr(Meta.@lower(begin a, b..., c = 1, 2, 3 end), :error)
@test Meta.isexpr(Meta.@lower(begin a, b..., c... = 1, 2, 3 end), :error)

@test_throws BoundsError begin x, y, z... = 1:1 end
Expand Down Expand Up @@ -3088,3 +3086,43 @@ function checkUserAccess(u::User)
return false
end
""")

@testset "slurping in non-final position" begin
res = begin x, y..., z = 1:7 end
@test res == 1:7
@test x == 1
@test y == Vector(2:6)
@test z == 7

res = begin x, y..., z = [1, 2] end
@test res == [1, 2]
@test x == 1
@test y == Int[]
@test z == 2

x, y, z... = 1:7
res = begin y, z..., x = z..., x, y end
@test res == ((3:7)..., 1, 2)
@test y == 3
@test z == ((4:7)..., 1)
@test x == 2

res = begin x, _..., y = 1, 2 end
@test res == (1, 2)
@test x == 1
@test y == 2

res = begin x, y..., z = 1, 2:4, 5 end
@test res == (1, 2:4, 5)
@test x == 1
@test y == (2:4,)
@test z == 5

@test_throws ErrorException begin x, y..., z = 1:1 end
@test_throws BoundsError begin x, y, _..., z = 1, 2 end

last((a..., b)) = b
front((a..., b)) = a
@test last(1:3) == 3
@test front(1:3) == [1, 2]
end