Hi,

when "TypeInfo" is used for managed record, then RTTI data for those record is stored twice into executable file (useless). Should I report this as bug? Maybe is some reason for that behavior? Example code to generate extra RTTI bytes:

===

uses
  typinfo;
type
  TFoo = record
    foo: string;
  end;
var
  f: TFoo;
begin
  WriteLn(Integer(GetTypeData(TypeInfo(TFoo))));
  WriteLn(f.foo);
end.

2016-03-01 20:35 GMT+01:00 Maciej Izak :

Seems right. Mhm. TypeInfo implicitly adds RTTI for RTTI table. Example TFoo record symbols (rtti mangled name) generated by attached above code:

INIT_$P$TEST_$$_TFOO
RTTI_$P$TEST_$$_TFOO

By default (without using TypeInfo) is generated INIT_$P$TEST2_$$_TFOO

Please report this in the bugtracker.

Michael.

2016-03-01 23:20 GMT+01:00 Michael Van Canneyt :

Please report this in the bugtracker.

Done, patch attached

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

This is by design and your patch would break existing code anyway.

The init RTTI contains merely those fields that need initialization (namely interfaces, strings and dynamic arrays) and which is used for the fpc_initialize(), fpc_finalize() and fpc_assign() helpers while the full RTTI contains all typed, even those that don't need initialization. Please note that not all types need a separate init RTTI (e.g. classes), but records do (this is determined by needs_separate_init_rtti that each def overrides).

Regards,
Sven

2016-03-02 0:16 GMT+01:00 Sven Barth :

my patch was created only on Michael request. ;) thanks for clarification, that makes sense, it works as expected (my second email as a proof). As far as i understand is possible to divide record RTTI data on INIT table and RTTI table.

INIT table:
-record size
-managed fields
-management operators (done in my branch, not committed yet)

RTTI table (eventually TODO):
-attributes
-other operators
-other info about properties and methods etc.

RTTI table in most of cases includes also most of INIT table data (probably full copy).

2016-03-02 0:48 GMT+01:00 Maciej Izak :

sidenote: at the moment the generation of RTTI table does not make sense. INIT table is sufficient also for exploring record RTTI by TypeInfo.

(probably full copy).

> sidenote: at the moment the generation of RTTI table does not make sense.

INIT table is sufficient also for exploring record RTTI by TypeInfo.

As I already wrote it's not. The INIT table is a subset that only includes managed fields while the RTTI table also contains unmanaged ones. This way the helpers don't need to work through larger records that only contain primitive types.
If that isn't the case then *that* is a bug.

Regards,
Sven

2016-03-02 7:56 GMT+01:00 Sven Barth :

Arghhhh, to much Delphi :\. All confusion is related to my Delphi knowledge. I took it as fact also for FPC.

For Delphi even for XE2 in record case there is no difference between RTTI and INIT table. RTTI table for record does not contains any information about unmanaged fields -,- . Just pure copy of INIT table. (Maybe in Delphi exist only INIT table in that case)

Output for attached example FPC is 2, output for Delphi is 1. In Delphi rich RTTI information about unmanaged fields for records is stored in different place.

=== example ===
type
  TR = record
    x: integer;
    s: string;
  end;
begin
  WriteLn(PTypeData(GetTypeData(typeinfo(TR)))^.ManagedFldCount);
  readln;
end.

table also contains unmanaged ones. This way the helpers don't need to work through larger records that only contain primitive types.

> If that isn't the case then *that* is a bug. Arghhhh, to much Delphi :\. All confusion is related to my Delphi

knowledge. I took it as fact also for FPC.

For Delphi even for XE2 in record case there is no difference between

RTTI and INIT table. RTTI table for record does not contains any information about unmanaged fields -,- . Just pure copy of INIT table. (Maybe in Delphi exist only INIT table in that case)

Output for attached example FPC is 2, output for Delphi is 1. In Delphi

rich RTTI information about unmanaged fields for records is stored in different place.

