View Issue Details

IDProjectCategoryView StatusLast Update
0038724FPCCompilerpublic2021-04-09 05:05
ReporterRoman Horváth Assigned ToMichael Van Canneyt  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionno change required 
PlatformWindows (PC)OSWindows 
Product Version3.2.0 
Summary0038724: Referencing array foreach loop control variable by @ works wrong…
DescriptionI found an interesting bug connected to the dynamic arrays in Free Pascal.

To introduce the story and explain it a little: I am a teacher, and we program the class Sprite currently. One of the demos is to show how we can click on the sprite, activate it, and move it with another click. The bug is connected to the @ operator. When we use it to the foreach control variable within the loop body, it always points to the last element no matter what, which is wrong.
Steps To ReproducePlease consider the following two fragments of code (to be found in hlavneOkno.pas, in the method TForm1.FormClick in the attached project):


  // This variant does not work.
  // The @Ball reference always points
  // to the last element no matter what…
  (* * )
  for Ball in Balls do
    if Ball.ptIn(ptMouse) then
    begin
      Current := @Ball;
      exit
    end;
  ( * *)


  // This variant does work.
  (* *)
  for i := 0 to 2 do
    if Balls[i].ptIn(ptMouse) then
    begin
      Current := @Balls[i];
      exit
    end;
  (* *)
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget-
Attached Files

Activities

Roman Horváth

2021-04-08 07:55

reporter  

ForEach-issue.7z (63,725 bytes)

Roman Horváth

2021-04-08 08:06

reporter   ~0130170

(To show the bug: Uncomment the first variant, launch the project and click on one of the blue balls. Then try to move the ball by another click. You will see that you always get the last element. (Or the first, if I am wrong, but it seems that it is the last according to the gravity rules… I would understand why it is the first but getting the last is even stranger.) Then try the other variant, and you will see that it works fine.)

Michael Van Canneyt

2021-04-08 09:10

administrator   ~0130171

This is normal behaviour:
In the first construct ball is a temporary variable to which the content of the element array is copied for every iteration.
You could put the @ statement wherever you want, the result will always be the same: the location of the ball variable, this is not the location of the array element.

In the second construct, the temp variable is I, but you are taking the address of the array element, which is what you want.


 

Roman Horváth

2021-04-08 16:03

reporter   ~0130175

Last edited: 2021-04-08 16:21

View 2 revisions

But how? … Why? … I do not understand.

If this is “normal behaviour” then it makes no sense at all. It cannot be used to reference array element… This does not seem right; this makes no good at all… In my opinion, this should be changed because there is no way that such behaviour could any programmer expect. (And any programming language is a communication tool between computer and human, where the human should not be the second in the row. It is the main reason for developing higher programming languages. I do not expect that this would be an easy fix, but I think that this is a necessary fix…)

And if so… How can I explain this to my students? How can I say: “You know, this a “normal behaviour,” because «…»” (please, complete the sentence for me).

Thank you for your kindness and for considering this.

Michael Van Canneyt

2021-04-08 16:55

administrator   ~0130177

To reference an array element, you must use the index in the array and use the @ to get the address of the element in the array.

The behaviour is normal because "for in" in makes a copy of the elements in the array.
That is how for..in is defined. So taking the address of the loop variable takes the address of the copy.

Do not forget, "For in" is defined more general than just for arrays. You can use it on sets, enumeration types and classes.

Take for example:

For A in TAlignment do

What address would @a point to if not the actual variable a?

There are cases where the array does not even actually exist:
if you use the 'GetEnumerator' of some class where the element is retrieved using a getter:
There is simply no address, so in thoses cases you cannot take the address of the elements.

This is completely consistent with the behaviour you are seeing for a real array.

So, if you want to take an address of an array element, you must always use the index in the array.

Bi0T1N

2021-04-08 17:03

reporter   ~0130178

Last edited: 2021-04-08 17:09

View 3 revisions

See my example below, the for-in loop variable uses a local temporary variable (copy) that has a different address than the items stored in the IntArr array. But actually it seems to be not documented at https://www.freepascal.org/docs-html/ref/refsu58.html or in the wiki.

program x;

{$mode Delphi}

uses SysUtils;

type
  PointerAddress = {$IFDEF CPU64}UInt64{$ELSE}UInt32{$ENDIF};

var
  pIntArr: Pointer;
  IntArr: Array of Integer;
  
procedure ArrayLoop(const AArr: Array of Integer);
var
  i: Integer;
begin
  pIntArr := @AArr;
  writeln('Argument: ', PointerAddress(pIntArr));
  pIntArr := @i;
  writeln('Local loop variable: ', PointerAddress(pIntArr));
  for i in AArr do
  begin
    pIntArr := @i;
    writeln('Loop variable: ', PointerAddress(pIntArr));
  end;
end;
  
begin
  // allow 3 items in the array
  SetLength(IntArr, 3);

  pIntArr := @pIntArr;
  writeln('global pIntArr address: ', PointerAddress(pIntArr));

  // IntArr is separate variable that points to first element
  pIntArr := @IntArr;
  writeln('global IntArr address: ', PointerAddress(pIntArr));
  pIntArr := @IntArr;
  writeln('IntArr points to: ', PointerAddress(pIntArr^));

  // addresses of the single items
  pIntArr := @IntArr[0];
  writeln('Item 1: ', PointerAddress(pIntArr));
  pIntArr := @IntArr[1];
  writeln('Item 2: ', PointerAddress(pIntArr));
  pIntArr := @IntArr[2];
  writeln('Item 3: ', PointerAddress(pIntArr));

  writeln('array loop');
  ArrayLoop(IntArr);
end.

