Skip to content

Commit

Permalink
fix #43960, evaluation order of splat inside ref (#44024)
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson authored Feb 7, 2022
1 parent 48b7d49 commit 546a774
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 25 deletions.
48 changes: 23 additions & 25 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -130,31 +130,29 @@
;; returns (values index-list stmts) where stmts are statements that need
;; to execute first.
(define (process-indices a i)
(let loop ((lst i)
(n 1)
(stmts '())
(tuples '())
(ret '()))
(if (null? lst)
(values (reverse ret) (reverse stmts))
(let ((idx (car lst))
(last (null? (cdr lst))))
(if (and (pair? idx) (eq? (car idx) '...))
(if (symbol-like? (cadr idx))
(loop (cdr lst) (+ n 1)
stmts
(cons (cadr idx) tuples)
(cons `(... ,(replace-beginend (cadr idx) a n tuples last))
ret))
(let ((g (make-ssavalue)))
(loop (cdr lst) (+ n 1)
(cons `(= ,g ,(replace-beginend (cadr idx) a n tuples last))
stmts)
(cons g tuples)
(cons `(... ,g) ret))))
(loop (cdr lst) (+ n 1)
stmts tuples
(cons (replace-beginend idx a n tuples last) ret)))))))
(let ((has-va? (any vararg? i)))
(let loop ((lst i)
(n 1)
(stmts '())
(tuples '())
(ret '()))
(if (null? lst)
(values (reverse ret) (reverse stmts))
(let* ((idx0 (car lst))
(idx (if (vararg? idx0) (cadr idx0) idx0))
(last (null? (cdr lst)))
(replaced (replace-beginend idx a n tuples last))
(idx (if (or (not has-va?) (simple-atom? replaced)) replaced (make-ssavalue))))
(loop (cdr lst) (+ n 1)
(if (eq? idx replaced)
stmts
(cons `(= ,idx ,replaced)
stmts))
(if (vararg? idx0) (cons idx tuples) tuples)
(cons (if (vararg? idx0)
`(... ,idx)
idx)
ret)))))))

;; GF method does not need to keep decl expressions on lambda args
;; except for rest arg
Expand Down
8 changes: 8 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,14 @@ end
@test [(0,0)... 1] == [0 0 1]
@test Float32[(0,0)... 1] == Float32[0 0 1]

# issue #43960, evaluation order of splatting in `ref`
let a = [], b = [4,3,2,1]
f() = (push!(a, 1); 2)
g() = (push!(a, 2); ())
@test b[f(), g()...] == 3
@test a == [1,2]
end

@testset "raw_str macro" begin
@test raw"$" == "\$"
@test raw"\n" == "\\n"
Expand Down

4 comments on commit 546a774

@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)

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

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 546a774 Feb 8, 2022

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.

Please sign in to comment.