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

deprecate special "inner constructor" syntax #20308

Merged
merged 1 commit into from
Feb 7, 2017
Merged

Conversation

JeffBezanson
Copy link
Member

No description provided.

@ararslan ararslan added deprecation This change introduces or involves a deprecation types and dispatch Types, subtyping and method dispatch labels Jan 29, 2017
@tkelman
Copy link
Contributor

tkelman commented Jan 29, 2017

foo{T}(x) where T = rhs is always going to read to my (and I suspect more than a few others') mental parsers as if it's talking about T = rhs. Can we at least add parens to one liner cases of this for single params in base even when not required? foo{T}(x) where (T) = rhs at least looks like you might be finished talking about T when you get to the =

@JeffBezanson
Copy link
Member Author

There are a few syntax proposals out there. I think some options are

f(x::T) where {T} = ...
f(x::T){T} = ...
f(x::T) where T<:Any = ... (making <:Any either required or strongly encouraged)

@martinholters
Copy link
Member

Is there a nice way to write code that does not hit a deprecation in 0.6, but still works with 0.5 and possibly even 0.4?
The following works on 0.5 and does not produce a deprecation warning:

type Foo{T}
    x::T
    (::Type{Foo{T}}){T}(x::T) = new{T}(x)
end

It does not work on 0.4, though. It parses, however, so is it worth having

type Foo{T}
    x::T
    @compat (::Type{Foo{T}}){T}(x::T) = new{T}(x)
end

(or another macro in Compat) do the right thing? Obviously, supporting the 0.6 syntax would be much better, but as the where did not parse until recently, that's probably not an option.

@tkelman
Copy link
Contributor

tkelman commented Jan 30, 2017

@compat should already be able to translate 0.5-style call overloading in a way 0.4 can handle, did you try it and have problems in some contexts? Maybe since new is a bit special inside type definition blocks that might be handled differently.

@martinholters
Copy link
Member

Ah, of course, Compat already does this. I less convoluted way of doing this would be nice, but looks like this is indeed usable. Sorry for the noise, then.

@KristofferC
Copy link
Member

What's the drawback of f(x::T){T} compared to f(x::T) where {T}. Seems nice to avoid the where.

@JeffBezanson
Copy link
Member Author

That does seem pretty good for short-form definitions. where takes up significant space in a 1-liner.

@yuyichao
Copy link
Contributor

Not sure if anyone have pointed it out but f(x::T){T} = ... parses on 0.4 and only error in lowering so it can be handled in packages / Compat with macros.

@JeffBezanson
Copy link
Member Author

Another related issue:

function f(x::T)::T where T

currently parses as f(x::T)::(T where T), since it is useful to be able to write x::Array{T} where T in other contexts, e.g. arguments. (cc @andyferris ) We could change this parsing after a function keyword, but I don't think it can be done for short-form definitions. Short-form might look like

f(x::T){T}::T = ...

which is pretty weird. I think we'd have to allow

f(x::T)::T {T}

but that introduces a space sensitivity.

@mbauman
Copy link
Member

mbauman commented Jan 30, 2017

It doesn't seem unreasonable to discourage (or maybe even disallow) method type assertions within short form definitions. If you want to use a method type assertion, you can use the long form… where that special parsing does seem to make sense.

@JeffBezanson JeffBezanson added this to the 0.6.0 milestone Jan 30, 2017
@andyferris
Copy link
Member

andyferris commented Jan 30, 2017

The where belongs to the tuple. For example, imagine an unnamed function (where I add a comma to the argument tuple)

function (x::T,) where T
    ...
end

There has always been an analogy between tuples and function arguments, and the where is there to discuss the tuple/argument types, not the output. I would argue the only way add a method type assertion to the above is like this:

function ((x::T,) where T)::T
    ...
end

(you could wrap the function in brackets too, not sure if that is worse) or perhaps for named function something like

function (f(x::T) where T)::T
    ...
end

So IMO Jeff's first example should be

(f(x::T) where T)::T = ...

Would that parse?

EDIT: Sorry, I got muddled in the last code snippet, fixed.

@andyferris
Copy link
Member

I've been thinking - another way to perhaps make this more obvious, as well as the case f(x::T) where T = T, would be to replace the = with -> for short form functions.

I'm sure this was brought up once before, but I can't find it right now. Consider the four ways of defining named/anoymous short/long functions in Julia:

f(x,y) = x + y
(x,y) -> x + y
function f(x,y); x + y; end
function (x,y); x + y; end

It is the first one which is the odd-one out (and we can't put = in the second). We could have

f(x::T) where T -> T

which seems more like a function than some T = T expression. Using something other than = might make it easier to parse the same with method output type assertions syntax, though I'm not sure.

@StefanKarpinski
Copy link
Member

The snippet f(x::T) where T -> T looks to me like it means f(x::T) where (T -> T), which is nonsense, of course. The idea of requiring long form function syntax for return type assertions seems ok to me since there's not really much point in short form definitions with return types.

@andyferris
Copy link
Member

looks to me like it means f(x::T) where (T -> T), which is nonsense, of course.

True. I guess we have the same problem with the current syntax looking like f(x::T) where (T = T). Personally, I think I'll be using f(x::T where T) = T, to avoid this.

@JeffBezanson
Copy link
Member Author

f(x::T where T) = T

That doesn't mean the same thing.

(f(x::T) where T)::T

I find this too weird --- the last T refers to a T that is introduced inside its expression. To me that just goes against too much intuition about how scope works.

@andyferris
Copy link
Member

That doesn't mean the same thing.

Oh right! Oops, I assumed the T variable would be visible outside... so apart from allowing triangular dispatch, this is the difference between having the where inside and outside the brackets... @JeffBezanson this kind of makes what I was asking for earlier a bit less useful, but I can understand why this is so.

Carry on...

@andyferris
Copy link
Member

the last T refers to a T that is introduced inside its expression.

To me, it seemed similar to (f(x::T) where T) = ... where T is in scope in the .... Variables introduced inside those brackets are visible outside the brackets, at least in some contexts.

But yes, that syntax does look weird.

@quinnj
Copy link
Member

quinnj commented Jan 31, 2017

This is somewhat continuing the derailing of the PR, but could we make the return type more similar to other languages and put it before the function name? i.e.

# long-form
function Void f(x, y)

end

# long-form w/ parameters
function T f(x::T, y::T) where T

end

# short-form
Void f(x,y) = ...

# short-form
T f(::T, y::T) where (T) = ...

This would be, of course, orthogonal to whether we keep where or not; just an idea of where to put the (optional) return type.

@JeffBezanson JeffBezanson force-pushed the jb/innerctors branch 2 times, most recently from e5242bd to b811390 Compare February 6, 2017 21:28
@JeffBezanson
Copy link
Member Author

JeffBezanson commented Feb 6, 2017

I've updated this to use @vtjnash 's suggestion (I think @Keno also suggested this once) of writing method parameters in curly braces, e.g. f(x::T, y::S) where {T,S}. I think this is better since it takes the exact existing syntax, and just moves it after the arguments plus where. Moving them after, plus switching curly braces to parens seems a bit scatterbrained. This is also closer to the idea in #20444; if we adopt that then we can simply say the where part is optional. Also note the symmetry:

typealias P{A<:Real, B<:Real} Pair{A,B}

P = Pair{A, B} where {A<:Real, B<:Real}

f(x::Pair{A, B}) where {A<:Real, B<:Real}

struct P{A<:Real, B<:Real}
    ...
end

@andyferris
Copy link
Member

So now the short function form is a bit less confusing?

f(::T) where {T} = T

Looks a bit better than having T = T at the end.

PS full symmetry is this:

P = Pair{A, B} where {A<:Real, B<:Real}

f(x::Pair{A, B}) where {A<:Real, B<:Real}

struct P{A, B} where {A<:Real, B<:Real}
    ...
end

but I do love what you have already, Jeff. (Is there some constraints on struct type params that can be expressed in this form but not the existing?

@JeffBezanson
Copy link
Member Author

Is there some constraints on struct type params that can be expressed in this form but not the existing?

I don't believe so. What it would let you express is type parameters that are fixed at a certain value, like

struct P{A, B, Int} where {A, B}

i.e. the third parameter is always Int. This seems pretty useless; I'd rather have the extra conciseness in type definitions. It would probably also break lots of stuff to remove the invariant that the # of parameters of a type equals the # of wheres.

@andyferris
Copy link
Member

OK, thanks.

@o314
Copy link
Contributor

o314 commented Feb 6, 2017

+1 with andy for the full symmetry syntax.


Maybe an idea for the where clause readability issue with T=T :

WHERE CLAUSE @ ONELINER FUNCTION FORM
move the where clause at last position, just before end of line, after the result type

f(::T) = T where {T}

or

f(a::Array{T}) = 1 where {T<:Int}

WHERE CLAUSE @ MANYLINER FUNCTION FORM
no change

f(a::Array{T}) where {T<:Int}
    1
end

Anyway, thanks you for all the work already done.

@JeffBezanson JeffBezanson merged commit 86f3ebd into master Feb 7, 2017
Nullable() = new(false)
Nullable(value::T, hasvalue::Bool=true) = new(hasvalue, value)
Nullable{T}() where T = new(false)
Nullable{T}(value::T, hasvalue::Bool=true) where T = new(hasvalue, value)
Copy link
Contributor

Choose a reason for hiding this comment

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

some form of brackets here, please

@tkelman
Copy link
Contributor

tkelman commented Feb 7, 2017

The readability request, made multiple times by multiple people, was not to have where T = rhs for the single parameter case.

@JeffBezanson
Copy link
Member Author

We will all be happier if we just get used to it.

@JeffBezanson
Copy link
Member Author

Ok, I will add braces to most of these.

@tkelman tkelman deleted the jb/innerctors branch February 7, 2017 19:36
@tkelman
Copy link
Contributor

tkelman commented Feb 8, 2017

There's no line number in the deprecation warning for this, see for example the one from using Documenter

stevengj added a commit to stevengj/julia that referenced this pull request Feb 8, 2017
stevengj added a commit to stevengj/julia that referenced this pull request Feb 10, 2017
stevengj added a commit to stevengj/julia that referenced this pull request Feb 11, 2017
maleadt added a commit to JuliaGPU/CUDAnative.jl that referenced this pull request Feb 13, 2017
maleadt added a commit to JuliaGPU/CUDAdrv.jl that referenced this pull request Feb 13, 2017
maleadt added a commit to JuliaGPU/CUDAnative.jl that referenced this pull request Feb 13, 2017
maleadt added a commit to JuliaAttic/CUDArt.jl that referenced this pull request Feb 16, 2017
vchuravy pushed a commit to JuliaAttic/CUDArt.jl that referenced this pull request Feb 17, 2017
Sacha0 added a commit to Sacha0/julia that referenced this pull request May 13, 2017
@Sacha0 Sacha0 added the needs news A NEWS entry is required for this change label May 13, 2017
@tkelman tkelman removed the needs news A NEWS entry is required for this change label May 14, 2017
tkelman pushed a commit that referenced this pull request May 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.