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

Jump to bottom

break-with-value and for/else implementation #23260

Closed
wants to merge 23 commits into from
Closed

break-with-value and for/else implementation #23260

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
976ff84
Parse + emit code for for/else and break-with-value
tkluck Aug 14, 2017
d8b2553
Implement findnext() in terms of for/else and break-with-value
tkluck Aug 14, 2017
8a8d4cc
Fix break-with-value in case the value expression is a symbol
tkluck Aug 15, 2017
3abd4d9
Simplify break-with-value implementation: don't distinguish break-blo…
tkluck Aug 15, 2017
ed6ff28
Document for/else and break-with-value
tkluck Aug 16, 2017
24c64f4
Fix error message from incomplete for loop
tkluck Aug 16, 2017
f15be30
Avoid allocating a value variable when break-block is in tail position
tkluck Aug 16, 2017
a67bc9f
Remove unnecessary if relating to break-labels contents
tkluck Aug 16, 2017
e0697d1
break-with-value: add elsebody to a few recursive expression traversals
tkluck Aug 17, 2017
aa48222
break-with-value: also parse 'else'-block for while loops
tkluck Aug 19, 2017
29d98ab
More consistent indentation in julia-parser.scm
tkluck Aug 19, 2017
b801f49
break-with-value: don't emit 'nothing' as the break value from the pa…
tkluck Aug 19, 2017
bad3c35
Revert "Implement findnext() in terms of for/else and break-with-value"
tkluck Jan 30, 2018
7c5f378
Merge remote-tracking branch 'origin/master' into break-with-value
tkluck Jan 30, 2018
98d5dd7
Test execution order of break-with-value inside a try/finally block
tkluck Jan 31, 2018
9af9bc2
Merge branch 'master' into break-with-value
tkluck Mar 11, 2018
fe10430
Merge remote-tracking branch 'origin/master' into break-with-value
tkluck Apr 6, 2018
742576a
Merge branch 'master' into break-with-value
tkluck Jun 12, 2018
e0a42c6
break-with-value doctest fixes (whitespace, trivial output)
tkluck Jun 16, 2018
ef077f7
Merge remote-tracking branch 'origin/master' into break-with-value
tkluck Aug 11, 2018
c9934e9
Merge remote-tracking branch 'origin/master' into break-with-value
tkluck Dec 23, 2018
e081226
Merge remote-tracking branch 'origin/master' into break-with-value
tkluck Jan 16, 2019
0aa316a
Merge remote-tracking branch 'origin/master' into break-with-value
tkluck Feb 17, 2019
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
20 changes: 12 additions & 8 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,10 +1251,11 @@ julia> findnext(A,3)
function findnext(A, start::Integer)
for i = start:length(A)
if A[i] != 0
return i
Copy link
Member

Choose a reason for hiding this comment

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

Not related to the feature itself, but I find explicit returns easier to reason about.

Copy link
Member

Choose a reason for hiding this comment

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

They are, but sometimes you don't want to factor a single loop into a function just so that you can use a return to get a value out. This basically lets you have that without a function.

Copy link
Contributor

@yuyichao yuyichao Aug 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I do like the for else feature but isn't v = for ...; break i; else j end always equivalent to for ...; v = i; break; else v = j end (with the correct scope of v)?
The latter seems much easier to understand than the former. It is similar to the return value of if else but in that case at least the return value is easier to find.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @KristofferC that the findnext example isn't the best use-case for this feature because a return suffices; I just included it for the sake of show-casing the working patch.

@yuyichao , yes that is indeed equivalent, and it's probably a matter of taste which is easier to understand.

IMHO, in a language where there is no distinction between statements and expressions, it is a bit wasteful and arguably unexpected (e.g. when compared to your if/else example) not to be able to specify a value for a loop expression. That's why I personally like this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Afterthought: it's also a bit more 'functional' because it's easier to reason about whether the resulting variable v is mutable or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's why I do like the for else and my example above includes the else block.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, I misread that. Got it.

Copy link
Member

Choose a reason for hiding this comment

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

I do agree, however, that these cases are clearer with return; but they do serve as an in-situ test of the new syntax, which is useful for a WIP pull request.

Copy link
Member

@rfourquet rfourquet Aug 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

A nice example is intersect, which can be improved into:

function intersect(s::Set, sets...)
    i = similar(s)
    for x in s
        for t in sets
            !in(x, t) && break false
        else true
        end && push!(i, x)
    end
    return i
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great example @rfourquet ! It could also be

