Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

Am 15.12.2016 09:53 schrieb "Maciej Izak" :
>
>
> 2016-12-14 20:06 GMT+01:00 Sven Barth :
>>
>> I found the "problem": write_rtti_ref() will always write an indirect
>> reference (which is by design), so I adjusted the code that it will
>> really write a direct one. Bye, bye, one level of indirection ;)
>
>
> ^_^ but...
>
> I found another problem. We have small typo for
RTTIRecordRttiInfoToInitInfo. Please merge ASAP
http://bugs.freepascal.org/view.php?id=31118 otherwise InitializeArray
FinalizeArray will stay totally broken >.<

Done :)

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-14 20:06 GMT+01:00 Sven Barth :

> I found the "problem": write_rtti_ref() will always write an indirect
> reference (which is by design), so I adjusted the code that it will
> really write a direct one. Bye, bye, one level of indirection ;)
>

^_^ but...

I found another problem. We have small typo for
RTTIRecordRttiInfoToInitInfo. Please merge ASAP
http://bugs.freepascal.org/view.php?id=31118 otherwise InitializeArray
FinalizeArray will stay totally broken >.<

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

On 14.12.2016 13:09, Maciej Izak wrote:
> 
> 2016-12-14 11:17 GMT+01:00 Sven Barth  >:
> 
> It should be a Pointer of course, not a PPointer... I will correct
> that later on (I shouldn't commit code that late -.- ).
> 
> So I will wait again ^^

I found the "problem": write_rtti_ref() will always write an indirect
reference (which is by design), so I adjusted the code that it will
really write a direct one. Bye, bye, one level of indirection ;)

Regards,
Sven

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

Am 14.12.2016 13:09 schrieb "Maciej Izak" :
>
>
> 2016-12-14 11:17 GMT+01:00 Sven Barth :
>>
>> It should be a Pointer of course, not a PPointer... I will correct that
later on (I shouldn't commit code that late -.- ).
>
> So I will wait again ^^

Oh and should you manage to reproduce the problem you mentioned, please
notify me.

>>
>> PPU version is only increased if the structure of the PPU file is
changed. That RTTI change has nothing to do with the PPU.
>
> Yes, that is true, but I want to be sure :) , at the end we have change
in critical place.

The PPU version is only to ensure that the compiler doesn't read the wrong
data when reading a PPU due to added contents.

For the development version it's always the user's responsibility to ensure
that she doesn't mix compiled units from different trunk revisions.

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-14 11:17 GMT+01:00 Sven Barth :

> It should be a Pointer of course, not a PPointer... I will correct that
> later on (I shouldn't commit code that late -.- ).
>
> So I will wait again ^^

> PPU version is only increased if the structure of the PPU file is changed.
> That RTTI change has nothing to do with the PPU.
>
Yes, that is true, but I want to be sure :) , at the end we have change in
critical place.

This change is big step forward in right direction.

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

Am 14.12.2016 10:00 schrieb "Maciej Izak" :
>
> 2016-12-14 0:16 GMT+01:00 Sven Barth :
>>
>> At least the patch as is (with PPointer) didn't trigger anything on
Win64...
>
>  Will it stay as PPointer (PPRecInitTable)? I am wondering why we have
bug (or expected behavior?) for this.

It should be a Pointer of course, not a PPointer... I will correct that
later on (I shouldn't commit code that late -.- ).

>>
>> Committed with a few adjustments. Now I only need to update User Changes
Trunk, but I'll do that tomorrow *yawns*
>
> Thanks... Now I need to merge "few adjustments" into NewPascal -,-'

:P

> Shall we increase CurrentPPUVersion or we will wait some time (we have
many changes pending so probably inc for CurrentPPUVersion has not much
sense yet)?

PPU version is only increased if the structure of the PPU file is changed.
That RTTI change has nothing to do with the PPU.

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-14 0:16 GMT+01:00 Sven Barth :

> At least the patch as is (with PPointer) didn't trigger anything on
> Win64...
>
>  Will it stay as PPointer (PPRecInitTable)? I am wondering why we have
bug (or expected behavior?) for this.

> Committed with a few adjustments. Now I only need to update User Changes
> Trunk, but I'll do that tomorrow *yawns*
>
Thanks... Now I need to merge "few adjustments" into NewPascal -,-'

Shall we increase CurrentPPUVersion or we will wait some time (we have many
changes pending so probably inc for CurrentPPUVersion has not much sense
yet)?

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

Am 13.12.2016 14:43 schrieb "Maciej Izak" :
>
>
> 2016-12-13 14:31 GMT+01:00 Sven Barth :
>>
>> No idea. But I have a couple of platforms to test this on myself :)
(x86_64-win64, i386-win32, powerpc-darwin, i386-linux, x86_64-linux,
arm-linux, m68k-linux
>
> It is easy to reproduce with PRecInitTable. You can play with "*PART 2*
patch" with PRecInitTable/PPRecInitTable pointers and
InitializeArray/FinalizeArray for records.

At least the patch as is (with PPointer) didn't trigger anything on Win64...

>>
>> Okay, will do that either tonight (don't know yet whether I'll have the
time however) or hopefully tomorrow evening.
>
> ^^ I will wait. Not bad info :)

