4

NOTE: Bear with me, I feel a little "flame grilled" due to some discussions over here and here and some issues I reported here and here.

Some background

Ye olde (pre 10.4) FreeAndNil looked like this:

FreeAndNil(var SomeObject)

The new and fresh FreeAndNil looks like this:

FreeAndNil(const [ref] SomeObject: TObject);

IMO both have their downsides:

  • The old one doesn't do any type checking, so calling FreeAndNil on pointers, records and interfaces compiles just fine, but produces interesting but usually unwanted effects during runtime. (Goes completely berserk or if you are lucky it halts with EAccessViolation, EInvalidOperation etc.)
  • The new one accepts a const parameter, and therefore any object. But then the provided object pointer is actually changed using some hacky-wacky code.
  • You can now call the new FreeAndNil like this: FreeAndNil(TObject.Create) and it will compile and even run just fine. I liked the old FreeAndNil that warned me when I went wrong and provided e.g. a property instead of a field. Unsure what happens if you provide a object type property to this FreeAndNil implementation. Didn't try.

If we would change the signature into FreeAndNil(var SomeObject:TObject) then it will not allow us to pass any other variable type then exactly the TObject type. Which also makes sense, as if it weren't FreeAndNil, one could easily change a variable provided as type TComponent in the routine change the var variable into an object of a completely different type, e.g. TCollection. Of course FreeAndNil will do no such thing, as it always changes the var parameter to nil.

So this makes FreeAndNil a special case. Maybe even special enough to convince delphi to add a compiler magic FreeAndNil implementation? Votes anyone?

Potential work-around

I came up with the code below as an alternative (here as a helper method, but could as well be part of TObject implementation) which kind-a combines both worlds. The Assert will help finding invalid calls during runtime.

procedure TSGObjectHelper.FreeAndNilObj(var aObject);
begin
  if Assigned(self) then
  begin
    Assert(TObject(aObject)=self,ClassName+'.FreeAndNil Wrong parameter provided!');
    pointer(aObject):=nil;
    Destroy;
  end;
end;

Usage would be something like this:

var MyObj:=TSOmeObject.Create;
...
MyObj.FreeAndNilObj(MyObj);

I have actually tested this routine, and it even is slightly faster than the 10.4 FreeAndNil implementation. I guess because I do the assignment check first and call Destroy directly. What I do not like so much is that:

  • the type checking takes place during runtime, and then only if Assertions are ON.
  • it feels like having to pass the same variable twice. Which isn't necessarily true/required. It has to be the same object, and the parameter has to be a variable.

Another investigation

But wouldn't it be great if one could call without the parameter

var MyObj:=TSomeObject.Create;
...
MyObj.FreeAndNil;

So I messed around with the self pointer and managed to set it to nil using the same Hacky-Wacky code that 10.4 utilizes in their FreeAndNil. Well... that worked inside the method, self pointed to nil. But after calling FreeAndNil like this, the MyObj variable wasn't nil, but a stale pointer. (This was what I expected.) Moreover, MyObj could be a property or (the result of) a routine, constructor etc.

so nope over here as well...

And finally the question:

Can you think of a cleaner/better solution or trick that would:

  • FreeAndNil(var aObject:TObject) with not-so-strict type checking compile time (maybe a Compiler directive?) so it allows compiling and calling for variables of any object type.
  • Complains compile time when something is passed that is not a variable/field of some object type
  • Help describing what is the best solution/requirement in RSP-29716
Stefan Glienke
  • 20,860
  • 2
  • 48
  • 102
