[fpc-devel] TCollection.FOwner

2017-12-27 Thread Juha Manninen
I am proposing to add a private
  FOwner: TPersistent;
to TCollection and remove it from derived classes which already have it.
At least TParams has it but it is not used.
The next step would be to assign a proper owner for derived
TCollection instances, case by case.
Lazarus has code to go up in hierarchy by TPersistent.GetOwner calls. See:
 function GetLookupRootForComponent(APersistent: TPersistent): TPersistent;
So, it already works perfectly if only the Owners are set.

My fundamental goal is to fix issue
  https://bugs.freepascal.org/view.php?id=25068
There is no link from a TCollection to its "owner" object which is
called "LookupRoot" in Lazarus sources. The Collection is not visible
in Object Inspector and changing a CollectionItem cannot mark the
top-level Form/DataUnit as modified.

Mattias has made a hack where the "LookupRoot" of certain TCollection
derivatives can be queried.
It was implemented for TFieldDefs with an explicit Dataset propery.
In r56855 changed it to query TDefCollection which actually defines
the Dataset and thus includes derived IndexDefs, too.
Yes, now FieldDefs and IndexDefs both show in OI and changing them
sets Modified flag correctly.
But, it is still a hack and does not scale. We need a general solution
for all TCollection derivatives.
What more, the "Owner" of TDefCollection is not used which is poor
design. It could be used instead of "Dataset".

TParams has FOwner which is correctly returned by overridden method GetOwner.
However it is never set which can be considered a bug.
I tried to assign owner DataSource.DataSet for it in the only place of
creation I found, but it has no effect.

  Function TCustomSQLStatement.CreateParams: TSQLDBParams;
  var
DS: TDataSet;
  begin
if Assigned(DataSource) then
  DS := DataSource.DataSet
else
  DS := Nil;
Result:=TSQLDBParams.Create(DS);
  end;

TFPWebActions is another TCollection derivative without an owner.
I am sure there are plenty more but I did not study them.
I can provide a patch to add FOwner to TCollection and to return it by
GetOwner but I don't know much about the derived classes in FPC libs.
Help is needed there.

One note: the TCollection.Owner explained here is not inherently tied
to memory management like TComponent.Owner is.

Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel


Re: [fpc-devel] TCollection.FOwner

2017-12-27 Thread Martin Schreiber


On 12/27/2017 04:15 PM, Juha Manninen wrote:
> I am proposing to add a private
>   FOwner: TPersistent;
> to TCollection and remove it from derived classes which already have it.

Just a warning which does not necessarily apply to the TCollection issue
you report:
Be very careful to change component streaming behavior. It is difficult
to see all consequences with inlined and inherited components and all
possible combinations.

Martin
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel


Re: [fpc-devel] TCollection.FOwner

2017-12-27 Thread Michael Van Canneyt



On Wed, 27 Dec 2017, Juha Manninen wrote:


I am proposing to add a private
 FOwner: TPersistent;
to TCollection and remove it from derived classes which already have it.
At least TParams has it but it is not used.


I don't want to burden TCollection with a field that will rarely be used.

The proper solution is to implement GetOwner correctly for all cases that
need it. That's why it is there.

So if you report all cases where GetOwner is missing/not correctly set, 
I will fix them.


Michael.
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel


Re: [fpc-devel] TCollection.FOwner

2017-12-27 Thread Juha Manninen
On Wed, Dec 27, 2017 at 6:39 PM, Michael Van Canneyt
 wrote:
> I don't want to burden TCollection with a field that will rarely be used.
>
> The proper solution is to implement GetOwner correctly for all cases that
> need it. That's why it is there.
>
> So if you report all cases where GetOwner is missing/not correctly set, I
> will fix them.

The cases I know were mentioned : TParams, TDefCollection and TFPWebActions.
There already is TOwnedCollection which adds an Owner. TDefCollection
is based on it but then it adds a useless Dataset instead of using the
Owner.
TParams adds its own FOwner instead of inheriting from TOwnedCollection.
I guess all classes that want to return their owner in GetOwner must
have a variable for it. They could as well inherit from
TOwnedCollection then.

I hope other people will report similar cases for other classes.

Regards,
Juha
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel