Recent

Author Topic: [Res.] Why is enumerating a dynamic array different from accessing it by Index?  (Read 2283 times)

Nimral

  • Full Member
  • ***
  • Posts: 154
  • Keep it simple.
I found out (the hard way ... wasted long hours on this) that the following code snippets produce different results:

These are the key parts showing the problem, a complete source is attached as well.

Code: Pascal  [Select][+][-]
  1. type
  2.   PPayload = ^TPayload;
  3.  
  4.   TPayload = record
  5.     ID: integer;
  6.     Name: string[50];
  7.   end;
  8.   TArrPayload = array of TPayload;
  9.  
  10. var
  11.   ArrPayload: TArrPayload;
  12.  
  13. // ....................
  14.  
  15.  
  16.   // initalize the nodes
  17.  
  18.   for i := low(ArrPayload) to high(ArrPayload) do
  19.   begin
  20.     VirtualStringTree1.AddChild(nil,@ArrPayload[i]);
  21.   end;
  22.  
  23.   for rec in ArrPayload do
  24.      VirtualStringTree1.AddChild(nil, @rec);
  25.  
  26.  

VirtualStringTree.AddChild expects a pointer to data. The for ... in loop does deliver an invalid address, the for loop with integer index counter works correctly. Whatever rec contains, it is not, like I expected, a pointer to a record from the array.

Anyone with deeper insight into FPC who can explain this?

Thnx, Armin.

Enclosed you find a little demo project to show the problem in context of a program, just in case anyone wants to see why I came to the conclusion that there must be a difference. The first 5 nodes are passed correctly, the other five contain a garbage address, and it's obviously 5 times the same address, since the garbage is always the same.

Armin.
« Last Edit: May 07, 2021, 08:50:48 am by Nimral »
Lazarus 2.0.10 on Windows 10, Raspberry Pi OS, VMWare Workstation

ASerge

  • Hero Member
  • *****
  • Posts: 1796
I found out (the hard way ... wasted long hours on this) that the following code snippets produce different results:
Code: Pascal  [Select][+][-]
  1. for rec in ArrPayload do
Here, rec is a local copy of the actual data. Saving the address of this variable is pointless.

Nimral

  • Full Member
  • ***
  • Posts: 154
  • Keep it simple.
I can assign like this, can't I?

Code: Pascal  [Select][+][-]
  1. for rec in ArrPayload do
  2.    rec.Name := 'Bla';
  3.  

and this will modify the records in ArrPayload, right? So it *looks* like rec is an equivalent to ArrPayload[ i ], but it isn't? If it is not, what then is rec? Would there have been a way to pass the address of every record, like intended, using the for ... in enumeration mechanism?

Armin.
« Last Edit: May 06, 2021, 10:00:50 pm by Nimral »
Lazarus 2.0.10 on Windows 10, Raspberry Pi OS, VMWare Workstation

Remy Lebeau

  • Hero Member
  • *****
  • Posts: 929
    • Lebeau Software
The for ... in loop does deliver an invalid address

No, it delivers a valid record, you are just taking the address of that record and expecting that address to outlive the actual lifetime of the record, which it doesn't.

Whatever rec contains, it is not, like I expected, a pointer to a record from the array.

It is not a pointer to a record, no.  It is an actual instance of a record, that has been copied from the array.

The first 5 nodes are passed correctly, the other five contain a garbage address, and it's obviously 5 times the same address, since the garbage is always the same.

It is not garbage. The for..in loop is simply generating a hidden variable for its enumeration to use, and you are storing the address of that variable 5 times.  The variable doesn't exist anymore when the loop is done.

I can assign like this, can't I?

Code: Pascal  [Select][+][-]
  1. for rec in ArrPayload do
  2.    rec.Name := 'Bla';

and this will modify the records in ArrPayload, right?

No, it will not, because ArrPayload is an array of records, so rec is a copy of each record in the array.

Now, if you had an array of object pointers, that would be a different story...

So it *looks* like rec is an equivalent to ArrPayload[ i ], but it isn't? If it is not, what then is rec?