for x in s
    for t in sets
        !in(x, t) && break
    else
        push!(i, x)
    end
end

which doesn't use break-with-value, but does use the else feature.

I personally like options with all or with comprehensions best, but that's a matter of taste and may not give the same performance (yet).

break i
end
else
0
end
return 0
end

"""
Expand Down Expand Up @@ -1301,10 +1302,11 @@ julia> findnext(A,4,3)
function findnext(A, v, start::Integer)
for i = start:length(A)
if A[i] == v
return i
break i
end
else
0
end
return 0
end
"""
findfirst(A, v)
Expand Down Expand Up @@ -1350,10 +1352,11 @@ julia> findnext(isodd, A, 2)
function findnext(testf::Function, A, start::Integer)
for i = start:length(A)
if testf(A[i])
return i
break i
end
else
0
end
return 0
end

"""
Expand Down Expand Up @@ -1399,9 +1402,10 @@ julia> findprev(A,1)
"""
function findprev(A, start::Integer)
for i = start:-1:1
A[i] != 0 && return i
A[i] != 0 && break i
else
0
end
return 0
end

"""
Expand Down
4 changes: 3 additions & 1 deletion src/jlfrontend.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
tab)))
((lambda) tab)
((local) tab)
((break-block) (find-possible-globals- (caddr e) tab))
((break-block) (find-possible-globals- (caddr e) tab))
;; TODO: elsebody in (cadddr e)
((break-block-with-value) (find-possible-globals- (caddr e) tab))
((module toplevel) '())
(else
(for-each (lambda (x) (find-possible-globals- x tab))
Expand Down
31 changes: 24 additions & 7 deletions src/julia-parser.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,11 +1286,22 @@
(expect-end s word)))
((for)
(let* ((ranges (parse-comma-separated-iters s))
(body (parse-block s)))
(expect-end s word)
`(for ,(if (length= ranges 1) (car ranges) (cons 'block ranges))
,body)))

(body (parse-block s))
(nxt (take-token s)))
(case nxt
((end)
`(for ,(if (length= ranges 1) (car ranges) (cons 'block ranges))
,body))
((else)
(let* ((elsebody (parse-block s)))
(expect-end s word)
`(for ,(if (length= ranges 1) (car ranges) (cons 'block ranges))
,body
,elsebody)))
(otherwise
(error (string "\"" word "\" at "
current-filename ":" input-port-line
" expected \"end\", got \"" nxt "\""))))))
((if elseif)
(if (newline? (peek-token s))
(error (string "missing condition in \"if\" at " current-filename
Expand Down Expand Up @@ -1481,14 +1492,20 @@
(if (or (eqv? t #\newline) (closing-token? t))
(list 'return '(null))
(list 'return (parse-eq s)))))
((break continue)
((continue)
(let ((t (peek-token s)))
(if (or (eof-object? t)
(and (eq? t 'end) (not end-symbol))
(memv t '(#\newline #\; #\) :)))
(list word)
(error (string "unexpected \"" t "\" after " word)))))

((break)
(let ((t (peek-token s)))
(if (or (eof-object? t)
(and (eq? t 'end) (not end-symbol))
(memv t '(#\newline #\; #\) :)))
(list word '(null))
(list word (parse-eq s)))))
((module baremodule)
(let* ((name (parse-unary-prefix s))
(loc (line-number-node s))
Expand Down
57 changes: 46 additions & 11 deletions src/julia-syntax.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,7 @@
(if ,g ,g
,(loop (cdr tail)))))))))))

(define (expand-for while lhs X body)
(define (expand-for while lhs X elsebody body)
;; (for (= lhs X) body)
(let ((coll (make-ssavalue))
(state (gensy)))
Expand All @@ -1685,7 +1685,8 @@
#;,@(map (lambda (v) `(local ,v)) (lhs-vars lhs))
,(lower-tuple-assignment (list lhs state)
`(call (top next) ,coll ,state))
,body))))))))
,body))
,elsebody))))))

;; convert an operator parsed as (op a b) to (call op a b)
(define (syntactic-op-to-call e)
Expand Down Expand Up @@ -2224,10 +2225,13 @@
'while
(lambda (e)
`(scope-block
(break-block loop-exit
(break-block-with-value loop-exit
(_while ,(expand-forms (cadr e))
(break-block loop-cont
,(expand-forms (caddr e)))))))
,(expand-forms (caddr e))))
,(if (length> e 3)
(expand-forms (cadddr e))
'(null)))))

'inner-while
(lambda (e)
Expand All @@ -2238,9 +2242,9 @@

'break
(lambda (e)
(if (pair? (cdr e))
(if (symbol? (cadr e))
e
'(break loop-exit)))
`(break loop-exit ,(cadr e))))

'continue (lambda (e) '(break loop-cont))

Expand All @@ -2253,6 +2257,9 @@
(expand-for (if first 'while 'inner-while)
(cadr (car ranges))
(caddr (car ranges))
(if (and (length> e 3) first)
(cadddr e) ;; elsebody
'(null))
(if (null? (cdr ranges))
(caddr e) ;; body
(nest (cdr ranges) #f)))))
Expand Down Expand Up @@ -2524,6 +2531,8 @@
((or (not (pair? e)) (quoted? e)) tab)
((memq (car e) '(lambda scope-block module toplevel)) tab)
((eq? (car e) 'break-block) (unbound-vars (caddr e) bound tab))
((eq? (car e) 'break-block-with-value) (unbound-vars (caddr e) bound tab))
;; TODO: elsebody in (cadddr e)
((eq? (car e) 'with-static-parameters) (unbound-vars (cadr e) bound tab))
(else (for-each (lambda (x) (unbound-vars x bound tab))
(cdr e))
Expand Down Expand Up @@ -2620,8 +2629,12 @@
((eq? (car e) 'module)
(error "module expression not at top level"))
((eq? (car e) 'break-block)
`(break-block ,(cadr e) ;; ignore type symbol of break-block expression
`(,(car e) ,(cadr e) ;; ignore type symbol of break-block expression
,(resolve-scopes- (caddr e) env outerglobals implicitglobals lam renames #f))) ;; body of break-block expression
((eq? (car e) 'break-block-with-value)
`(,(car e) ,(cadr e) ;; ignore type symbol of break-block expression
,(resolve-scopes- (caddr e) env outerglobals implicitglobals lam renames #f) ;; body of break-block expression
,(resolve-scopes- (cadddr e) env outerglobals implicitglobals lam renames #f))) ;; elsebody of break-block-with-value expression
((eq? (car e) 'with-static-parameters)
`(with-static-parameters ;; ignore list of sparams in break-block expression
,(resolve-scopes- (cadr e) env outerglobals implicitglobals lam renames #f)
Expand All @@ -2646,6 +2659,8 @@
((symbol? e) (put! tab e #t))
((and (pair? e) (eq? (car e) 'outerref)) (put! tab (cadr e) #t))
((and (pair? e) (eq? (car e) 'break-block)) (free-vars- (caddr e) tab))
;; TODO: elsebody in (cadddr e)
((and (pair? e) (eq? (car e) 'break-block-with-value)) (free-vars- (caddr e) tab))
((and (pair? e) (eq? (car e) 'with-static-parameters)) (free-vars- (cadr e) tab))
((or (atom? e) (quoted? e)) tab)
((eq? (car e) 'lambda)
Expand Down Expand Up @@ -3553,14 +3568,34 @@ f(x) = yt(x)
value #f)
(mark-label endl)))
(if value (compile '(null) break-labels value tail)))
((break-block-with-value)
(let ((endl (make-label))
(valvar (if (or value tail) (new-mutable-var) #f)))
(compile (caddr e)
(cons (list (cadr e) endl handler-level valvar)
break-labels)
value #f)
(if (length> e 3)
(let ((elseval (compile (cadddr e) break-labels valvar #f)))
(if valvar (emit `(= ,valvar ,elseval))))
(let ((elseval (compile '(null) break-labels valvar #f)))
(if (and valvar elseval) (emit `(= ,valvar ,elseval)))))
(mark-label endl)
(if tail (emit-return valvar) valvar)))
((break)
(let ((labl (assq (cadr e) break-labels)))
(if (not labl)
(error "break or continue outside loop")
(begin
(if (> handler-level (caddr labl))
(emit `(leave ,(- handler-level (caddr labl)))))
(emit `(goto ,(cadr labl)))))))
(let ((endl (cadr labl))
(hlevel (caddr labl))
(valtarget (if (length> labl 3) (cadddr labl) #f))
(valexpr (if (length> e 2) (caddr e) '(null))))
(let ((res (compile valexpr break-labels valtarget #f)))
(if valtarget
(emit `(= ,valtarget ,res))))
(if (> handler-level hlevel)
(emit `(leave ,(- handler-level hlevel))))
(emit `(goto ,endl))))))
((label symboliclabel)
(if (eq? (car e) 'symboliclabel)
(if (has? label-level (cadr e))
Expand Down