Committed with a few adjustments. Now I only need to update User Changes
Trunk, but I'll do that tomorrow *yawns*

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-13 14:31 GMT+01:00 Sven Barth :

> No idea. But I have a couple of platforms to test this on myself :)
> (x86_64-win64, i386-win32, powerpc-darwin, i386-linux, x86_64-linux,
> arm-linux, m68k-linux
>
It is easy to reproduce with PRecInitTable. You can play with "*PART 2*
patch" with PRecInitTable/PPRecInitTable pointers and
InitializeArray/FinalizeArray for records.

> Okay, will do that either tonight (don't know yet whether I'll have the
> time however) or hopefully tomorrow evening.
>
^^ I will wait. Not bad info :)
-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

Am 13.12.2016 11:34 schrieb "Maciej Izak" :
>> Okay, I think we can indeed go with this aside from one small remark:
the double indirection for Pointers is only needed if data resides in
different units as those might be in different dynamic packages. Since the
INIT and FULL RTTI of a record are always in the same unit a single
indirection is sufficient (thus PRecInitTable instead of PPRecInitTable).
>
> IIRC something is (was?) wrong (AV) for Mac i386 and Win64 when is used
PRecInitTable instead of (indirect) PPRecInitTable... It is strange because
for example for Win32 and Aarch64 PRecInitTable works fine... Have you an
idea?

No idea. But I have a couple of platforms to test this on myself :)
(x86_64-win64, i386-win32, powerpc-darwin, i386-linux, x86_64-linux,
arm-linux, m68k-linux)

>>
>> Other than that I think we've found a good compromise for our wishes and
hopes for this :)
>
> Phew! So... Please merge PART 2 of MO :P after this step I can start work
on RTTI.pas and on extended info for records (with small "baby steps" ofc).

Okay, will do that either tonight (don't know yet whether I'll have the
time however) or hopefully tomorrow evening.

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-13 11:09 GMT+01:00 Sven Barth :

> Comment regarding your previous mail: right, I forgot that due to
> InitializeArray() and FinalizeArray() being publicly available version of
> the internal helpers we either need to keep INIT and FULL RTTI in sync or a
> way to differentiate them... That's why you added the Terminator field I
> guess?
>
Exactly.

> Okay, I think we can indeed go with this aside from one small remark: the
> double indirection for Pointers is only needed if data resides in different
> units as those might be in different dynamic packages. Since the INIT and
> FULL RTTI of a record are always in the same unit a single indirection is
> sufficient (thus PRecInitTable instead of PPRecInitTable).
>
IIRC something is (was?) wrong (AV) for Mac i386 and Win64 when is used
PRecInitTable instead of (indirect) PPRecInitTable... It is strange because
for example for Win32 and Aarch64 PRecInitTable works fine... Have you an
idea?

> Other than that I think we've found a good compromise for our wishes and
> hopes for this :)
>
Phew! So... Please merge PART 2 of MO :P after this step I can start work
on RTTI.pas and on extended info for records (with small "baby steps" ofc).

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

Am 13.12.2016 09:22 schrieb "Maciej Izak" :
>
>
> 2016-12-12 23:14 GMT+01:00 Maciej Izak :
>>
>> With second iteration we could add your modifications, I have no other
objections.
>