output (addresses will differ):
pIntArr address: 4295122960
IntArr address: 4295122976
IntArr points to: 940288
Item 1: 940288 <-- memory address of the first item
Item 2: 940292
Item 3: 940296
array loop
Argument: 940288 <-- same memory address as Item 1
Local loop variable: 20971004 <-- local temporary variable i
Loop variable: 20971004 <-- local temporary variable i
Loop variable: 20971004 <-- local temporary variable i
Loop variable: 20971004 <-- local temporary variable i


You know, this is a “normal behaviour”, because for-in loops are creating temporary copies of the real values that are stored in the original variable”.

Roman Horváth

2021-04-08 21:40

reporter   ~0130183

Thank you, now I understand how it works, but that is not my point. I am not a Pascal programmer. Normally, I use Java, where I use in principle that same algorithm for years (where it works, so I didn’t even think about it could not work in here). However, as a teacher, I became into a situation where I have to teach Pascal, and I am not complaining because Pascal is a good language for teaching and for future teachers (whose are my students), too.

With all the respect, let me put one another simple question. Please, imagine me as average programmer and explain to me how useful and wonderful such behaviour is (no matter how “normal” it is) and why it is much, much better to leave it this way; and I will stop complaining.

I fully understand the staying mechanism that causes the behaviour. However, I am not sure that it is useful and safe to leave it this way. (I can imagine at least one way to fix it – the compiler should check if the foreach control variable is referenced by @ operator and create “backup” of the original pointer in such case, then provide the original pointer at the place of @ appearance…)

I understand that current behaviour might be for safety(?); maybe optimisation(?); reasons, but I am not sure if this is genuinely safe… As if “I” (or some programmer) want(s) to refer to an object using a pointer, I am aware that I want to use the original object and possibly change its properties. That is the benefit of using the pointers…

So, if all people around the Free Pascal community agree that this is a perfectly useful feature, I am not complaining, so be it, but my opinion is that since this was completely unexpected for me, it is not useful and rather it should be considered as unintended kind of a “bug” (maybe it is a strong word for it, but in any case, for me, it was wholly unexpected and I cannot quite agree with that it is “normal”).

(From my point of view, it would be safer and simpler to make “a bypass” for it, let’s say “fix” it, than forever explain why it is working in such a way and why it is “impossible” to make any “fix” for it. 😊)

Michael Van Canneyt

2021-04-08 22:13

administrator   ~0130186

Ahah....

I thought from the beginning your question is very strange and now I think it is
probably due to the fact you come from Java.

As a pascal programmer, I think the compiler behaviour is perfectly normal.
@Ball returns the address of the local variable Ball.

It is a local variable, which gets in turn the values of the elements from the list.
I don't expect Ball to be a reference to an element of the list.
So I also don't expect @Ball to point at the element of the list.

Maybe in Java where everything is an object, this is considered normal behaviour,
but not so in Pascal.

Just as the loop variable i is a local variable which gets in turn the values of 0..2.
I don't expect @i to change either when the loop runs.

I suppose the confusion may also be due to the fact that you may misunderstand how classes are implemented.
A variable of type Class is already a pointer to a memory block that contains the data for your object.

That means, since you are using classes, that the variable Ball itself is already a
pointer to a memory block where the data of the object is located.

So by simply defining
   Current: TSprite;
and changing the code in the loop to
  Current:=Ball;
instead of taking the address, it will achieve what you want.
On Exit, Current will point to the memory block where the data of the correct ball is located.

And your second loop becomes
for i := 0 to 2 do
    if Balls[i].ptIn(ptMouse) then
      Current := Balls[i];

There is no need to take addresses at all, since Ball is already a pointer to an address.

I hope this is more clear now, and you see that there is no need to change anything.

Roman Horváth

2021-04-09 05:05

reporter   ~0130187

Actually, I am “multilanguage”… and my confusion came from C++, where you must use pointers to classes and reference the objects when you want to use them that way. In Java, it works exactly the same way as here (in fact, Java hides all pointers from the programmer; Java pretends that pointers do not exist; but it allows to assign null (nil in Pascal) to object variables – like Pascal does, as I just tried).

I considered Pascal (with its pointers) closer to C++, so I did not expect that I should behave like in Java. I rewrote my program according to your instruction, I removed the PSprite = ^TSprite; type completely, and it works. (Sometimes is hard to be “multilanguage.” It gives you some view to different programming philosophies, but keeping track of the differences and similarities could be difficult.)

Thank you!
🙂

Issue History

Date Modified Username Field Change
2021-04-08 07:55 Roman Horváth New Issue
2021-04-08 07:55 Roman Horváth File Added: ForEach-issue.7z
2021-04-08 08:06 Roman Horváth Note Added: 0130170
2021-04-08 09:10 Michael Van Canneyt Assigned To => Michael Van Canneyt
2021-04-08 09:10 Michael Van Canneyt Status new => resolved
2021-04-08 09:10 Michael Van Canneyt Resolution open => no change required
2021-04-08 09:10 Michael Van Canneyt FPCTarget => -
2021-04-08 09:10 Michael Van Canneyt Note Added: 0130171
2021-04-08 16:03 Roman Horváth Note Added: 0130175
2021-04-08 16:21 Roman Horváth Note Edited: 0130175 View Revisions
2021-04-08 16:55 Michael Van Canneyt Note Added: 0130177
2021-04-08 17:03 Bi0T1N Note Added: 0130178
2021-04-08 17:07 Bi0T1N Note Edited: 0130178 View Revisions
2021-04-08 17:09 Bi0T1N Note Edited: 0130178 View Revisions
2021-04-08 21:40 Roman Horváth Note Added: 0130183
2021-04-08 22:13 Michael Van Canneyt Note Added: 0130186
2021-04-09 05:05 Roman Horváth Note Added: 0130187