Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2017-01

Management operators : Copy and Clone confusion...

6 messages · 4 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Maciej Izak

Hi,

in patch/feature proposed in http://bugs.freepascal.org/view.php?id=30687 we have Copy/Clone operators naming convention proposed by Florian (my initial idea was AddRef/Copy).

I'd like to say that Copy/Clone naming convention is terrible in practical usage. :( As the main author I have many troubles and I have always confusion >.< I always must to check which is for adding reference and which one is for copying...

For example Paul Ishenin said in previous discussion ("ANN: Management operators - final patch") : I know Florian insisted on Copy and Clone names but still for my taste it would be the best to have conformity between compiler and RTL.

In current FPC implementation of management operators "Copy" operator is *always* used for increasing reference count and "Clone" operator is used to create copy... In this area I have objections for my own patch.

Maybe someone have other ideas? Maybe we could back to AddRef/Copy which is RTL compatible and is more "real life" friendly?

Best regards, Maciej Izak

Reply by email

Michael Van Canneyt
to Maciej Izak
Maciej Izak wrote 22 lines
Hi, in patch/feature proposed in http://bugs.freepascal.org/view.php?id=30687 we have Copy/Clone operators naming convention proposed by Florian (my initial idea was AddRef/Copy). I'd like to say that Copy/Clone naming convention is terrible in practical usage. :( As the main author I have many troubles and I have always confusion >.< I always must to check which is for adding reference and which one is for copying... For example Paul Ishenin said in previous discussion ("ANN: Management operators - final patch") : I know Florian insisted on Copy and Clone names but still for my taste it would be the best to have conformity between compiler and RTL. In current FPC implementation of management operators "Copy" operator is *always* used for increasing reference count and "Clone" operator is used to create copy... In this area I have objections for my own patch. Maybe someone have other ideas? Maybe we could back to AddRef/Copy which is RTL compatible and is more "real life" friendly?

At the risk of writing nonsense:

I would think that a method name should give a clue as to what it actually does. In that sense AddRef/Clone seems better to me ?
(unless Copy does something else besides increasing the ref count ?)

AddRef is clear to anyone, I think.

If I see "Clone", I immediatly think of an object whose properties are a copy of the original, but which otherwise has nothing to do with the original.

Michael.

Reply by email

Maciej Izak
to Michael Van Canneyt

2017-01-19 10:44 GMT+01:00 Michael Van Canneyt :

At the risk of writing nonsense:

Seems ok to me :)

Show quoted text 11 lines
I would think that a method name should give a clue as to what it actually does. In that sense AddRef/Clone seems better to me ? (unless Copy does something else besides increasing the ref count ?) AddRef is clear to anyone, I think. If I see "Clone", I immediatly think of an object whose properties are a copy of the original, but which otherwise has nothing to do with the original.

current "Clone" works like expected:

(1)
https://github.com/maciej-izak/PascalSmartPointers/blob/1ecf773128b523a328a11f30a9b766858ac57c99/sources/SmartObj.pas#L63-L77 (2)
https://github.com/maciej-izak/PascalSmartPointers/blob/1ecf773128b523a328a11f30a9b766858ac57c99/sources/SmartPtr.pas#L62-L76

"Copy" in (1) and (2) looks somehow strange for me. But for example for Nullable type, convention proposed by Florian is not bad:

(3)
https://github.com/maciej-izak/PascalSmartPointers/blob/1ecf773128b523a328a11f30a9b766858ac57c99/sources/Nullable.pas#L115-L143

"Clone" seems like right move for all examples. But together with "Copy" IMO is crappy. AddRef in (3) is not correct at all but for (1) and (2) is perfect.

Maybe I thinking too much by RTL and Copy/Clone is not that bad? Anyway is hard for me to ignore RTL...

Copy operator with single parameter seems for me really strange. When I starting any of new code for management operators my first declaration looks like:

class operator Copy(constref Src: T; var Dest: T); // >.< meh...