Comment regarding your previous mail: right, I forgot that due to
InitializeArray() and FinalizeArray() being publicly available version of
the internal helpers we either need to keep INIT and FULL RTTI in sync or a
way to differentiate them... That's why you added the Terminator field I
guess?

> New day new ideas :)
>
> With my patch we don't really need ManagedFieldCount, ManagedFields,
ManagedFieldTable in TTypeData (each of Managed* can still be added later).
It means also much better memory usage and it solves RTL problems (which
was mentioned in my previous message). All what we need is:
>
> === code begin ===
> type
>   TInitManagedField = record
> TypeRefRef: TypeInfoPtr;
> FldOffset: SizeInt;
>   end;
>
>   PPRecInitTable = ^PRecInitTable;
>   PRecInitTable = TRecInitTable;
>   TRecInitTable = packed record
> Size: Longint;
> Terminator: Pointer;
> ManagedFieldCount: Longint;
> { ManagedFields: array[0..ManagedFieldCount - 1]
of TInitManagedField ; }
>   end;
>
>   TRecordField = record
> Visibility: TVisibility;
> TypeRef: TypeInfoPtr;
> FldOffset: SizeInt;
>
> Name: PShortString; { can be NULL if no ext RTTI }
>   end;
>
>   TManagedField = TRecordField deprecated;
>
>   TTypeData = record
>   // ...
> tkRecord: (
>   RecSize: Integer;
>   RecInitTable: PPRecInitTable;
>   case Boolean of
> True: (ManagedFldCount: Integer deprecated
'Use RecInitTable^^.ManagedFieldCount or TotalFieldCount depending on your
use case')
> False: (TotalFieldCount: Integer)
>   { Fields: array[1..TotalFieldCount] of TRecordField }
>   { Names: array[1..TotalFieldCount] of {packed} ShortString }
> );
>   end;
> === code end ===
>
> I have hope that this is final solution :)

Okay, I think we can indeed go with this aside from one small remark: the
double indirection for Pointers is only needed if data resides in different
units as those might be in different dynamic packages. Since the INIT and
FULL RTTI of a record are always in the same unit a single indirection is
sufficient (thus PRecInitTable instead of PPRecInitTable).

Other than that I think we've found a good compromise for our wishes and
hopes for this :)

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-12 23:14 GMT+01:00 Maciej Izak :

> With second iteration we could add your modifications, I have no other
> objections.


New day new ideas :)

With my patch we don't really need ManagedFieldCount, ManagedFields,
ManagedFieldTable in TTypeData (each of Managed* can still be added later).
It means also much better memory usage and it solves RTL problems (which
was mentioned in my previous message). All what we need is:

=== code begin ===
type
  TInitManagedField = record
TypeRefRef: TypeInfoPtr;
FldOffset: SizeInt;
  end;

  PPRecInitTable = ^PRecInitTable;
  PRecInitTable = TRecInitTable;
  TRecInitTable = packed record
Size: Longint;
Terminator: Pointer;
ManagedFieldCount: Longint;
{ ManagedFields: array[0..ManagedFieldCount - 1] of TInitManagedField
; }
  end;

  TRecordField = record
Visibility: TVisibility;
TypeRef: TypeInfoPtr;
FldOffset: SizeInt;
Name: PShortString; { can be NULL if no ext RTTI }
  end;

  TManagedField = TRecordField deprecated;

  TTypeData = record
  // ...
tkRecord: (
  RecSize: Integer;
  RecInitTable: PPRecInitTable;
  case Boolean of
True: (ManagedFldCount: Integer deprecated 'Use
RecInitTable^^.ManagedFieldCount
or TotalFieldCount depending on your use case')
False: (TotalFieldCount: Integer)
  { Fields: array[1..TotalFieldCount] of TRecordField }
  { Names: array[1..TotalFieldCount] of {packed} ShortString }
);
  end;
=== code end ===

I have hope that this is final solution :)

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-12 20:41 GMT+01:00 Sven Barth :

> So, my own idea for this.


First insight looks good (kudos for union with "deprecated field" ^^) and
we don't have redundant info, but sorry it can't work in presented form.

With your changes INIT RTTI is incompatible with RTTI, which is critical
defect for RTL. FPC_(INITIALIZE|FINALIZE|COPY|ADDREF) (rtl/inc/rtti.inc) is
used in many contexts - both for internal usage (where generally is used
INIT RTTI) and for external usage (with regular RTTI). Since we have
InitializeArray/FinalizeArray/CopyArray is possible to execute
 FPC_(INITIALIZE|FINALIZE|COPY) with RTTI for records (instead of INIT RTTI
for records) which means AV.