H.Hasenack
  • 1,094
  • 8
  • 27
  • 2
    Just don't use FreeAndNil in the first place! If you really must set the variable to nil, do it yourself. – Uwe Raabe Jun 19 '20 at 22:30
  • I don't want to start a big discussion, but then, @UweRaabe, you lose the "exception safety" property of `FreeAndNil`. – Andreas Rejbrand Jun 19 '20 at 22:37
  • @H.Hasenack: You could use a generic `FreeAndNil` with the `class` constraint, but it is a bit too verbose: `TFreer.FreeAndNil(MyFrog);`. – Andreas Rejbrand Jun 19 '20 at 23:04
  • 4
    Huh? FreeAndNill that takes a const [ref], yet it still sets the passed in reference to nil? The function is lying now. While it was rather unsafe previously (the new one is no more safe), at least it didn't lie about it. – Allen Bauer Jun 19 '20 at 23:12
  • 9
    @AllenBauer any chance you would ever consider going back to Embarcadero and cleanup/fix things that have gone awry in your absence? – Remy Lebeau Jun 20 '20 at 02:32
  • 1
    @AllenBauer Delphi had many great developers working on it, but if I could point single greatest loss for Delphi after Anders Hejlsberg left, it would be your departure. I fully agree with Remy... things have gone awry and there is a lot to fix... I don't expect that happening... but having you back would be great beyond description. – Dalija Prasnikar Jun 20 '20 at 07:17
  • 1
    The only meaningful solution for FreeAndNil would be implementing https://quality.embarcadero.com/browse/RSP-13724 Overthinking about other solutions will be just waste of time. RSP-29716 is not possible because you could end up with incompatible object instance in your variable - "pass the dog, get the cat" situation. – Dalija Prasnikar Jun 20 '20 at 08:08
  • 2
    Implementing RSP-29716 is impossible because a var parameter (one that goes in *and* out) is simply invariant - read up about type variance. The trick of treating the value as covariant like FreeAndNil does just works because the out value will only ever be nil and nothing else. – Stefan Glienke Jun 20 '20 at 13:02
  • I had a workaround for the FreeAndNil(var aObjectLT) by creating a TObjectHelper=class helprt for TObject. That works, but the restriction of only 1 helper class at any spot makes me unhappy. FTM Embarcadero could add a class method FreeAndNil at TObject level. – H.Hasenack Jun 21 '20 at 20:35
  • What I meant by a magic compiler routine *could* be something like an overloaded Dispose, This would take the object variable, and since it is overloaded will link to the object based variant of dispose, and set the provided object pointer to nil. and complain compiletime it its not an object variable. The handling of pointers would not have to change,.. – H.Hasenack Jun 21 '20 at 20:40
  • @Stefan Glienke - this is what I meant with less strict type checking, comparable to the $T+ and $V+ compiler directives. One could apply this then for thisrtoutine (and maybe *some* others - who knows. But generally it should be a nonono). – H.Hasenack Jun 21 '20 at 20:45
  • @Allen Bauer - yes! it seems one devil has been traded in for another. Now we get type checking, but lack checking that is a variable etc... *and* the procedure lies. – H.Hasenack Jun 21 '20 at 20:47
  • @H.Hasenack What type checking? It's still an untyped const [ref]. I don't understand the reasoning behind the change. FreeAndNill was intended as a convenience for freeing and object and setting the *variable* to nil. Setting a constant reference to nil is, IMO, a bit hideous. As Dalija has stated, the best and safest solution is one that Delphi doesn't currently support. – Allen Bauer Jun 22 '20 at 02:39
  • 1
    @RemyLebeau, I'm afraid they would not be able to afford me :). All kidding aside, I'm still doing well at Google. I'm learning a lot of new things while also leveraging a lot of my past experience with designing and implementing frameworks and libraries. – Allen Bauer Jun 22 '20 at 02:42
  • @Allen Bauer I meant to have "relaxed" type checking (comparable to $T- $V- options but for objects and class inheritance) in place when it is declared as FreeAndNil(var aObject:TObject) - Sorry I got a bit lost in all threads and discussions :o – H.Hasenack Jun 22 '20 at 06:46

2 Answers2

7

The only proper solution to FreeAndNil that is both type safe and does not allow freeing function results and properties would be generic var parameter:

 procedure FreeAndNil<T: class>(var Obj: T); inline;

But, currently Delphi compiler does not allow generics on standalone procedures and functions https://quality.embarcadero.com/browse/RSP-13724

Still, that does not mean you cannot have generic FreeAndNil implementation, only that it will be a bit more verbose than necessary.

type
  TObj = class
  public
    class procedure FreeAndNil<T: class>(var Obj: T); static; inline;
  end;

class procedure TObj.FreeAndNil<T>(var Obj: T);
var
  Temp: TObject;
begin
  Temp := Obj;
  Obj := nil;
  Temp.Free;
end;

Type inference introduced in Rio will allow you to call it without specifying generic signature:

TObj.FreeAndNil(Obj);

Calling (and using) generic FreeAndNil in older Delphi versions is also possible but even more verbose

TObj.FreeAndNil<TFoo>(Obj);
Dalija Prasnikar
  • 27,212
  • 44
  • 82
  • 159
  • Ah it was the type inference that allowed me to leave out the for the SGFreeAndNil of my TObjectHelper implementation, This allows using my SGFreeAndNil(FSomeFIeld) from within any object. Which covers about 99% of my code where (owned) objects are destroyed. Only in global routines (eg initialization/.finalization) would the speclal call of FreeAndNil have to be called as TObject.FreeAndNil(MyGLobalObject) – H.Hasenack Jun 21 '20 at 20:51
  • 1
    For the lack of generic var on global routines, I would make to suggestion to have an implementation of `class procedure TObject.FreeAndNil(var aObject:T)` at the TObject class level and to deprecate the global `FreeAndNil` alltogether. This also avoids the need of a helper class at `TObject` level. And only when called from global routines one would need `TObject.FreeAndNil(SomeObjectVariable)`. From within all object methods it would simply be FreeAndNil(SomeObject). – H.Hasenack Jun 21 '20 at 21:16
  • What I like less about this solution is that introduces a new class that you do not really need/use. Hence my solution below. For the current 10.4 one might implement this as a `TObjectHelper =class helper for TObject` class, for future versions it should be added to the TObject class. – H.Hasenack Jun 22 '20 at 06:50
2

Because we cannot create a global procedure FreeAndNil<T:class>(var aObject:T) I would suggest the code below as a method to the TObject class. (rtl change to be made by embarcadero, but does not need a compiler change)

