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.

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

RFC: move => to precedence level of |> #12285

Merged
merged 1 commit into from
Jul 24, 2015
Merged

Conversation

JeffBezanson
Copy link
Member

Before:

julia> 1=>2 in Dict()
1=>false

after:

julia> 1=>2 in Dict()
false

My reasoning is that the precedence of => needs to be higher than in or ==, but lower than +. It should also be lower than :, since you can't make ranges of Pairs. That puts it directly in the precedence level of |>, which also makes sense since |> and => look vaguely similar.

@vtjnash
Copy link
Member

vtjnash commented Jul 24, 2015

makes sense. while => has an equals sign in it, it isn't actually an assignment.

@stevengj
Copy link
Member

👍

@stevengj
Copy link
Member

Wouldn't similar arguments apply to prec-arrow? Or, no, those should be like ->, right (i.e. lower than == or in)? It depends on whether we think of prec-arrow as being for function-like operators (->) or relation-like operators (=>)?

@JeffBezanson
Copy link
Member Author

If you think of an arrow as "implies", then it makes sense for it to be lower than ==.

JeffBezanson added a commit that referenced this pull request Jul 24, 2015
RFC: move `=>` to precedence level of `|>`
@JeffBezanson JeffBezanson merged commit c9cf421 into master Jul 24, 2015
@tkelman tkelman deleted the jb/pairprecedence branch July 25, 2015 01:21
@tkelman
Copy link
Contributor

tkelman commented Jul 25, 2015

This breaks some packages' uses of FactCheck, hopefully all fixable with enough parentheses. I strongly encourage anyone who cares about the health of the package ecosystem leading up to 0.4 to install Vagrant and do some local PackageEvaluator runs while Iain's system is out of commission.

tkelman added a commit to jump-dev/JuMP.jl that referenced this pull request Jul 25, 2015
precedence of `in` vs `=>` changed in JuliaLang/julia#12285
tkelman added a commit to jump-dev/JuMP.jl that referenced this pull request Jul 25, 2015
precedence of `in` vs `=>` changed in JuliaLang/julia#12285

parens in test/model.jl

and sdp

I should be running this locally but I'm not, can squash when done

missed a comparison in sdp
@tkelman tkelman mentioned this pull request Jul 26, 2015
@IainNZ
Copy link
Member

IainNZ commented Jul 27, 2015

Kind of funny, this is probably the only really breaking change that seems to have happened in the last couple of weeks, just because of the FactCheck thing. Its not too many packages though, and an easy enough thing to fix.

tkelman added a commit to jump-dev/JuMP.jl that referenced this pull request Jul 27, 2015
precedence of `in` vs `=>` changed in JuliaLang/julia#12285

parens in test/model.jl

and sdp

I should be running this locally but I'm not, can squash when done

missed a comparison in sdp
jakebolewski added a commit to JuliaAttic/FactCheck.jl that referenced this pull request Jul 28, 2015
This gets around precedence issues of `=>` due to JuliaLang/julia#12285

closes #51
jakebolewski added a commit to JuliaAttic/FactCheck.jl that referenced this pull request Jul 28, 2015
This gets around precedence issues of `=>` due to JuliaLang/julia#12285

closes #51
jakebolewski added a commit to JuliaAttic/FactCheck.jl that referenced this pull request Jul 28, 2015
This gets around precedence issues of `=>` due to JuliaLang/julia#12285

closes #51
@rened
Copy link
Member

rened commented Aug 4, 2015

I have to say I rarely use the 1=>2 in Dict() idiom, but use the following quite a bit more often:

Dict(:key => a == 1, :key2 => b == "hi" || b == "hey")

The precedence change makes it fail now:

Dict(:key => a == 1)  
# equals
Dict(=>(:key, a) == 1)  
# equals 
Dict(false)

