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

finalizer function should be first argument #6454

Closed
StefanKarpinski opened this issue Apr 7, 2014 · 13 comments
Closed

finalizer function should be first argument #6454

StefanKarpinski opened this issue Apr 7, 2014 · 13 comments
Labels
breaking This change will break code help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@StefanKarpinski
Copy link
Member

To be consistent with other APIs and allow the use of do-block syntax, the finalizer argument to finalizer should be the first argument, not the second.

@JeffBezanson
Copy link
Member

Maybe a different name for the alternative form would be appropriate. Such as tofinalize(x) do ... or finalizing(x) do ....

@StefanKarpinski
Copy link
Member Author

Why?

@JeffBezanson
Copy link
Member

Trying to avoid breakage. Or we could allow both argument orders, leaving finalizer(::Function, ::Function) ambiguous, which is probably ok as I doubt there are any current uses of that.

@StefanKarpinski
Copy link
Member Author

Sure. Should we deprecate the finalizer(obj, fun) version?

@jariou
Copy link

jariou commented Apr 8, 2014

I don't really have anything to add about the finalizer question per se. However, I have concerns about a somewhat related issue that I wanted to bring up. It is about the pipe forward operator |> and how on the one hand, I like a lot, while on the other I find it much too restrictive in Julia.

In FSharp, a common pattern is to use the pipe operator as in the following pseudo code

loadSomeData
|> map someTransform
|> map someOtherTransform
|> map yetOneMoreTransform
|> reduce reductionFunction

The map and reduce functions take (data, function). In each of the calls above, the resulting data from the previous line is used as the first input to map or reduce and the second parameter is the function.

This is a pattern that is not available in Julia for a number of reasons. The first reason is that the |> operator seems to only apply to functions of one parameter only.

julia> function myMap( x::Int, f::Function)
f(x)
end
myMap (generic function with 1 method)

julia> myMap(3, x->x^2)
9

julia> 3 |> myMap(x->x^2)
ERROR: no method myMap(Function)

julia> exp(3)
20.085536923187668

julia> 3|>exp
20.085536923187668

I find that strange and unnecessarily restrictive. I doubt that it would be much harder from a parsing and compiling point of view to handle functions of multiple parameters. It is essentially done right now with the "do" construct (Thanks Stefan for pointing it out to me on the users list)

`julia> function myMap2( f::Function, x::Int)
f(x)
end
myMap2 (generic function with 1 method)

julia> myMap2( x->x^2, 3)
9

julia> myMap2(3) do x
x^2
end
9
`
The other reason why this pattern is not available is that there is no way to tell the compiler "Hey, I'm not done yet". If by the end of the line you have a complete expression, that is the end of that, you can't force it to continue on the next line. Of course, assuming you would support the pipe operator with function of more than one parameter, lining up 3 or 4 of those on a single line would be far from readable.

Perhaps there is a line continuation character I have not paid attention to and that part is not an issue. Perhaps the do pattern can be continuated and chained in a non obtrusive way but even if it did, I think that for full usefulness, it might have to be used in conjunction with the pipe operator because to get the pattern I describe from FSharp, the function needs to be the second parameter.

So I would like to hear from you two as to whether this is something that was considered and already rejected or is this something you see might make the language more useful possibly. I for one, am a little taken aback by the limitation of the pipe forward operator. So I am suggesting that while you guys are thinking of changing the order of parameters to facilitate one pattern over another, you should take this one in consideration.

Thanks for your consideration

@JeffBezanson
Copy link
Member

Check out #5571; there's been a bunch of discussion there about that very issue.

@jariou
Copy link

jariou commented Apr 8, 2014

Thanks for the pointer. So I see it has been considered. I don't really see a conclusion there though.

I am still puzzled as to why the piping operator is restricted to one variable functions. I see the points made about "what is so special about the first parameter?" My take on it is that it is the simplest one to single out. And if we agree on that, then it should guide decisions for Julia API's such as the one about finalizer. By the way, I rather like tofinalize(x) do.

I'll keep an eye open on issues about the pipe operator and will chime in if they come up.

ivarne added a commit to ivarne/julia that referenced this issue Apr 8, 2014
Will enable the syntax
finalizer(x) do x
   ccall(:free_x, Void, (Ptr{None},), x)
end

See: JuliaLang#6454
@ivarne
Copy link
Member

ivarne commented Apr 8, 2014

I made a simple attempt at this, but the deprecation is creating warnings at build time.

ivarne added a commit to ivarne/julia that referenced this issue Apr 8, 2014
Will enable the syntax
finalizer(x) do x
   ccall(:free_x, Void, (Ptr{None},), x)
end

See: JuliaLang#6454
ivarne added a commit to ivarne/julia that referenced this issue Apr 9, 2014
Will enable the syntax
finalizer(x) do x
   ccall(:free_x, Void, (Ptr{None},), x)
end

See: JuliaLang#6454
@jakebolewski
Copy link
Member

Bump, this needs a rebase.

@ivarne
Copy link
Member

ivarne commented Aug 30, 2014

With so many changes do different parts of the code base, it needs to be done again, rather than a rebase. We should probably also consider changing the order of arguments in the jl_gc_add_finalizer C function for consistency.

@jakebolewski
Copy link
Member

We have gone two releases without changing the argument order.

@tkelman
Copy link
Contributor

tkelman commented Sep 28, 2015

That doesn't mean we shouldn't still do it...

@StefanKarpinski
Copy link
Member Author

I have to agree. I still think we should do this. The main hesitation is that if we completely change the way finalizers work at some point, this will have been a lot of pointless noise. Again, @JeffBezanson's input would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants