View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0036115||FPC||RTL||public||2019-09-29 14:45||2019-10-10 18:44|
|Reporter||Thaddy de Koning||Assigned To||Sven Barth|
|Product Version||3.3.1||Product Build|
|Target Version||Fixed in Version|
|Summary||0036115: code from "Feature request: type safe FreeAndNil" causes Internal error when added to sysutils|
|Description||Ghist: https://wiert.me/2017/12/21/another-case-against-freeandnil/ and much older discussions|
In FreePascal it is possible to write a fully type safe FreeAndNil using a generic procedure.
This would remove some of the objections against it.
procedure FreeAndNil<T:class>(var obj:T);
temp := obj;
obj := nil;
Or in objfpc mode:
generic procedure FreeAndNil<T:class>(var obj:T);
temp := obj;
obj := nil;
|Steps To Reproduce||Try one of the examples where FreeAndNil is your enemy.|
|Tags||No tags attached.|
|Fixed in Revision|
||Assigned to Sven. Adding this to sysutils causes an internal compiler error.|
||Note that we only consider adding this as a PR measure, as we don't think it actually solves any problem.|
If it is only PR -which I don't think it is - I withdraw and plz close.
I will not pursue this if the following is considered:
As it stands, the current implementation of FreeAndNil can be called on anything, like in a List of pointers, Tstringlist.Objects or even a TControl.tag.
FreeAndNil can be called on literally anything! since the parameter is untyped var.
Agree, those are by current standards common mistakes but I see them all too often.
That's aside from some of the issues mentioned by Allen Bauer and Jeroen Pluimers.
As well as the link in the original report.
||Closing as per OP's request.|
Sorry to re-open:
There is a strong anti-pattern here: https://eurekalog.blogspot.com/2009/04/why-should-you-always-use-freeandnil_28.html
If the available pointers - including the above - still not satisfy developers, close again.
I'm reopening this, because adding this code nevertheless points to a bug in the compiler that must be fixed and I want to keep this as a reminder.
Also an argument against FreeAndNil from Nick Hodges: http://www.nickhodges.com/post/Using-FreeAndNil.aspx
Further discussion about this should be moved to the forum or the mailing lists however.
Updated title to reflect current position on report.
MHO: I'm more on Nick Hodges with this, and see no reason for overuse of freeandnil.
||I hope fpc is smart enough to notice that the assembly code of that function is the same for every type and only generate one copy of function in the final program.|
||Currently it's not able to detect that. It's even worse when you have the same specialization in different, unrelated parts of a project, cause then the same specialization for even the same type parameters will be placed in the binary multiple times. I already have some ideas to address this, but the topic is not easy.|
||Then this new code is a bad idea|
Sven reopened the bugreport not because he will actually add the new call, but because an attempt to add it revealed a bug in the compiler.
As said originally, the idea to add it was purely a PR move.
I agree that if the result is that the FreeAndNil call is duplicated for every class that it is used for, it is even a very bad idea.
As far as the assembly code duplication, while that may be the case, presumably the generic FreeAndNil would not actually be used *in* the FPC codebase itself and so would not have any effect whatsoever on binary size by default.
It's no different at all from literally any other generic function or procedure that might ever be added somewhere in the RTL.
For example, currently there is a generic version of IfThen available in SysUtils, which obviously does not cause any bloat issues simply by existing.
In many cases having a generic version of a function or procedure available is actually likely to reduce bloat, I'd say, because they do not generate any assembly code *at all* unless you actively use them in your program, compared to overloaded non-generic functions and procedures (such as the three non-generic overloads of IfThen in math.pp) which *always* generate assembly code regardless of if they're ever used or not.
So in general, I don't think the bloat argument really makes any sense as far as RTL routines, because if they're not actually called *in* the RTL then they literally take up zero space.
IMHO the generic version would be used since it would be selected through automatic type inference.
So it would end up being copied all over the place.
I suppose that might be true, although I don't quite recall exactly how Ryan's patch would handle that particular scenario (where one of the options takes untyped var.)
At the same time I think that would possibly have the benefit of directly exposing "wrong" uses of FreeAndNil though (as in where Obj was not in fact a class at all.)
On the other hand, that could also be avoided completely by just calling the generic version something slightly different, like "FreeAndNilG" or whatever, since there's no real reason that it has to have exactly the same name in the first place.
|2019-09-29 14:45||Thaddy de Koning||New Issue|
|2019-09-29 14:57||Michael Van Canneyt||Assigned To||=> Michael Van Canneyt|
|2019-09-29 14:57||Michael Van Canneyt||Status||new => assigned|
|2019-09-30 12:02||Michael Van Canneyt||Assigned To||Michael Van Canneyt => Sven Barth|
|2019-09-30 12:03||Michael Van Canneyt||Note Added: 0118210|
|2019-09-30 12:04||Michael Van Canneyt||Note Added: 0118211|
|2019-09-30 14:09||Thaddy de Koning||Note Added: 0118213|
|2019-09-30 14:11||Thaddy de Koning||Note Edited: 0118213||View Revisions|
|2019-09-30 14:12||Thaddy de Koning||Note Edited: 0118213||View Revisions|
|2019-09-30 14:19||Michael Van Canneyt||Status||assigned => resolved|
|2019-09-30 14:19||Michael Van Canneyt||Resolution||open => no change required|
|2019-09-30 14:19||Michael Van Canneyt||FPCTarget||=> -|
|2019-09-30 14:19||Michael Van Canneyt||Note Added: 0118214|
|2019-09-30 14:22||Thaddy de Koning||Status||resolved => feedback|
|2019-09-30 14:22||Thaddy de Koning||Resolution||no change required => reopened|
|2019-09-30 14:22||Thaddy de Koning||Note Added: 0118216|
|2019-09-30 14:26||Michael Van Canneyt||Status||feedback => resolved|
|2019-09-30 14:26||Michael Van Canneyt||Resolution||reopened => no change required|
|2019-09-30 14:26||Michael Van Canneyt||Assigned To||Sven Barth => Michael Van Canneyt|
|2019-10-03 14:51||Sven Barth||Assigned To||Michael Van Canneyt => Sven Barth|
|2019-10-03 14:51||Sven Barth||Status||resolved => feedback|
|2019-10-03 14:51||Sven Barth||Resolution||no change required => reopened|
|2019-10-03 14:51||Sven Barth||Note Added: 0118275|
|2019-10-03 14:51||Sven Barth||Status||feedback => assigned|
|2019-10-04 13:25||Marco van de Voort||Summary||Feature request: type safe FreeAndNil => code from "Feature request: type safe FreeAndNil" causes Internal error when added to sysutils|
|2019-10-04 13:25||Marco van de Voort||Note Added: 0118319|
|2019-10-04 13:26||Marco van de Voort||Note Edited: 0118319||View Revisions|
|2019-10-04 16:34||Benito van der Zander||Note Added: 0118324|
|2019-10-05 10:34||Sven Barth||Note Added: 0118344|
|2019-10-05 12:38||Benito van der Zander||Note Added: 0118346|
|2019-10-05 12:57||Michael Van Canneyt||Note Added: 0118347|
|2019-10-10 17:22||Akira1364||Note Added: 0118466|
|2019-10-10 17:24||Akira1364||Note Edited: 0118466||View Revisions|
|2019-10-10 17:26||Akira1364||Note Edited: 0118466||View Revisions|
|2019-10-10 17:27||Akira1364||Note Edited: 0118466||View Revisions|
|2019-10-10 17:30||Michael Van Canneyt||Note Added: 0118467|
|2019-10-10 18:26||Akira1364||Note Added: 0118472|
|2019-10-10 18:27||Akira1364||Note Edited: 0118472||View Revisions|
|2019-10-10 18:44||Akira1364||Note Edited: 0118472||View Revisions|