To solve this issue you need obligatory to add RecInitTable: PPointer; (and
Terminator for INIT RTTI). To move forward presented patch is obligatory:

https://github.com/maciej-izak/freepascal/commit/ea23ca80630fae488990dcd4bc62ddc94b18d304
*PART 2* http://bugs.freepascal.org/view.php?id=30687#c96093

This is first step. With second iteration we could add your modifications,
I have no other objections. See above patch - it is already prepared for
differences between INIT RTTI (TRecordInfoInit) and FULL RTTI
(TRecordInfoFull)

Finally we can merge both ideas. User has access to:

* Classical array of compact "managed field" by RecInitTable
* All fields
* All managed fields (in your solution it has sense)

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

On 09.12.2016 22:57, Maciej Izak wrote:
> Hi,
> 
> thanks Sven, finally we have initial RTTI.pas version on trunk. Let me
> start with first serious issue and eventually patch:
> 
> function IsManaged(TypeInfo: PTypeInfo): boolean;
> 
> IsManaged can't work with records because we need know managed fields
> count (in Delphi when ManagedFldCount is bigger than 0 it means that
> record has managed fields and problem is solved). Two possible (and IMO
> correct) solutions:

So, my own idea for this.

Keep the following points in mind however:
- reasonable source(!) backwards compatibility (see for example what I
did when I switched from PTypeInfo to PPTypeInfo: the fields were
changed to PPTypeInfo, but properties were added that return PTypeInfo;
this means that code using e.g. ParentInfo will continue to work, but
code that uses the address to ParentInfo will not; also code that writes
to ParentInfo will fail (as did Lazarus for example))
- the INIT RTTI is left as is; it only contains the managed fields
already and should be as compact as possible as it has to be written for
every record while the FULL RTTI is only written for records that need
it (used by TypeInfo(), used as published property (ToDo)) [Note: I'm
aware that extended RTTI will influence that due to the typelist, but
even then the availability can still be controlled by the user (think
$weaklinkrtti for example)]

=== code begin ===

type
  TRecordField = record
Visibility: TVisibility;
TypeRef: PPTypeInfo;
FldOffset: Integer;
Name: PShortString; { can be NULL if no ext RTTI }
  end;

  TManagedField = TRecordField deprecated;

  TTypeData = record
  // ...
