finalizer takes its arguments backwards (function should be first argument)

[oxinabox]

Finalizer is currently define to be finalizer(x, function).
But (I assert that) it should be finalizer(function, x)
as this allows do-block syntax to define the function.

Right now I must write

function finalize_state(st)
        close(st.pending)
        close(st.complete)
end
finalizer(state, finalize_state)

Where as if it took the function as first argument,
like map, filter, (Dict) get! and remotecall (#11406)
I could be writing:

finalizer(state, finalize_state) do st
        close(st.pending)
        close(st.complete)
end

I suggest deprecating, and creating the new overload.

---

I wonder if it isn't worth search through the whole of Base and Core looking for methods that take functions (or things that look like functions), as a non-first parameter, then I could raise just one issue for all of them (or do PR)
(I could script that pretty easy using parse)

[yuyichao]

Note that the second argument could also be a C pointer.

[ivarne]

Duplicate of #6454.

[tkelman]

We should probably have at least one of them open, since I think many people agree this would be good to do.

[oxinabox]

Oops I thought the other was still open. I'll leave it to a maintainer to work out which to open/close.

[JeffreySarnoff]

I agree that doing this offers more good and less of the less good than obtains not doing this.

[JeffBezanson]

Triage agrees we should change this.

[yuyichao]

And leave the inconsistency for pointer arguments?

[JeffBezanson]

We can change the order for pointer arguments as well to be consistent.

[yuyichao]

It'll still be inconsistent for the pointer one (and also the normal one) since both of them are operating on the object which is also usually (and more widely so) the first argument.

[StefanKarpinski]

... and that is why there is an issue to decide on a precedence of conventions. That precedence determines which argument goes first in this case. The proposal that @JeffBezanson made for convention precedence (which seems like a good one) dictates that the function argument goes first.

[JeffBezanson]

I'll grant that there is no convention for where to put a C function pointer in an argument list. But @yuyichao is there even a single other function that uses the argument order f(object, function)?

[yuyichao]

That proposal is based on the do syntax so while it is good in general, it is not good for this function and I strongly believe we shouldn't simply apply a single "order" and ignore all the special cases for a particular function.

For finalize, it are two arguments against putting the object the second,

1. The function can be replaced with a C function pointer.
2. The usecase (do block) should not be used and must not be encouraged as finalizer. Doing that is usually a sign that the library provide no safe way to explicitly free external resources.

Also note that the list doesn't even cover the case where an single object is to be operated on (not "Thing being mutated"). It is likely due to the absense/low number of functions in base that needs it (though there are a LOT in packages). That should ideally go to the first in the long term so that when we allow more explicit namespace control per object/type, we can write object.f(function) which should also support do syntax without any additional lowering/parser change. Obviously we don't have it now so recommend it now globally would be premature but given this likely future direction and the argument against changing this for finalize given above, I don't think we should change finalize now.

[JeffBezanson]

that when we allow more explicit namespace control per object/type, we can write object.f(function)

We are never doing that.

Are you saying we should change the order of arguments to map? What about the multi-argument case?

though there are a LOT in packages

Are you saying a lot of packages use the argument order object, function? Example?

[StefanKarpinski]

If there are a lot of packages using f(object, function) then that should be fixed. I don't buy the "we should just consider each function as a unique and special snowflake and make everything unpredictable and ad hoc" argument. That's how you end up with a language where you have to look up every single function in order to use it.

[yuyichao]

We are never doing that.

Well, I believe that's one of the usecases of #1974 (or similar) where most people have agreed on. (allowing namespace separation)

Are you saying we should change the order of arguments to map?

Of course NOT.

What about the multi-argument case?

Same as above, I'm only talking about functions (admittedly rare in Base) that there's an clear object that it operates on, or in another word, the cases that fits the traditional OO way well.

Are you saying a lot of packages use the argument order object, function? Example?

No. I'm saying functions where one expect a certain main object is the first object is very common in packages. And yes, there are object's methods (in OO sense) that accept functions (callbacks) too. Having only those functions that takes the arguments in a different order will be really unexpected.

I don't buy the "we should just consider each function as a unique and special snowflake and make everything unpredictable and ad hoc" argument.

For this exact argument you should not have an API that does

obj = create()
do_thing1(obj, v1)
do_thing2(obj, v2)
do_thing3(callback, obj, v3)
do_thing4(obj, v4)

When there isn't an obj argument, map being a good example, we should certainly put the function at the first, if there is though, people are much more likely to call these functions with the same object than with the same functions so putting the function as the first argument will break any consistency. If you want globally recommanded order that's fine too. Put object where the function clearly operations on (i.e. OO methods) as the first argument. That's the case that's used in many package that isn't covered in that list yet.

[rfourquet]

I won't comment the do_thing5 API design, but finalizer doesn't seem to be concerned by that.

The usecase (do block) should not be used [...] as finalizer.

Base itself currently uses anonymous function as second arg of finalizer in a few places (e.g. in "regex.jl"); if the order is switched, obviously the do syntax would be used; I don't see what's wrong with that.

[yuyichao]

Base itself currently uses anonymous function as second arg of finalizer in a few places (e.g. in "regex.jl"); if the order is switched, obviously the do syntax would be used; I don't see what's wrong with that.

So those should be fixed.

[StefanKarpinski]

This argument that finalize shouldn't be used with a do block is a total red herring. We have an argument ordering convention and the fact that a function should not be used with a do block is not a good argument for breaking that convention. Moreover, the current limitation that using a do block is a bad idea for a finalizer is an implementation detail. That could cease to be a bad idea at any point in the future. It already works and there are occasions where it's fine to use it.

[yuyichao]

is not a good argument for breaking that convention.

It's good enough to close this issue since that's the whole reason this was opened.

Moreover, the current limitation that using a do block is a bad idea for a finalizer is an implementation detail.

No. It is a bad idea because, well, it's a bad idea to not close resources explicitly. We can have more deterministic destruction syntax but that won't be a finalizer.

[StefanKarpinski]

This argument doesn't make any sense. You basically want to get rid of finalize, which is fine and all, but even if we avoid using it more, we're not going to get rid of it entirely. And the fact that we want to minimize use of finalizers does not justify them not following best practices for the rest of the standard library.

[yuyichao]

You basically want to get rid of finalize, which is fine and all, but even if we avoid using it more, we're not going to get rid of it entirely.

No. I don't want to get rid of it. I just want to prevent the bad style the original post want to do.

And the fact that we want to minimize use of finalizers does not justify them not following best practices for the rest of the standard library.

And so that's exactly what the code in #16307 (comment) is about.

[oxinabox]

So the guts of this argument is over the priority of the order of operands conventions.
With the conflict being between:

• Methods that are "Classic OO Single Dispatch method"-like should take the object as their first argument. (Like python etc's self)
  vs
• Methods that take a call-able argument should take that in first position. (So do blocks work, where appropriate)

Surely this is an argument not to be had here, but instead at #19150
Though I appreciate that this issue is much older.

A better discussion would be at #19150 talking in the abstract.
Thus going for something that is all-round optimal.
Not merely optimal for finalizer

[rfourquet]

So those should be fixed.

I think I'm missing something here; but if this is technically incorrect to use an anonymous function in finalizer, the docs should be fixed too.

[StefanKarpinski]

Correct, @oxinabox, and since we don't actually have OO, the conflict does not really exist. As I understand it, @yuyichao wants to keep the order of the arguments to finalizer as is, despite not following the proposed convention, because somehow that acts as some kind of slap on the wrist to people who would commit the cardinal sin of using an anonymous function as a finalizer.

[yuyichao]

we don't actually have OO, the conflict does not really exist.

We don't have a lot that in Base which is why the convention doesn't have it. But it does exist in packages.

despite not following the proposed convention,

No. I'm also proposing to add that to the convention and I don't agree with the current convention on this.

because somehow that acts as some kind of slap on the wrist to people who would commit the cardinal sin of using an anonymous function as a finalizer.

Apparently this is true. #16307 (comment) is by itself a good example how the code must not be written.

[StefanKarpinski]

If you can make the case on #19150 that there's some other convention that should take precedence over function arguments being first, and that convention applies tofinalizer then so be it, but so far you haven't done that, you've only made a very specific argument about this one single function, insisting that it should be treated in a completely ad hoc and inconsistent manner.

[yuyichao]

Of course I have!. Multiple times.

[StefanKarpinski]

If you have a issues with Jeff's proposed precedence of conventions, then make that clear over there, not on random comments on issues that are tangentially related. If the general argument holds up, then it automatically applies to this issue. If not, then finalizer is not going to be the one and only exception to the rules.

[yuyichao]

not on random comments on issues that are tangentially related

I don't want to comment there mainly because of the absense/low number of functions in base that needs it. This is the right place for this discussion since there isn't many other function in base that need to follow this rule and this is also the only function AFAICT where such a rule conflict with the function first rule.

If not then finalizer is not going to be the one and only exception to the rules.

And I still disagree with this completely.

[StefanKarpinski]

If a general case can't be made then there's no argument at all.

[yuyichao]

If a general case can't be made then there's no argument at all.

Depending on what do you mean by "general case". There's of course a general case but there isn't as many cases in Base as in packages and the only overlapping case in base is finalize.

[StefanKarpinski]

You should explain the general case in #19150. Even if there aren't many examples in Base, it should be in the convention prioritization.

[JeffBezanson]

@vtjnash suggests that we should deprecate the finalize function. That would encourage defining and using an explicit resource-freeing function like close. I think this is a good idea --- if an object supports close then finalize is merely a slow way to call it.