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

2016-12-13 Thread 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

2016-12-13 Thread 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

2016-12-13 Thread 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

2016-12-13 Thread 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

2016-12-13 Thread 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

2016-12-13 Thread 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