-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
Changes from all commits
fc075da
d4cefef
4fce905
a1ada06
d984b60
5ab22cd
9d1e5b7
6c2eedb
29e002e
0b438c4
3ec586a
b14182b
a2f1cad
0d9d04b
34a24c3
73c8155
ae13239
e3c1066
d20ba83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1506,15 +1506,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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I see, it is also passed to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -2187,6 +2231,59 @@ | |
lhss) | ||
(unnecessary ,xx)))) | ||
|
||
;; implement tuple destructuring, possibly with slurping | ||
;; | ||
;; `i`: index of the current lhs arg | ||
;; `lhss`: remaining lhs args | ||
;; `xx`: the rhs, already either an ssavalue or something simple | ||
;; `st`: empty list if i=1, otherwise contains the iteration state | ||
;; `n`: total nr of lhs args | ||
;; `end`: car collects statements to be executed afterwards. | ||
;; In general, actual assignments should only happen after | ||
;; the whole iterater is desctructured (https://github.com/JuliaLang/julia/issues/40574) | ||
(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 ,(- 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 ... | ||
|
@@ -2203,64 +2300,26 @@ | |
(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 (maybe-ssavalue lhss x in-lhs?)) | ||
(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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to use either indexing or SubString for both values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my reasoning was that the type of
last_n
doesn't really matter as long as it iterates correctly, so we might as well avoid the extra allocation. I guess we could makefront
aSubString
as well, but that might be wasteful for small string ifn
is not much smaller than the string length, since we can't free the original string