tkRecord: (
  RecSize: Integer;
  { maybe if needed: RecInitTable: Pointer; can still be added later
on }
  ManagedFieldCount: Integer;
  ManagedFieldTable: PRecordField; // points to first entry of
ManagedFields
  case Boolean of
True: (ManagedFldCount: Integer deprecated 'Use
ManagedFieldCount or TotalFieldCount depending on your use case')
False: (TotalFieldCount: Integer)
  { Fields: array[1..TotalFieldCount] of TRecordField }
  { ManagedFields: array[1..ManagedFieldCount] of PRecordField } //
points to corresponding entry in Fields
  { Names: array[1..TotalFieldCount] of {packed} ShortString }
);
  end;

=== code end ===

This allows all fields to be enumerated as usual (namely by getting a
pointer to the area after ManagedFldCount/TotalFieldCount and indexing
TRecordField/TManagedField there.
ManagedFieldTable allows a fast access to a table of pointers to all
managed fields (considering that RTTI is usually used in more processing
intensive contexts (e.g. form instantiation, scripting, invoke, etc) the
additional indirection is acceptable.
Additionally there is the optional name table which is referenced from
each field.

To ease access we could add properties that do the heavy lifting, though
I'd move all that to a new type TRecordData which is referenced from
TTypeData inside a case branch as RecordData: TRecordData with RecSize,
ManagedFieldCount, ManagedFieldTable and the
ManagedFldCount/TotalFieldCount case in the other branch.

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-12 15:53 GMT+01:00 Maciej Izak :

> Field: TManagedField; // :\ a little redundant info (we have this info
> also in TotalFields, but we need to deal with $RTTI EXPLICIT)


To save memory I think we can use FieldIndex: Integer (instead of Field:
TManagedField). FieldIndex is index to TotalFields item...

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-12 13:32 GMT+01:00 Maciej Izak :

> Status of #31102 is unclear. I need clear response about ManagedFldCount
> to decide how to do it - as part of FPC or NewPascal or some mixed way to
> keep compatibility as much as possible (in NewPascal for sure I will
> correct ManagedFldCount, current value of ManagedFldCount and form of
> ManagedFields is unacceptable).


My more complex proposition from point 2. Full backward compatible... Fair
enough, ready for Management Operators + fix for Initialize/FinalizeArray:

=== code begin ===
type
  PPRecInitTable = ^PRecInitTable;
  PRecInitTable = TRecInitTable;
  TRecInitTable = packed record
Size: Longint;
Terminator: Pointer;
ManagedFieldCount: Longint;
{ ManagedFields: array[0..ManagedFieldCount - 1] of TManagedField; }
  end;

  TRecordTypeField = packed record
Field: TManagedField; // :\ a little redundant info (we have this info
also in TotalFields, but we need to deal with $RTTI EXPLICIT)
Flags: Byte; // visibility
Name: ShortString;
  end;

  TTypeData =
...
{ tkRecord }
property ManagedFldCount: Integer read GetManagedFldCount;
 deprecated 'Use TotalFieldCount'; // return TotalFieldCount
...
 tkRecord: (
RecSize: Integer;
RecInitTable: PPRecInitTable; // here is accessible
ManagedFieldCount and ManagedFields
TotalFieldCount: Integer;
   {TotalFields: array[0..TotalFieldCount - 1] of TManagedField;
RecFldCnt: Integer; // for $RTTI EXPLICIT ... FIELDS
RecFields: array[0..RecFldCnt - 1] of TRecordTypeField});
=== code end ===

So merge for PART 2 of Management Operators might be not bad idea:

https://github.com/maciej-izak/freepascal/commit/ea23ca80630fae488990dcd4bc62ddc94b18d304

Now I feel that all of my ideas to solve this are expressed.

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-12 13:07 GMT+01:00 silvioprog :

> Could you do that please? :-)


Status of #31102 is unclear. I need clear response about ManagedFldCount to
decide how to do it - as part of FPC or NewPascal or some mixed way to keep
compatibility as much as possible (in NewPascal for sure I will correct
ManagedFldCount, current value of ManagedFldCount and form of ManagedFields
is unacceptable).

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[silvioprog]

On Mon, Dec 12, 2016 at 4:52 AM, Maciej Izak  wrote:

>
> 2016-12-11 13:18 GMT+01:00 Maciej Izak :
>
>> http://bugs.freepascal.org/view.php?id=31102
>
>
> I can provide proper patch for #31102 and for #31108 (for fields part).
>

Could you do that please? :-)


> TotalFieldCount seems redundant (finally we have in conception RecFldCnt),
> we could point deprecated ManagedFldCount property to RecFldCnt. New
> ManagedFieldCount might be compromise solution. Anyway I have feeling that
> keeping both : ManagedFldCount and ManagedFieldCount is very bad idea
> (especially with new layout for ManagedFields which must contains managed
> fields only).
>
> --
> Best regards,
> Maciej Izak
>

-- 
Silvio Clécio
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-11 13:18 GMT+01:00 Maciej Izak :

> http://bugs.freepascal.org/view.php?id=31102


I can provide proper patch for #31102 and for #31108 (for fields part).

TotalFieldCount seems redundant (finally we have in conception RecFldCnt),
we could point deprecated ManagedFldCount property to RecFldCnt. New
ManagedFieldCount might be compromise solution. Anyway I have feeling that
keeping both : ManagedFldCount and ManagedFieldCount is very bad idea
(especially with new layout for ManagedFields which must contains managed
fields only).

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

2016-12-10 0:54 GMT+01:00 Sven Barth :

> We rename ManagedFldCount to TotalFieldCount, add a field
> ManagedFieldCount and a property ManagedFldCount that returns
> TotalFieldCount for backwards compatibility (and maybe marked as
> deprecated).
>

I've created new bug report to express my strong stand about this problem:

http://bugs.freepascal.org/view.php?id=31102
(copied from bug report)

FPC has long standing critical issue. ManagedFldCount field in TypInfo
(TTypeData) contains "count" for all fields (but should contains "count"
for managed fields only - as name suggest). It is not feature but highly
visible bug.

