Discussion:
RTTI module and "IsManaged" critical problem
(too old to reply)
Maciej Izak
2016-12-09 21:57:51 UTC
Permalink
Raw Message
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.

2. http://bugs.freepascal.org/view.php?id=30687#c96093 PART 2 of reworked
patch for Management Operators should be accepted. With this path we have
access to Init RTTI table. Anyway solution 1 is strongly recommended.

Other "solutions" (IMO totally wrong):

3. New compiler intrinsic routine:

function IsManagedType(T: Type): Boolean;

which is no real alternative because IsManaged from RTTI.pas is more
run-time dedicated and is impossible to replace IsManaged by IsManagedType
for all scenarios. Generally new intrinsic IsManagedType is not wrong but
in our context is useless...

4. New additional RTTI generated for records, for example new field for
TTypeData:

ThisIsRealAndNotFakeManagedFldCount: Integer;

with additional list declared after ManagedFields:
array[1..ManagedFldCount] of TManagedField:

ThisIsRealAndNotFakeManagedFields:
array[1..ThisIsRealAndNotFakeManagedFldCount] of TRealAndNotFakeManagedField

Or some flag in TTypeData for records:

YesIamManagedRecordAndIHaveRealManagedFieldsAndYesPleaseIgnoreManagedFldCountWhichIsLying:
boolean;
--
Best regards,
Maciej Izak
Maciej Izak
2016-12-09 22:12:05 UTC
Permalink
Raw Message
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
Sven Barth
2016-12-09 23:54:17 UTC
Permalink
Raw Message
Post by Maciej Izak
Hi,
thanks Sven, finally we have initial RTTI.pas version on trunk. Let me
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
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-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Maciej Izak
2016-12-10 09:15:10 UTC
Permalink
Raw Message
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.
Maciej Izak
2016-12-11 12:18:12 UTC
Permalink
Raw Message
Post by 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
Maciej Izak
2016-12-12 07:52:54 UTC
Permalink
Raw Message
Post by 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
silvioprog
2016-12-12 12:07:04 UTC
Permalink
Raw Message
Post by Maciej Izak
Post by 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? :-)
Post by Maciej Izak
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
Maciej Izak
2016-12-12 12:32:52 UTC
Permalink
Raw Message
Post by 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
Maciej Izak
2016-12-12 14:53:59 UTC
Permalink
Raw Message
Post by 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
Maciej Izak
2016-12-12 17:17:29 UTC
Permalink
Raw Message
Post by 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
Sven Barth
2016-12-12 19:41:43 UTC
Permalink
Raw Message
Post by Maciej Izak
Hi,
thanks Sven, finally we have initial RTTI.pas version on trunk. Let me
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
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-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Maciej Izak
2016-12-12 22:14:30 UTC
Permalink
Raw Message
Post by 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
Maciej Izak
2016-12-13 08:22:06 UTC
Permalink
Raw Message
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
Sven Barth
2016-12-13 10:09:38 UTC
Permalink
Raw Message
Post by 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?
Post by Maciej Izak
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
Post by Maciej Izak
=== 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 ; }
Post by Maciej Izak
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')
Post by Maciej Izak
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
Maciej Izak
2016-12-13 10:33:45 UTC
Permalink
Raw Message
Post by 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.
Post by Sven Barth
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?
Post by Sven Barth
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
Sven Barth
2016-12-13 13:31:59 UTC
Permalink
Raw Message
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).
Post by Maciej Izak
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)
Post by Maciej Izak
Post by Sven Barth
Other than that I think we've found a good compromise for our wishes and
hopes for this :)
Post by Maciej Izak
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
Maciej Izak
2016-12-13 13:43:26 UTC
Permalink
Raw Message
Post by 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.
Post by Sven Barth
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
Sven Barth
2016-12-13 23:16:57 UTC
Permalink
Raw Message
Post by Maciej Izak
Post by 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
Post by Maciej Izak
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...
Post by Maciej Izak
Post by Sven Barth
Okay, will do that either tonight (don't know yet whether I'll have the
time however) or hopefully tomorrow evening.
Post by Maciej Izak
^^ 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
Maciej Izak
2016-12-14 09:00:22 UTC
Permalink
Raw Message
Post by 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.
Post by Sven Barth
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
Sven Barth
2016-12-14 10:17:46 UTC
Permalink
Raw Message
Post by Sven Barth
Post by 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 -.- ).
Post by Sven Barth
Post by Sven Barth
Committed with a few adjustments. Now I only need to update User Changes
Trunk, but I'll do that tomorrow *yawns*
Post by Sven Barth
Thanks... Now I need to merge "few adjustments" into NewPascal -,-'
:P
Post by Sven Barth
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
Maciej Izak
2016-12-14 12:09:37 UTC
Permalink
Raw Message
Post by 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
Sven Barth
2016-12-14 13:23:09 UTC
Permalink
Raw Message
Post by Sven Barth
Post by 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 -.- ).
Post by Sven Barth
So I will wait again ^^
Oh and should you manage to reproduce the problem you mentioned, please
notify me.
Post by Sven Barth
Post by Sven Barth
PPU version is only increased if the structure of the PPU file is
changed. That RTTI change has nothing to do with the PPU.
Post by Sven Barth
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
Sven Barth
2016-12-14 19:06:45 UTC
Permalink
Raw Message
Post by 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-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Maciej Izak
2016-12-15 08:53:36 UTC
Permalink
Raw Message
Post by 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
Sven Barth
2016-12-15 16:12:14 UTC
Permalink
Raw Message
Post by Maciej Izak
Post by 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

Loading...