class procedure TObject.InternalFreeAndNil(var Object:TObject); static; // strict private class method
begin
  if Assigned(Object) then
  begin
    var tmp:=Object;
    Object:=nil;
    tmp.Destroy;
  end;
end;


class procedure TObject.FreeAndNil<T:class>(var Object:T); inline; // public generic class method
begin
  InternalFreeAndNil(TObject(Object));
end;

and to have the current (10.4 and earlier) FreeAndNil removed from the sysutils unit to avoid ambiguity.

When the new generic FreeAndNil method is called from within any other method, one can simply call:

FreeAndNil(SomeObjectVariable)

and 10.3+ type inference avoids having to write:

FreeAndNil<TMyClassSpec>(SomeObjectVariable)

which is nice because most of your code will compile nicely without a change.

In some other spots, eg global routines and initialization / finalization sections one would have to call:

TObject.FreeAndNil(SomeObjectVariable)

Which to me would be acceptable, and a lot better than the current and historical half-way solutions with a FreeAndNil(const [ref] aObject:TObject) or an untyped FreeAndNil(var aObject)

And since the routine is so utterly simple and performance appears to be an issue, one could argue to have an assembler implementation for it. Though I am not sure if this is allowed/possible for generic, (and preferably inline) methods.

FTM: One could also just keep FreeAndNil(var aObject:TObject) and tell people to do a typecast like below, which also avoids the compiler complaining about the var type. But in this case, probably a lot of source code has to be adjusted. On the other hand it saves on code bloat, still avoids Invalid use of function results, properties or invalid types like records and pointers as parameter to FreeAndNil, and is utterly simple to change/implement.

...
var Obj:=TSomeObject.Create;
try
   DoSOmethingUseFulWithObj(Obj);
finally
  FreeAndNil(TObject(Obj)); // typecast avoids compiler complaining. Compiler wont allow invalid typecasts
end;
...
H.Hasenack
  • 1,094
  • 8
  • 27
  • While you can do whatever you suits you the best in your code, I don't like the ideas of using class helper nor adding FreeAndNil to TObject class in RTL. Class helper is not ideal because you can have only one in scope, so I don't like to use them on such global level because it may collide with another one. Official RTL implementation on TObject class would break backward compatibility without gaining too much. – Dalija Prasnikar Jun 22 '20 at 08:03
  • Also having FreeAndNil on TObject allows mistakes like `Foo.FreeAndNil(Bar)` This is why I prefer having separate class if the option of having generic standalone procedure is out of reach (again, that would be the best solution for the future) – Dalija Prasnikar Jun 22 '20 at 08:03
  • FreeAndNil generated code is simple enough and adding assembler version would not improve it. There are no performance issues in FreeAndNil. Problem with performance would arise only for your proposal of making FreeAndNil thread safe - which again is wrong solution to thread safety issues - FreeAndNil alone cannot make code thread safe. – Dalija Prasnikar Jun 22 '20 at 08:10
  • @DalijaPrasnikar Can you elaborate on what would go wrong with `Foo.FreeAndNil(Bar)` or the potential mistake? It would simple destroy Bar if any, and set Bar to nil. FTM it would be quivalent to `TFoo.FreeAndNil(Bar)` since it is a class proc. It relies on type inference and generics to get the correct `TObject.FreeAndNil(var aObject:TInferredType)`. I Agree a global `FreeAndNil` would be the nicest, but would require a compiler change. RTL Source change seems more likely to happen than compiler change, but who knows. – H.Hasenack Jun 22 '20 at 08:49
  • It could go wrong if you actually wanted to free Foo instead of Bar. If you read that code fast enough you can easily ignore actual parameter Bar because Foo has more weight while reading. – Dalija Prasnikar Jun 22 '20 at 09:32
  • Class name TObj in my example is completely arbitrary... you can choose any name you like. – Dalija Prasnikar Jun 22 '20 at 09:35
  • @DalijaPrasnikar : I think `Foo.FreeAndNil(Bar)` is indeed confusing and should be written as `TFoo.FreeAndNil(Bar)` in the first place. TObj is arbitrary indeed, and I like `TObject` so 1) we can avoid the class helper. And 2) since it is a class method (and not a regular method), it allows for a drop-in-replacement of the existing `FreeAndNil` and would not have impact on the virtual table AFAIK. 3) I don't expect the TObject interface change to break any existing object related code. But then again this might be a lack of my delphi experience. – H.Hasenack Jun 22 '20 at 09:54
  • Hope no-one is annoyed by my english variations :s – H.Hasenack Jun 22 '20 at 09:57
  • There is difference between "should" and "reality". That was the problem with first RTL FreeAndNil implementation, that is the problem with second one. I strongly oppose adding another half baked solution to RTL in terms of TObject.FreeAndNil and next RTL should be proper one - standalone generic. – Dalija Prasnikar Jun 22 '20 at 11:00
  • @DalijaPrasnikar Indeed. And that's why I voted your solution as primary solution. – H.Hasenack Jun 22 '20 at 11:54