One could say that in case of FPC the name "ManagedFldCount" isn't quite correct ;)

Regards,
Sven

2016-03-02 11:10 GMT+01:00 Sven Barth :

One could say that in case of FPC the name "ManagedFldCount" isn't quite correct ;)

Maybe is good idea to adjust this? Storing unmanaged fields info as managed fields in RTTI table is a little confusing... There is no gain from storing unmanaged fields in that way. Any additional RTTI information for records should be stored in separated place. For now we have:

-Incorrect generated RTTI table with confusing naming -More incompatible Delphi RTTI
-Bigger RTTI table
-For low level solutions is required additionally (unintuitive) check to confirm field stored inside managed fields array as managed field (!sic) :\

we should do the things in proper way. that can't be explained as "by design".

That depends. RTTI contains all published fields, managed or not. as Sven pointed out, the doubling is there for efficiency reasons.

Michael.

2016-03-02 15:14 GMT+01:00 Michael Van Canneyt :

That depends. RTTI contains all published fields, managed or not. as Sven pointed out, the doubling is there for efficiency reasons.

It can be solved more correctly and also efficiency. Delphi is storing full record RTTI data in other place without confusion. Init rtti is untouched.

Anyway, maybe is good idea to fix this "bug" on the occasion of implementing extended RTTI.

2016-03-02 15:42 GMT+01:00 Maciej Izak :

Delphi is storing full record RTTI data in other place without confusion. Init rtti is untouched.

Ofc. init rtti is also untouched in FPC I mean using the same structure for full rtti and init rtti as breaking thing.

2016-03-02 15:54 GMT+01:00 Maciej Izak :

and I forgot to mention that TypeInfo(TRec) should point to init table or to identical copy of that structure in rtti table for consistent data. For second variant rtti table, may contain additionally data after standard copy of init data.

Maciej Izak wrote on Wed, 02 Mar 2016:

and I forgot to mention that TypeInfo(TRec) should point to init table or to identical copy of that structure in rtti table for consistent data. For second variant rtti table, may contain additionally data after standard copy of init data.

FWIW, our documentation explicitly mentions that our RTTI format is not the same as Delphi's:
http://www.freepascal.org/docs-html/user/usersu87.html (7th bullet)

Jonas

2016-03-02 16:07 GMT+01:00 Jonas Maebe :

FWIW, our documentation explicitly mentions that our RTTI format is not the same as Delphi's: http://www.freepascal.org/docs-html/user/usersu87.html (7th bullet)

There is also in this point: "This should not be a problem if the API of the TypeInfo unit is used and no direct access to the RTTI information is attempted"

according this point, current solution is buggy. Managed fields means managed fields, no less, no more -,- I got unmanaged field description from managed fields description array. You can torture me but I'll never tell: "that is right/correct". According mentioned facts my patch looks correct.

This solution would leave one unable to aquire information about non manged record fields.
So in my opinion either adding seperate information for non manged fields or a simple renaming(making it clear what information to expect) are preferable.

Steve.

2016-03-02 16:40 GMT+01:00 Steve Hildebrandt :

separate information can be added later, together with other rtti additions like attributes, detailed information about fields like field name etc. (which make more sense with mentioned rtti additions for unmanaged record fields rtti. current information about unmanaged fields in "managed fields array" is useless and confusing). Renaming in typinfo module, IMO is bad idea, it breaks more compatibility.

While I can agree that the current way is confusing and renaming might not be the best solution, the aditional Information is not useless. I think it would be bad if changes reduce the amount of obtainable information,
since it leaves people unable to fix their programs if they are reliant on, in this case rtti for unmanaged record fields.

Steve.

full record RTTI data in other place without confusion. Init rtti is untouched.

Anyway, maybe is good idea to fix this "bug" on the occasion of

implementing extended RTTI.

This will not be changed due to backwards compatibility. This is not up for discussion.

Regards,
Sven

2016-03-02 17:15 GMT+01:00 Sven Barth :

This will not be changed due to backwards compatibility. This is not up for discussion.

