Crystal: Rename `finalize` special method to use this method name in user code

Created on 4 Jul 2018  路  18Comments  路  Source: crystal-lang/crystal

Sometime I'd like to have a method called finalize but I can't as it's used in the stdlib as the GC finalizer method.

I suggest to rename the stdlib finalizer method to a more specialized name, like gc_finalizer, this way I can make a method finalize for my code.

I think having a more specialized name for the stdlib finalizer method is better because I directly see that it is going to do something related to the gc, and it'd be more difficult to override it by mistake.

WDYT?

help-wanted topicruntime

Most helpful comment

Naming wise it would make sense, but nobody knows about finalize. initialize is everywhere, finalize is quickly forgotten. Make a poll among Crystal developers if they have a) heard of and b) used finalize. Maybe a few know of it.

It's just not obvious that finalize has a special meaning and the compiler won't keep you from using it like any other method that comes with surprisingly unexpected behaviour.
The method is used internally by the Crystal runtime (and not as popular as initialize) and the name should express that it does something special to avoid accidental implementations.

Using GC.add_finalizer(self) do is great, because it explicitly communicates there is something special about it and it doesn't add a GC callback to as public API method. It's completely internal implementation.

All 18 comments

Naming is hard, why not deinit or sth similar?

Why all these :-1: ?

Probably because you can just use another name for your method.

Because finalize goes nicely with initialize, just like destructor fits with constructor. They go hand in hand. Also because the only use of finalize in the whole crystal stdlib would be because of LLVM, which uses the finalize term (but could use something else).

And because this method is a finalizer, as depicted in https://en.wikipedia.org/wiki/Finalizer.

As an alternative, we could entirely remove finalize from the language, and you'd have to do GC.add_finalizer, similar to Ruby. I think finalize is confusing, specially for beginners, because they tend to use it a lot when it's just needed in very few specific cases.

def finalize still has certain restrictions, right? can't raise, can't change fibers...

It might be a good thing to go that route because it makes it far easier to document the unsafety of finalizers.

It might be a good thing to go that route because...

What do you mean by that route @RX14 ?


Also, I'm not suggesting to rename this method to a completely different terminology like destructor, just to make it _harder_ to use by mistake. initialize is common in crystal code and is meant to be defined explicitly, but finalize is not meant to be used often at all, hence my renaming suggestion.
(the fact that I want to use a method named finalize has actually very little to do with this suggestion)

@bew I mean making it GC.add_finalizer(self) do...

We should leave def finalize for if we ever implement struct finalizers like in C++.

We should leave def finalize for if we ever implement struct finalizers like in C++.

But it could also be named differently. I think the name finalize should be free to use. Only initialize should have a special meaning. But it's common and people know about it.

@straight-shoota

But it could also be named differently.

It could but why should it? to quote @ysbaddaden:

Because finalize goes nicely with initialize, just like destructor fits with constructor.

Naming wise it would make sense, but nobody knows about finalize. initialize is everywhere, finalize is quickly forgotten. Make a poll among Crystal developers if they have a) heard of and b) used finalize. Maybe a few know of it.

It's just not obvious that finalize has a special meaning and the compiler won't keep you from using it like any other method that comes with surprisingly unexpected behaviour.
The method is used internally by the Crystal runtime (and not as popular as initialize) and the name should express that it does something special to avoid accidental implementations.

Using GC.add_finalizer(self) do is great, because it explicitly communicates there is something special about it and it doesn't add a GC callback to as public API method. It's completely internal implementation.

@crystal-lang/crystallers Do we won't to proceed with this? It seems like a pretty small breaking change, but would be great to do before 1.0

Proposing just GC.add_finalizer(self) is not enough, because it must call some method anyway. And passing a proc would force a new allocation for each object.

I don't know, I didn't hear too much complain about the current name. In any case it should just be documented better. .NET and Java uses the exact same name.

We _could_ have GC.add_finalizer(obj, method_name) as a macro:

module GC
  macro add_finalizer(obj, method_name = "finalize")
      %obj = obj
      LibGC.register_finalizer_ignore_self(%obj.as(Void*),
      ->(obj, data) { obj.as(typeof(%obj)).{{method_name}} },
      nil, nil, nil)
  end
end

No closure, and you can use names other than "finalize".

It would also mean that at the end of creating a type you would register the finalizer. If an exception happens before that, you are responsible for rescuing it and doing any extra cleanup. It makes things more explicit. For example in Ruby it's ObjectSpace#define_finalizer.

Probably finalizer methods are not very common, so having to do this other thing might be cleaner than having all objects have a "finalize" method. It also makes the compiler code simpler because there's no need to inject "GC.add_finalizer" for types that have a "finalize" method. There's also no accidental "I defined a finalize method but I didn't know that's the GC finalizer".

That might work, although I'd prefer not using macros unless required.
But is it strictly necessary to introduce this breaking change being so close to 1.0? I don't think so. I just did a quick search on GitHub and there are many shards already using it to free resources. Doing this change should at least raise a warning somehow when the finalize method is defined without a call to add_finalizer. I'd rather do this after 1.0, deprecating the current behaviour until 2.0.

Wouldn't renaming finalize! do the trick?

This solves the initial issue of adding something to warn users it is not a regular method.

Generally in the stdlib, having ! at the end of a method is a clear indication of a potentially unexpected behavior.

I think there's no need to keep discussing over this issue. Nobody else complained in 2 years.

We can improve the situation but it's not needed before 1.0. So let's continue the discussion after 1.0.

Thank you!

Was this page helpful?
0 / 5 - 0 ratings