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] FPC internal linker issues with Win64 .a startic-link libraries by GCC newer than 4.8.1

[Sven Barth]

Am 12.12.2016 15:09 schrieb "Maciej Izak" :
>
>
> 2016-12-12 14:27 GMT+01:00 Sven Barth :
>>
>> I'm still thinking about a solution that will satisfy both of our points
as good as possible - yes, I already have an idea. Maybe I'll share it this
evening when I'm home again.
>
>
> Do you mean :
>
> === code begin ===
>   TRecordTypeField = packed record // without "Field: TManagedField"
> Flags: Byte; // visibility
> Name: ShortString;
>   end;
>
> ...
> { tkRecord }
> property ManagedFldCount: Integer read GetManagedFldCount;
 deprecated 'Use TotalFieldCount'; // return TotalFieldCount
> ...
>   tkRecord: (
> RecSize: Integer;
> ManagedFieldCount: Integer;
> TotalFieldCount: Integer;
>{TotalFields: array[0..TotalFieldCount - 1] of TManagedField;
> ManagedFields: array[0..ManagedFieldCount - 1] of TManagedField;
// or indirect pointer to init table ?
> RecFields: array[0..TotalFieldCount - 1] of TRecordTypeField});
> === code end ===
>
> Full backward compatible... Fair enough and a little Delphi incompatible
(it is already incompatible so no big difference and no redundant info for
TRecordTypeField)

Nearly. ;) I'll reply later on in the other thread.

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 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] FPC internal linker issues with Win64 .a startic-link libraries by GCC newer than 4.8.1

[Maciej Izak]

2016-12-12 14:27 GMT+01:00 Sven Barth :

> I'm still thinking about a solution that will satisfy both of our points
> as good as possible - yes, I already have an idea. Maybe I'll share it this
> evening when I'm home again.
>

Do you mean :

=== code begin ===
  TRecordTypeField = packed record // without "Field: TManagedField"
Flags: Byte; // visibility
Name: ShortString;
  end;

...
{ tkRecord }
property ManagedFldCount: Integer read GetManagedFldCount;
 deprecated 'Use TotalFieldCount'; // return TotalFieldCount
...
  tkRecord: (
RecSize: Integer;
ManagedFieldCount: Integer;
TotalFieldCount: Integer;
   {TotalFields: array[0..TotalFieldCount - 1] of TManagedField;
ManagedFields: array[0..ManagedFieldCount - 1] of TManagedField; //
or indirect pointer to init table ?
RecFields: array[0..TotalFieldCount - 1] of TRecordTypeField});
=== code end ===

Full backward compatible... Fair enough and a little Delphi incompatible
(it is already incompatible so no big difference and no redundant info for
TRecordTypeField)

> Then don't pick such controversial topics :P
>
You never know what is controversial :P. Core team is unpredictable. ;)

-- 
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] FPC internal linker issues with Win64 .a startic-link libraries by GCC newer than 4.8.1

[Sven Barth]

Am 12.12.2016 12:31 schrieb "Maciej Izak" :
>
>
> 2016-12-09 13:49 GMT+01:00 Sven Barth :
>>
>> Improvements for cppclass are definitely welcome :)
>
>
> Rather not mine :P I have packed cppclass into record. For "wrapped
cppclass" I have "default" field (aka proxy record) for direct access
combined with "managament operators" to handle "new" & "destroy" operators.

Okay, that is indeed something I would not want to see in the compiler...

> I have always many troubles with my patches/reports (many times reopened
- #25610, #25607, long pending time - #27206, not merged - #30687). I don't
have real response about ManagedFldCount (#31102) nor vManagedTable topic.

I'm still thinking about a solution that will satisfy both of our points as
good as possible - yes, I already have an idea. Maybe I'll share it this
evening when I'm home again.

> I always need to use many arguments to force my changes (even simple and
basic changes). It costs me many time, energy and nerves.

Then don't pick such controversial topics :P

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 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] FPC internal linker issues with Win64 .a startic-link libraries by GCC newer than 4.8.1

[Maciej Izak]

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

> Improvements for cppclass are definitely welcome :)


Rather not mine :P I have packed cppclass into record. For "wrapped
cppclass" I have "default" field (aka proxy record) for direct access
combined with "managament operators" to handle "new" & "destroy" operators.

I have always many troubles with my patches/reports (many times reopened -
#25610, #25607, long pending time - #27206, not merged - #30687). I don't
have real response about ManagedFldCount (#31102) nor vManagedTable topic.
I always need to use many arguments to force my changes (even simple and
basic changes). It costs me many time, energy and nerves. I see no sense in
contributing improvements for cppclass. Especially that Jonas is in
opposition to default field/proxy records. I have patch for this but... It
has no sense. #30687 is not merged yet, so what is the sense for
contributing "yet another big patch" when I am not sure about future of
#30687 which is base for many new patches?

I will continue my work in less problematic topics like minor/major bug
fixes for FPC. Improved cppclass will be merged to NewPascal because I
can't see any chance in FPC trunk for my modifications.

Good deal for all :).

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