Have you read the documentation yet?

https://www.freepascal.org/docs-html/ref/refsu58.html

Essentially, your for..in loop is basically the same as doing this:

Code: Pascal  [Select][+][-]
  1. var
  2.   rec: TPayload;
  3.   i: Integer;
  4.  
  5. for i := Low(ArrPayload) to High(ArrPayload) do
  6. begin
  7.   rec := ArrPayload[i]; // <-- COPY MADE HERE!
  8.   rec.Name := 'Bla'; // <-- MODIFYING THE COPY, NOT THE ORIGINAL!
  9.   VirtualStringTree1.AddChild(nil, @rec); // <-- STORING THE ADDRESS OF THE COPY!
  10. end; // <-- COPY NOT ASSIGNED BACK TO ARRAY!
« Last Edit: May 07, 2021, 02:45:00 am by Remy Lebeau »
Remy Lebeau
Lebeau Software - Owner, Developer
Internet Direct (Indy) - Admin, Developer (Support forum)

Nimral

  • Full Member
  • ***
  • Posts: 154
  • Keep it simple.
Thanks Remy, for the brief explanation.

I did, however, assume (wrongly) then through the loop variable (rec) I get a "handle" to the current element of the enumerated structure, but it seems that the designers od Pascal had other ideas.

What is the purpose of introducing a local copy then? It doesn't make much sense to me?

And, yes, I flipped through the man pages, I found no information regarding this "local copy". It says (last sentence) that I may not change the loop variable (like it is forbidden to change the counter of a for ... to loop), but it doesn't say anything that it is also forbidden or at least pointless to change the data structure rec points to.

Anyway, languages are defined the way they are, and not like it may seem logical to someone like me.

Armin.
« Last Edit: May 06, 2021, 10:31:14 pm by Nimral »
Lazarus 2.0.10 on Windows 10, Raspberry Pi OS, VMWare Workstation

lucamar

  • Hero Member
  • *****
  • Posts: 4007
What is the purpose of introducing a local copy then? It doesn't make much sense to me?

Using enumerators (which is the "technical" name of that ... hmmm ... technic) in a for...in loop is basically a strategy, or a convenient way, to read through dynamic array-like structures without the need to know or care (as would be case with e.g. a for Low(x) to High(h)) for its size or indexing. That's it.

As Remy explained, inside the for...in loop you deal with a copy of the contained object, so write operations are valid only for that copy, not for the original.

But don't worry too much: this has been bugging out people all over the world since it was introduced ;D
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

egsuh

  • Hero Member
  • *****
  • Posts: 739
Try in following way.

Code: Pascal  [Select][+][-]
  1. type
  2.   PPayload = ^TPayload;
  3.  
  4.   TPayload = record
  5.     ID: integer;
  6.     Name: string[50];
  7.   end;
  8.  
  9.   TArrPayload = array of PPayload;  // not array of TPayload
  10.  
  11. var
  12.   ArrPayload: TArrPayload;
  13.   rec: PPayLoad;                    // define rec as pointer type, not record type
  14.   // ....................
  15.   for rec in ArrPayload do
  16.      VirtualStringTree1.AddChild(nil, rec);
     //  rec itself is a pointer.

BTW, I came to use VST through answering to your questions. So, do not thank me :D

Nimral

  • Full Member
  • ***
  • Posts: 154
  • Keep it simple.
@egsuh,

welcome to the VST hell :-) 3+ Years of development by Mike Lischke alone, 2600 pages of pdf documentation, 32000+ lines of code, working with pointers, giving almost unlimited flexibility which leaves almost unlimited opportunities for hard to find glitches.

Aside from this minor obstacles VST is a great and very versatile component :-) It took me about 3 weeks to get along with it, and I am still not quite done.

Regarding your suggestion: did you test it using my little demo project linked above? I did a quick test, and it won't compile, Error "Got TPayload, expected pointer".