* The bug has critical meaning for development of RTTI.pas because is
impossible to implement in rational way IsManaged function (ManagedFldCount
is used for that)
* The bug has many performance issues  especially for
InitializeArray/FinalizeArray.
* The bug means more Delphi incompatibility RTTI.
* For scripting languages it means many troubles (for Delphi porting code)
* TManagedField entry for each (I mean here unmanaged fields) field has
zero value for programmer, it has no sense.

any partial fix with pseudo backward compatibility has no sens and will
bring much more noise and confusion  and more complicated RTTI code without
real gain.

the proper layout for part related to records (in current state) in
TTypeData is:

  tkRecord: ( // could be extended for info about attributes, methods
and operators (maybe also properties)
RecSize: Integer;
ManagedFldCount: Integer;
   {ManagedFields: array[0..ManagedFldCnt - 1] of TManagedField;
RecFldCnt: Integer;
RecFields: array[1..RecFldCnt] of TRecordTypeField});

where

  TRecordTypeField = packed record
Field: TManagedField;
Flags: Byte; // visibility
Name: ShortString;
  end;


When we have additional "Name" and "Flag" for managed and unamanaged fields
it has sense, because when we have more info than pure TManagedField for
each field, it could be used for ORM or for scripts. Probably is good idea
to correlate RecFields together with:

{$RTTI EXPLICIT METHODS([]) PROPERTIES([]) FIELDS([])}

The patch provided in #29767 is wrong when we thinking about extended RTTI
(init table should stay simple as possible), same for MO PART 2 (
http://bugs.freepascal.org/view.php?id=30687#c96093 )

It must be changed same as was needed change for indirect type info
(PPTypeInfo) for greater good.

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

10.12.2016 00:54 "Sven Barth":


We rename ManagedFldCount to TotalFieldCount, add a field
ManagedFieldCount and a property ManagedFldCount that returns
TotalFieldCount for backwards compatibility (and maybe marked as
deprecated).


Looks almost like point 4. We need also to adjust ManagedFields.
ManagedFieldCount
is not enough. User still is unable to figure which fields are managed
without many additional conditions -,- . Delphi layout:


ManagedFields: array[0..ManagedFldCnt - 1] of TManagedField;
NumOps: Byte;
RecOps: array[1..NumOps] of Pointer;
RecFldCnt: Integer;
RecFields: array[1..RecFldCnt] of TRecordTypeField;
RecAttrData: TAttrData;
RecMethCnt: Word;
RecMeths: array[1..RecMethCnt] of TRecordTypeMethod

In this context TotalFieldCount looks like yet another incompatibility.
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Sven Barth]

On 09.12.2016 22:57, Maciej Izak wrote:
> Hi,
> 
> thanks Sven, finally we have initial RTTI.pas version on trunk. Let me
> start with first serious issue and eventually patch:
> 
> function IsManaged(TypeInfo: PTypeInfo): boolean;
> 
> IsManaged can't work with records because we need know managed fields
> count (in Delphi when ManagedFldCount is bigger than 0 it means that
> record has managed fields and problem is solved). Two possible (and IMO
> correct) solutions:
> 
> 1. I think it's time to new position in "User Changes" wiki page. We can
> fix that by new behavior (a more Delphi compatible, more logical, more
> proper and a little incompatible with old code, but the risk is
> minimal): ManagedFldCount should be fixed/corrected as mentioned in
> http://bugs.freepascal.org/view.php?id=29767 (so it should count *real*
> managed field as the name "ManagedFldCount" suggests. Otherwise we can't
> implement this function. Btw. old behavior for ManagedFldCount will be
> not usefully anymore with RTTI module/improved TypInfo for counting all
> fields. 

We rename ManagedFldCount to TotalFieldCount, add a field
ManagedFieldCount and a property ManagedFldCount that returns
TotalFieldCount for backwards compatibility (and maybe marked as
deprecated).

Regards,
Sven

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

Re: [fpc-devel] RTTI module and "IsManaged" critical problem

[Maciej Izak]

5. Super slowly checking for each field in ManagedFields, and eventually
new checking for each record in ManagedFields... I am so sad that I need to
post this point :\

-- 
Best regards,
Maciej Izak
___
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel