Skip to content

Commit

Permalink
parse 2-arg comparisons as calls instead of comparison exprs
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Mar 18, 2016
1 parent eb82b4f commit 8d76566
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 24 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ Language changes
* `A <: B` is parsed as `Expr(:(<:), :A, :B)` in all cases ([#9503]). This also applies to the
`>:` operator.

* Simple 2-argument comparisons like `A < B` are parsed as calls intead of using the
`:comparison` expression type.

This comment has been minimized.

Copy link
@tonyhffong

tonyhffong Apr 3, 2016

What is the rationale for having 2 separate ast representations for comparison (2arg vs 3+args)?
It broke Lint.jl, so I noticed. I had to duplicate :call with comparison op as special cases of :comparison nodes lint logic. it's okay.

This comment has been minimized.

Copy link
@JeffBezanson

JeffBezanson Apr 3, 2016

Author Member

Sorry about the breaking change. I really think the new version makes a lot more sense though. With 2 arguments, there is nothing special at all about a comparison --- it's just a function call. Only the chained comparison form is special. The purpose of the comparison expression head was not to identify which operators are comparisons, but to identify the special interpretation of x < y < z.

This comment has been minimized.

Copy link
@tonyhffong

tonyhffong Apr 3, 2016

wouldn't it be further lowered to the same thing anyway? I'd imagine that the chained expression would be further lowered into a series of calls with && chaining their results. So there is no penalty of performance or is there?

Command-line option changes
---------------------------

Expand Down
4 changes: 2 additions & 2 deletions base/linalg/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -276,15 +276,15 @@ end

function det(C::Cholesky)
dd = one(eltype(C))
for i in 1:size(C.factors,1) dd *= abs2(C.factors[i,i]) end
for i in 1:size(C.factors,1); dd *= abs2(C.factors[i,i]) end
dd
end

det(C::CholeskyPivoted) = C.rank < size(C.factors, 1) ? real(zero(eltype(C))) : prod(abs2(diag(C.factors)))

function logdet(C::Cholesky)
dd = zero(eltype(C))
for i in 1:size(C.factors,1) dd += log(C.factors[i,i]) end
for i in 1:size(C.factors,1); dd += log(C.factors[i,i]) end
dd + dd # instead of 2.0dd which can change the type
end

Expand Down
6 changes: 3 additions & 3 deletions base/linalg/dense.jl
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ function expm!{T<:BlasFloat}(A::StridedMatrix{T})
LAPACK.gesv!(V-U, X)

if s > 0 # squaring to reverse dividing by power of 2
for t=1:si X *= X end
for t=1:si; X *= X end
end
end

Expand All @@ -264,10 +264,10 @@ function expm!{T<:BlasFloat}(A::StridedMatrix{T})
end

if ilo > 1 # apply lower permutations in reverse order
for j in (ilo-1):-1:1 rcswap!(j, Int(scale[j]), X) end
for j in (ilo-1):-1:1; rcswap!(j, Int(scale[j]), X) end
end
if ihi < n # apply upper permutations in forward order
for j in (ihi+1):n rcswap!(j, Int(scale[j]), X) end
for j in (ihi+1):n; rcswap!(j, Int(scale[j]), X) end
end
X
end
Expand Down
2 changes: 1 addition & 1 deletion base/sparse/cholmod.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1517,7 +1517,7 @@ end
function logdet{Tv<:VTypes}(F::Factor{Tv})
f = unsafe_load(get(F.p))
res = zero(Tv)
for d in diag(F) res += log(abs(d)) end
for d in diag(F); res += log(abs(d)) end
f.is_ll!=0 ? 2res : res
end

Expand Down
4 changes: 2 additions & 2 deletions base/sparse/linalg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import Base.LinAlg: checksquare

# Convert from 1-based to 0-based indices
function decrement!{T<:Integer}(A::AbstractArray{T})
for i in 1:length(A) A[i] -= one(T) end
for i in 1:length(A); A[i] -= one(T) end
A
end
decrement{T<:Integer}(A::AbstractArray{T}) = decrement!(copy(A))

# Convert from 0-based to 1-based indices
function increment!{T<:Integer}(A::AbstractArray{T})
for i in 1:length(A) A[i] += one(T) end
for i in 1:length(A); A[i] += one(T) end
A
end
increment{T<:Integer}(A::AbstractArray{T}) = increment!(copy(A))
Expand Down
2 changes: 1 addition & 1 deletion base/sparse/sparsematrix.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3084,7 +3084,7 @@ spdiagm(B::AbstractVector, d::Number=0) = spdiagm((B,), (d,))
function expandptr{T<:Integer}(V::Vector{T})
if V[1] != 1 throw(ArgumentError("first index must be one")) end
res = similar(V, (Int64(V[end]-1),))
for i in 1:(length(V)-1), j in V[i]:(V[i+1] - 1) res[j] = i end
for i in 1:(length(V)-1), j in V[i]:(V[i+1] - 1); res[j] = i end
res
end

Expand Down
2 changes: 1 addition & 1 deletion base/strings/basic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ getindex(s::AbstractString, i::Integer) = s[Int(i)]
getindex{T<:Integer}(s::AbstractString, r::UnitRange{T}) = s[Int(first(r)):Int(last(r))]
# TODO: handle other ranges with stride ±1 specially?
getindex(s::AbstractString, v::AbstractVector) =
sprint(length(v), io->(for i in v write(io,s[i]) end))
sprint(length(v), io->(for i in v; write(io,s[i]) end))

symbol(s::AbstractString) = symbol(bytestring(s))

Expand Down
5 changes: 5 additions & 0 deletions base/test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ Returns a `Pass` `Result` if it does, a `Fail` `Result` if it is
"""
macro test(ex)
orig_ex = Expr(:quote,ex)
# Normalize comparison operator calls to :comparison expressions
if isa(ex, Expr) && ex.head == :call && length(ex.args)==3 &&
Base.operator_precedence(ex.args[1]) == Base.operator_precedence(:(==))
ex = Expr(:comparison, ex.args[2], ex.args[1], ex.args[3])
end
# If the test is a comparison
if isa(ex, Expr) && ex.head == :comparison
# Generate a temporary for every term in the expression
Expand Down
24 changes: 15 additions & 9 deletions src/julia-parser.scm
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@
(arg2 (cadddr ex)))
(if (or (eq? op '|<:|) (eq? op '|>:|))
`(,op ,arg1 ,arg2)
ex)))
`(call ,op ,arg1 ,arg2))))
(else ex)))))

(define closing-token?
Expand Down Expand Up @@ -1400,14 +1400,20 @@

;; as above, but allows both "i=r" and "i in r"
(define (parse-iteration-spec s)
(let ((r (parse-eq* s)))
(cond ((and (pair? r) (eq? (car r) '=)) r)
((eq? r ':) r)
((and (length= r 4) (eq? (car r) 'comparison)
(or (eq? (caddr r) 'in) (eq? (caddr r) '∈)))
`(= ,(cadr r) ,(cadddr r)))
(else
(error "invalid iteration specification")))))
(let* ((lhs (parse-pipes s))
(t (peek-token s)))
(cond ((memq t '(= in ∈))
(take-token s)
(let* ((rhs (parse-pipes s))
(t (peek-token s)))
#;(if (not (or (closing-token? t) (newline? t)))
;; should be: (error "invalid iteration specification")
(syntax-deprecation s (string "for " (deparse `(= ,lhs ,rhs)) " " t)
(string "for " (deparse `(= ,lhs ,rhs)) "; " t)))
`(= ,lhs ,rhs)))
((and (eq? lhs ':) (closing-token? t))
':)
(else (error "invalid iteration specification")))))

(define (parse-comma-separated-iters s)
(let loop ((ranges '()))
Expand Down
1 change: 0 additions & 1 deletion src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1522,7 +1522,6 @@
(lambda (e)
`(call (top getfield) ,(expand-forms (cadr e)) ,(expand-forms (caddr e))))

'in syntactic-op-to-call
'|<:| syntactic-op-to-call
'|>:| syntactic-op-to-call

Expand Down
8 changes: 4 additions & 4 deletions test/parse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ let
("5.≥x", "5.>=x"),
("5.≤x", "5.<=x")]
ex1 = parse(ex1); ex2 = parse(ex2)
@test ex1.head === :comparison && (ex1.head === ex2.head)
@test ex1.args[1] === 5 && ex2.args[1] === 5
@test is(eval(Main, ex1.args[2]), eval(Main, ex2.args[2]))
@test ex1.head === :call && (ex1.head === ex2.head)
@test ex1.args[2] === 5 && ex2.args[2] === 5
@test is(eval(Main, ex1.args[1]), eval(Main, ex2.args[1]))
@test ex1.args[3] === :x && (ex1.args[3] === ex2.args[3])
end
end
Expand Down Expand Up @@ -291,7 +291,7 @@ for T in (UInt8,UInt16,UInt32,UInt64)
@test_throws OverflowError parse(T,string(big(typemax(T))+1))
end

@test parse("1 == 2|>3") == Expr(:comparison, 1, :(==), Expr(:call, :(|>), 2, 3))
@test parse("1 == 2|>3") == Expr(:call, :(==), 1, Expr(:call, :(|>), 2, 3))

# issue #12501 and pr #12502
parse("""
Expand Down

0 comments on commit 8d76566

Please sign in to comment.