IMHO, like Remy and others have outlined, rec is a local copy of the enumerated record, not a pointer, and adding its address to the tree will later, when you try to access the record via the stored address, create memory havoc, since the local copy of rec is long gone by then. My test program behaved just like that, so I assume that yours won't work.

Greetings from rainy Bavaria,

Armin.
« Last Edit: May 07, 2021, 09:03:02 am by Nimral »
Lazarus 2.0.10 on Windows 10, Raspberry Pi OS, VMWare Workstation

Thaddy

  • Hero Member
  • *****
  • Posts: 10783
If you want to store... Always use packed arrays. Problem solved..

Nimral

  • Full Member
  • ***
  • Posts: 154
  • Keep it simple.
Hm, how would a packed array help in this situation?

Armin.

Btw, you seem to be the most rapidy typing person in the world ...? See attachment :-)
« Last Edit: May 07, 2021, 09:55:29 am by Nimral »
Lazarus 2.0.10 on Windows 10, Raspberry Pi OS, VMWare Workstation

wp

  • Hero Member
  • *****
  • Posts: 8347
Btw, you seem to be the most rapidy typing person in the world ...? See attachment :-)
I think you are confusing decimal and thousand separators.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

egsuh

  • Hero Member
  • *****
  • Posts: 739
I have modified some of your codes, and it seems to work.

Code: Pascal  [Select][+][-]
  1. type
  2.     // ............
  3.     TArrPayload = array of PPayload;  // As I told, set this to array of pointers, not records
  4.      // .......
  5.  
  6. var
  7.     Form1: TForm1;
  8.     ArrPayload: TArrPayload;
  9.  
  10.  
  11. implementation
  12.  
  13. procedure TForm1.FormCreate(Sender: TObject);
  14. const
  15.   NumItems = 5;
  16.  
  17. var
  18.    i: integer;
  19.    rec: PPayload;
  20.  
  21. begin
  22.     // populate a dynamic array
  23.     // ArrPayload := TArrPayload.Create;
  24.     SetLength(ArrPayLoad, NumItems);   // I fixed length, but your approach would be nicer if the size is not known.
  25.  
  26.     for i := 0 to High(ArrPayLoad) do
  27.     begin
  28.         New(rec);                 // You need to allocate memory to rec with New
  29.         rec^.ID := i;
  30.         rec^.Name := Format('Node %d', [i]);
  31.  
  32.         ArrPayLoad[i] := rec;      //  rec, which is pointer is stored in the array.
  33.    end;
  34.  
  35.     // initalize the nodes - want the adress of the records as payloads
  36.     VirtualStringTree1.NodeDataSize := SizeOf(PPayload);
  37.  
  38.     {   // I commented out these
  39.     // these work ...,  
  40.      for i := low(ArrPayload) to high(ArrPayload) do
  41.      begin
  42.             VirtualStringTree1.AddChild(nil,@ArrPayload[i]);
  43.     end;
  44.     //  }
  45.  
  46.   //  following seems to work
  47.   for rec in ArrPayload do
  48.        VirtualStringTree1.AddChild(nil, rec);
  49.  
  50. end;
  51.  
  52. procedure TForm1.FormDestroy(Sender: TObject);
  53. var
  54.      rec: PPayLoad;
  55. begin
  56.      for rec in ArrPayLoad do Dispose(rec);  // You have to free memory, as these are using heap memory, not stack.
  57. end;
  58.  
  59.  

Thaddy

  • Hero Member
  • *****
  • Posts: 10783
The point of packed arrays is that you can store it at once. No iterations....

Nimral

  • Full Member
  • ***
  • Posts: 154
  • Keep it simple.
I can't see how this behaviour is related to my problem. Would you please clarify?

Armin
Lazarus 2.0.10 on Windows 10, Raspberry Pi OS, VMWare Workstation

PascalDragon

  • Hero Member
  • *****
  • Posts: 3056
  • Compiler Developer
Hm, how would a packed array help in this situation?

They don't. Thaddy just provides usual nonsense help... ::)

As others already mentioned: best use the ordinary for-loop, the forin-loop is not really designed for this.

 

TinyPortal © 2005-2018