TLTR:
Clone might stay as-is but what with "class operator Copy(var Rec: TNullable<T>);"? AddRef for example (3) might be bad idea. Current pair Copy/Clone operators is used in different context for different ideas. At the end RTL naming convention (AddRef/Copy) might be not bad...

Best regards, Maciej Izak

Reply by email

Florian Klämpfl
to Michael Van Canneyt: At the risk of writing nonsense: I would think that a method name should give a clue as…
Michael Van Canneyt wrote 5 lines
At the risk of writing nonsense: I would think that a method name should give a clue as to what it actually does. In that sense AddRef/Clone seems better to me ?

Even if it is used by types not being ref. counted? Ref. counted types are only a sub-set of managed types.

(unless Copy does something else besides increasing the ref count ?) AddRef is clear to anyone, I think.

For non-ref. counted types?

If I see "Clone", I immediatly think of an object whose properties are a copy of the original, but which otherwise has nothing to do with the original.

This is indeed what it does.

Reply by email

Michael Van Canneyt
to Florian Klämpfl
Florian Klämpfl wrote 15 lines
Am 19.01.2017 um 10:44 schrieb Michael Van Canneyt: > > At the risk of writing nonsense: > > I would think that a method name should give a clue as to what it actually does. > In that sense AddRef/Clone seems better to me ? Even if it is used by types not being ref. counted? Ref. counted types are only a sub-set of managed types. > (unless Copy does something else besides increasing the ref count ?) > > AddRef is clear to anyone, I think. For non-ref. counted types?

I didn't know it is used for non-ref. counted types as well. For those types, there is no difference between copy/clone then?

Michael.

Reply by email

Thorsten Engler
to Michael Van Canneyt

It's been a few years since I looked at the relevant code in the FPC compiler and RTL, so my knowledge may be woefully out of date and/or degraded by time, but IIRC:

- Copy was called with a source and target pointer and is responsible for copying everything that's needed from the source to the target.
- AddRef is called with just a single pointer, after the contents of a variable has been duplicated (simply by copying the contents byte by byte, not by calling the "Copy" operator code).

Again, IIRC, the reason for both to exist is that there are cases (e.g. passing an interface parameter) where the callee is responsible for any lifetime management implications (and the caller might not even be implemented using fpc). The caller just passes the interface pointer on the stack (or in a register), which, while it creates a copy of the bytes, does not actually invoke the copy operator. In cases where reference counting is required (parameter not marked as const), the callee may decide to simply use the value that was passed on the stack as the backing for the parameter, so it calls AddRef on that, without performing another copy.

Given this context, I find the terms AddRef and Copy to be perfectly adequate, even when applied to types that aren't strictly speaking performing reference counting; e.g. IIRC, AddRef for a BSTR (ole string type for windows which does NOT have a reference count) actually involves making a copy of the string contents and replacing the BSTR pointer.

AddRef here means "this is now an *additional* *reference* to whatever these bytes might refer to".

Cheers,
Thorsten

Show quoted text 6 lines
-----Original Message----- From: [mailto:fpc-devel- On Behalf Of Michael Van Canneyt Sent: Monday, 23 January 2017 19:30 To: FPC developers' list Subject: Re: [fpc-devel] Management operators : Copy and Clone

confusion...

Show quoted text 10 lines
On Sat, 21 Jan 2017, Florian Klämpfl wrote: > Am 19.01.2017 um 10:44 schrieb Michael Van Canneyt: >> >> At the risk of writing nonsense: >> >> I would think that a method name should give a clue as to what it

actually

Show quoted text 16 lines
does. >> In that sense AddRef/Clone seems better to me ? > > Even if it is used by types not being ref. counted? Ref. counted types > are only a sub-set of managed types. > >> (unless Copy does something else besides increasing the ref count ?) >> >> AddRef is clear to anyone, I think. > > For non-ref. counted types? I didn't know it is used for non-ref. counted types as well. For those types, there is no difference between copy/clone then? Michael.

Reply by email