No. Real argument please and technical detail. You can argue in this way any buggy element in compiler. Any logical example where this bug "feature" is used? I think it works in current way by accident. By fixing this bug more code from Delphi can be ported in correct way, *without functional workaround*.

If you don't have any logical argument, I'll keep it on Delphi compatible and logical way in my branch.

it will not break any correctly written code, even more - after fixing this many correct code from Delphi can start work.

"Maciej Izak" wrote

2016-03-02 17:15 GMT+01:00 Sven Barth : > > This will not be changed due to backwards compatibility. This is not up

for discussion.

No. Real argument please and technical detail. You can argue in this way

any buggy element in compiler. Any logical example where this bug "feature" is used? I think it works in current way by accident. By fixing this bug more code from Delphi can be ported in correct way, *without functional workaround*.

If you don't have any logical argument, I'll keep it on Delphi compatible

and logical way in my branch.

it will not break any correctly written code, even more - after fixing

this many correct code from Delphi can start work.

It will break code that relies on non-managed fields being present in the RTTI. As Jonas said our RTTI is not guaranteed to be Delphi compatible, so *introducing* Delphi compatibility while *sacrificing* backwards compatibility (namely to enumerate non-managed fields) is *not* acceptable. I can already tell you now that this part of your code will definitely not be merged then.

Regards,
Sven

2016-03-02 19:30 GMT+01:00 Sven Barth :

I can already tell you now that this part of your code will definitely not be merged then.

ok. no problem with that, I got used to, similar like many other Delphi compatible code - for example Generics.Collections. ;)

Who is using non-managed fields from array that presents managed fields? Srsly? Proper code will not work with current implementation. It can be different in *details*, but it should be *functional similar* to Delphi - in this case it is not.

The usage of this FPC part is marginal so patch probably don't break anything, and even more - it provide better Delphi compatibility on "functional" level.

FPC is holding non-managed fields in array for managed fields - terrible. Nothing more to say.

"Maciej Izak" wrote

2016-03-02 19:30 GMT+01:00 Sven Barth : > > I can already tell you now that this part of your code will definitely

not be merged then.

ok. no problem with that, I got used to, similar like many other Delphi

compatible code - for example Generics.Collections. ;)

With your collection it's more that I feel uneasy with your implementation. In general I'm in favor of the addition... But this topic is definitely different.

> > It will break code that relies on non-managed fields being present in

the RTTI. As Jonas said our RTTI is not guaranteed to be Delphi compatible, so *introducing* Delphi compatibility while *sacrificing* backwards compatibility (namely to enumerate non-managed fields) is *not* acceptable.

Who is using non-managed fields from array that presents managed fields?

Srsly? Proper code will not work with current implementation. It can be different in *details*, but it should be *functional similar* to Delphi - in this case it is not.

The usage of this FPC part is marginal so patch probably don't break

anything, and even more - it provide better Delphi compatibility on "functional" level.

FPC is holding non-managed fields in array for managed fields - terrible.

Nothing more to say.

How can you know that no one is using this? And only because it's called ManagedFields (or so) that doesn't mean that users haven't discovered that it contains all fields and use it as such.
Also any valid code should work with the additional fields as well since it will have to check the type kinds anyway.
Changing this would break *any* code that relies on the fields being in there, while only that code that does an Assert(False) or something alike will fail with the additional ones.
So again: we won't change this.

Regards,
Sven

2016-03-02 23:38 GMT+01:00 Sven Barth :

conclusion: I can live with that. I will not implement any of my code that will break current rtti table record solution -,- . Eh... thanks for patience in discussion with me!

Maciej Izak wrote

we should do the things in proper way. that can't be explained as "by design".

while i do not understand all the deep reasonings and such, i question the "do things in the proper way" comment... who says that delphi is "doing it in the proper way"?? as long as delphi compatibility up to version X is supported, FPC can do things in their own fashion and still be ""proper""... even if it means aliasing certain functions to conform to delphi's methods ;)