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 lhs of assignment #37410

Merged
merged 4 commits into from
Oct 26, 2020
Merged

allow slurping in lhs of assignment #37410

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b09f69
allow vararg in lhs of assignment
simeonschaub Aug 29, 2020
c6fa3aa
fix `rest(::Tuple, i)`
simeonschaub Oct 23, 2020
d357579
add tests and docs for Base.rest
simeonschaub Oct 24, 2020
a9922d0
add news [ci skip]
simeonschaub Oct 24, 2020
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
8 changes: 8 additions & 0 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2399,3 +2399,11 @@ function hash(A::AbstractArray, h::UInt)

return h
end

# The semantics of `collect` are weird. Better to write our own
Copy link
Member

Choose a reason for hiding this comment

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

How about collect(T, Iterators.rest(a, state))? Then for the 1-argument case Vector{T}(a). Better to avoid repeated push! since it's a bit slow.

Copy link
Member Author

@simeonschaub simeonschaub Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Then for the 1-argument case Vector{T}(a).

That won't work for multi-dimensional arrays, unfortunately:

julia> Vector{Int}([1 2; 3 4])
ERROR: MethodError: no method matching Array{Int64,1}(::Array{Int64,2})
Closest candidates are:
  Array{Int64,1}(::AbstractArray{S,N}) where {T, N, S} at array.jl:562
  Array{Int64,1}() where T at boot.jl:425
  Array{Int64,1}(::UndefInitializer, ::Int64) where T at boot.jl:406
  ...
Stacktrace:
 [1] top-level scope at REPL[25]:1

collect also doesn't always return a Vector and I think it would be weird if Base.rest(::A) and Base.rest(::A, st) returned different types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, note that:

julia> Base.IteratorSize(Iterators.rest([1 2; 3 4], 1))
Base.SizeUnknown()

so collect will end up just using push! anyways.

function rest(a::AbstractArray{T}, state...) where {T}
v = Vector{T}(undef, 0)
# assume only very few items are taken from the front
sizehint!(v, length(a))
return foldl(push!, Iterators.rest(a, state...), init=v)
end
5 changes: 5 additions & 0 deletions base/tuple.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ function indexed_iterate(I, i, state)
x
end

rest(t::Tuple) = t
rest(t::NTuple{N}, i::Int) where {N} = ntuple(x -> getfield(t, x+i-1), N-i+1)
simeonschaub marked this conversation as resolved.
Show resolved Hide resolved
rest(a::Array, i::Int=1) = a[i:end]
rest(itr, state...) = Iterators.rest(itr, state...)

# Use dispatch to avoid a branch in first
first(::Tuple{}) = throw(ArgumentError("tuple must be non-empty"))
first(t::Tuple) = t[1]
Expand Down
71 changes: 50 additions & 21 deletions src/julia-syntax.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,8 @@
,@(reverse after)
(unnecessary (tuple ,@(reverse elts))))
(let ((L (car lhss))
(R (car rhss)))
;; rhss can be null iff L is a vararg
(R (if (null? rhss) '() (car rhss))))
(cond ((and (symbol-like? L)
(or (not (pair? R)) (quoted? R) (equal? R '(null)))
;; overwrite var immediately if it doesn't occur elsewhere
Expand All @@ -1442,6 +1443,16 @@
(cons (make-assignment L R) stmts)
after
(cons R elts)))
((vararg? L)
(if (null? (cdr lhss))
(let ((temp (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) "\""))))
((vararg? R)
(let ((temp (make-ssavalue)))
`(block ,@(reverse stmts)
Expand Down Expand Up @@ -2066,6 +2077,7 @@
(define (sides-match? l r)
;; l and r either have equal lengths, or r has a trailing ...
(cond ((null? l) (null? r))
((vararg? (car l)) #t)
((null? r) #f)
((vararg? (car r)) (null? (cdr r)))
(else (sides-match? (cdr l) (cdr r)))))
Expand All @@ -2075,26 +2087,43 @@
(expand-forms
(tuple-to-assignments lhss x))
;; (a, b, ...) = other
(let* ((xx (if (or (and (symbol? x) (not (memq x lhss)))
(ssavalue? x))
x (make-ssavalue)))
(ini (if (eq? x xx) '() (list (sink-assignment xx (expand-forms x)))))
(n (length lhss))
(st (gensy)))
`(block
(local ,st)
,@ini
,.(map (lambda (i lhs)
(expand-forms
(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)
(unnecessary ,xx))))))
(begin
;; like memq, but if last element of lhss 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? 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)))
(ini (if (eq? x xx) '() (list (sink-assignment xx (expand-forms x)))))
(n (length lhss))
(st (gensy)))
`(block
(local ,st)
,@ini
,.(map (lambda (i lhs)
(expand-forms
(if (and (pair? lhs) (eq? (car 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)))))))
simeonschaub marked this conversation as resolved.
Show resolved Hide resolved
(iota n)
lhss)
(unnecessary ,xx)))))))
((typed_hcat)
(error "invalid spacing in left side of indexed assignment"))
((typed_vcat)
Expand Down
51 changes: 51 additions & 0 deletions test/syntax.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2493,3 +2493,54 @@ end

# PR #37973
@test Meta.parse("1¦2⌿3") == Expr(:call, :¦, 1, Expr(:call, :⌿, 2, 3))

@testset "slurp in assignments" begin
res = begin x, y, z... = 1:7 end
@test res == 1:7
@test x == 1 && y == 2
@test z == Vector(3:7)

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

x = 1
res = begin x..., = x end
@test res == 1
@test x == 1

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
@test x == ((5:7)..., 1, 2)

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

res = begin x, y... = 1 end
@test res == 1
@test x == 1
@test y == Iterators.rest(1, nothing)

res = begin x, y, z... = 1, 2, 3:5 end
@test res == (1, 2, 3:5)
@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
@test_throws BoundsError begin x, y, _, z... = 1, 2 end

car((a, d...)) = a
cdr((a, d...)) = d
@test car(1:3) == 1
@test cdr(1:3) == [2, 3]
end