(sidenote, this currently throws a BoundsError, see #12451)

Of course, the following works but I always feel an itch to remove the "unnecessary" parens:

Dict(:key => (a == 1), :key2 => (b == "hi" || b == "hey"))

Personally, I'd prefer to see the behavior reverted to what is was before this commit, as I believe it is more widely used.

@JeffBezanson
Copy link
Member Author

JeffBezanson commented Aug 4, 2015 via email

@stevengj
Copy link
Member

stevengj commented Aug 4, 2015

I don't have strong feeling either way, but I can't find any a => b == c or similar expressions in the current packages.

@mdcfrancis
Copy link

This breaks the precedence of the ? operator as well, it is a common idiom to use a condition when assigning to the RHS of a pair.

#12948

Notably this is inconsistent with other languages

x = { a : true?true:false }
Object {a: true}

@stevengj
Copy link
Member

stevengj commented Sep 4, 2015

Yeah, I have to admit this change has not grown on me. => seems to be used far more often in assignment-like contexts than in in contexts.

@nalimilan
Copy link
Member

@mdcfrancis Could you be more specific about "inconsistent with other languages"? Which ones?

@mdcfrancis
Copy link

@nalimilan - that's a JavaScript snippet.

@mdcfrancis
Copy link

I don't think I've ever used

( :a=>1 ) in Dict( :a=>2 )

as I would commonly be checking based on the key and would use haskey. It is rare that I would be checking for value equality as well.

@ScottPJones
Copy link
Contributor

I also think this was a step backwards.
=> seems like it should be higher than =, but lower than ==, ?:, etc.

Other languages? Well, ':' is not an operator in other languages, but the way => is treated now in Julia is inconsistent with probably most languages that have a table/dict/associative array constructor syntax, such as: Lua, Python, JavaScript.

@JeffBezanson
Copy link
Member Author

Ok I see the case for reverting this. I'm not sure we want to add a new precedence level between ? and = though.

@ScottPJones
Copy link
Contributor

I think you really should though, for this, so that assignment of a Pair is handled well, unless you feel that that people really need to use () around the pair.

a = :b => "string"
x = :b => cond ? "foo" : "bar"

@JeffBezanson
Copy link
Member Author

The = precedence level binds right to left, so that's parsed as (a = (:b => "string")).

@ScottPJones
Copy link
Contributor

Yes, in those examples, it works out fine, but I do think it's better to have it explicitly higher precedence than assignment, as it isn't really an assignment.
Is there a downside to adding a new precedence level?

@JeffBezanson
Copy link
Member Author

Well there are already about 16 of them, and it's pretty confusing. Also if it has strictly higher precedence then a = b => c = d is a = (b => c) = d. That means to define a method of => returning d, then the => function is assigned to a. That's probably unexpected. However in these cases one is tempted to require parentheses.

@ScottPJones
Copy link
Contributor

OK, so you'd just add it to the assignment precedence list in julia-parser.scm? Telling people to use () in general is a good thing also 😀 (I've seen too many bugs from developers using multiple languages with different precedence levels - my fix, always use ()!)

@JeffBezanson
Copy link
Member Author

For now I'm just going to revert this. I also realized that => should be right-associative since it is "arrowlike", and this change broke that.

@mdcfrancis
Copy link

Thanks Jeff, much appreciated.

@StephenVavasis
Copy link
Contributor

Sorry to reopen this topic, but something seems to be wrong with => precedence in 0.4-rc1 and 0.5. Observe (from rc1):

julia> d = Dict(1=>2,3=>4)
Dict{Int64,Int64} with 2 entries:
3 => 4
1 => 2

julia> 1=>2,3=>4
1=>((2,3)=>4)

The second result is certainly unexpected, especially given the first result

@JeffBezanson
Copy link
Member Author

Yeah well, too late now.

@ScottPJones
Copy link
Contributor

Is the problem here really the parsing of a,b as a tuple, instead of requiring parenthesis, i.e. (a,b)?

@JeffBezanson
Copy link
Member Author

The precedence of => is fun and games compared to removing a, b = x syntax. Major league breakage.

Implication is the lowest precedence logical operator, so the arrows (which are not much used, so not a huge deal so far) should really be moved below ||. I will definitely do that for 0.5. It would make sense for => to be part of that set, but folks want to use ? as an argument to =>, so it seems the only possible choice is @ScottPJones ' suggestion of inserting => by itself between assignment and ?. I believe that would also fix the 1=>2,3=>4 case.

@tkelman
Copy link
Contributor

tkelman commented Sep 11, 2015

The most noticeable thing the => precedence change broke was FactCheck, which changed to use one of the arrows. Changing the precedence of the arrows would probably necessitate finding yet another operator to use for FactCheck.

@JeffBezanson
Copy link
Member Author

Which precedence change broke FactCheck: the original change or the more recent revert of it?

@tkelman
Copy link
Contributor

tkelman commented Sep 11, 2015

The original one. So it should be safe now to go back on that deprecation where FactCheck went from => to -->, but any future changes to whichever operator FactCheck is recommending should be done with some care and/or warning.

edit: also possible that the improvements to Base.Test will make FactCheck less necessary

@JeffBezanson
Copy link
Member Author

We should be ok; => will not move far from where it is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants