Strange bugs with Advanced Records - SOLVED

[Joanna from IRC]

I thought it might be for multi threaded programs because of

This qualifier informs the compiler that within the entire program there is no code that will change the value of the parameter while the procedure/function is executing.
This means that not only the parameter, but also the variable (in above example: x) passed by the caller (e.g. a global var) must not be changed until the call with the constref parameter has returned.

[Fibonacci]

This qualifier informs the compiler that within the entire program there is no code that will change the value of the parameter while the procedure/function is executing.

That is so unbeliveable I had to test it. And it turns out to NOT be true. A variable CAN be modified while some other rountine is "using" the variable as a constref.

It must mean something else, or thats simply not true.

Code: Pascal  [Select][+][-]

1. program app;
2.  
3. uses SysUtils;
4.  
5. var
6.   aint: integer = 0;
7.   cs: TRTLCriticalSection;
8.  
9. function thr1(p: pointer): ptrint;
10. var
11.   i: integer;
12. begin
13.   result := 0;
14.  
15.   // for 5 secs
16.   for i := 1 to 5 do begin
17.     // each 1 second modify "aint"
18.     sleep(1000);
19.     // in a critical section! extra protection
20.     EnterCriticalSection(cs);
21.     try
22.       write('thr1 modifying aint... ');
23.       aint += 1;
24.       write('value now = ', aint, '...');
25.       writeln;
26.     finally
27.       LeaveCriticalSection(cs);
28.     end;
29.   end;
30. end;
31.  
32. procedure proc1(constref x: integer);
33. var
34.   i, calc: integer;
35. begin
36.   writeln('proc1: simulating using the value');
37.   writeln('proc1: is @x = @aint? ', @x = @aint);
38.   calc := 0;
39.   // for 6 secds
40.   for i := 1 to 6 do begin
41.     // each 1 second use "x" which is "aint"
42.     sleep(1000);
43.     // "use" the "x"    
44.     // in a critical section! extra protection
45.     EnterCriticalSection(cs);
46.     try
47.       writeln('proc1: value of constref = ', x);
48.       calc += x;
49.     finally
50.       LeaveCriticalSection(cs);
51.     end;
52.   end;
53.   writeln('proc1: value of "calc" = ', calc, ', SHOULD BE 0 if other code couldnt modify the constref');  
54.   writeln('proc1: work done, constref should be released now');
55. end;
56.  
57. procedure main;
58. begin
59.   InitCriticalSection(cs);
60.   writeln('main: current value = ', aint);
61.   writeln('main: starting thr1 that will try to modify the value');
62.   writeln('main: ... each 1 second, first try after 1 second');
63.   BeginThread(@thr1, nil);
64.   sleep(100); // little sleep after thread start
65.   writeln('main: in the meantime call proc1 which will work in a loop for 6 seconds');
66.   writeln('main: calling proc1 (lock for 6 sec)');
67.   proc1(aint);
68.   writeln('main: proc1 done');
69.   writeln('main: current value = ', aint);
70.   writeln;
71.   DoneCriticalSection(cs);
72.   readln;
73. end;
74.  
75. begin
76.   main;
77. end.

Code: Text  [Select][+][-]

1. main: current value = 0
2. main: starting thr1 that will try to modify the value
3. main: ... each 1 second, first try after 1 second
4. main: in the meantime call proc1 which will work in a loop for 6 seconds
5. main: calling proc1 (lock for 6 sec)
6. proc1: simulating using the value
7. proc1: is @x = @aint? TRUE
8. thr1 modifying aint... value now = 1...
9. proc1: value of constref = 1
10. thr1 modifying aint... value now = 2...
11. proc1: value of constref = 2
12. thr1 modifying aint... value now = 3...
13. proc1: value of constref = 3
14. thr1 modifying aint... value now = 4...
15. proc1: value of constref = 4
16. thr1 modifying aint... value now = 5...
17. proc1: value of constref = 5
18. proc1: value of constref = 5
19. proc1: value of "calc" = 20, SHOULD BE 0 if other code couldnt modify the constref
20. proc1: work done, constref should be released now
21. main: proc1 done
22. main: current value = 5

[Fibonacci]

https://wiki.freepascal.org/Constref

Also what does this example code show? Nothing, what is it?

Switch to the French version, the truth is there.

[TRon]

Switch to the French version, the truth is there.

I rather keep it to the official documentation about constant parameters

[cdbc]

Hi

Also what does this example code show?

AFAICS it tries to show you, that you can call a /reference/ parameter(constref) with a literal value too...
Regards Benny

[Thaddy]

It shows a border case where you can not change the parameter through T, but if T shares the same address as a global or accessible variable elsewhere, then the compiler assumes you want to modify through S instead and allows that.
It demonstrate that gotcha that is applicable to both const and constref, btw.
T is protected within the scope of the method, but S itself has higher scope and within that scope modification of S is allowed. It  violates basically the contract beween compiler and programmer as described.

[ad1mt]

This code demonstrates the problem...

Code: Pascal  [Select][+][-]

1. {$MODE OBJFPC}
2. {$MODESWITCH ADVANCEDRECORDS}
3. program test_extended_record_5;
4. const
5. ISize=5;
6. IMax=4;
7.  
8. type
9. REC = record
10.         private
11.         I:array of integer;
12.         public
13.         class operator :=(const v1:integer):REC;
14.         end;
15.  
16. class operator REC.:=(const v1:integer):REC;
17. var     N:integer;
18. begin
19. setlength(Result.I,ISize);
20. for N:=0 to IMax do Result.I[N]:= v1;
21. end;
22.  
23. var N:integer;
24.  
25. procedure TrashREC(const P1:REC; var P2:REC);
26. begin
27. writeln('TrashREC');
28. P2.I[0]:= 100;
29. end;
30.  
31. procedure P;
32. var     U,V :REC;
33. begin
34. U:= 99;
35. V:= U;
36. write('U='); for N:=0 to IMax do write(U.I[N],' '); writeln;
37. write('V='); for N:=0 to IMax do write(V.I[N],' '); writeln;
38. TrashREC(U,V);
39. write('U='); for N:=0 to IMax do write(U.I[N],' '); writeln;
40. write('V='); for N:=0 to IMax do write(V.I[N],' '); writeln;
41. writeln;
42. end;
43.  
44. begin
45. P;
46. end.
47.  

After calling the TrashREC procedure, the first array element of U is also incorrectly overwritten, as follows:

U=99 99 99 99 99
V=99 99 99 99 99
TrashREC
U=100 99 99 99 99
V=100 99 99 99 99
While investigating this, I found this page in the wiki...
https://wiki.freepascal.org/management_operators
This gives an explanation of what is happening, and a solution to the problem. The fixed/working program is:

Code: Pascal  [Select][+][-]

1. {$MODE OBJFPC}
2. {$MODESWITCH ADVANCEDRECORDS}
3. program test_extended_record_6;
4. const
5. ISize=5;
6. IMax=4;
7.  
8. type
9. REC = record
10.         private
11.         I:array of integer;
12.         public
13.         class operator :=(const v1:integer):REC;
14.         class operator copy(constref v1:REC; var V2:REC);
15.         end;
16.  
17. class operator REC.copy(constref v1:REC; var V2:REC);
18. var     N:integer;
19. begin
20. setlength(v2.I,ISize);
21. for N:=0 to IMax do v2.I[N]:= v1.I[N];
22. end;
23.  
24. class operator REC.:=(const v1:integer):REC;
25. var     N:integer;
26. begin
27. setlength(Result.I,ISize);
28. for N:=0 to IMax do Result.I[N]:= v1;
29. end;
30.  
31. var N:integer;
32.  
33. procedure TrashREC(const P1:REC; var P2:REC);
34. begin
35. writeln('TrashREC');
36. P2.I[0]:= 100;
37. end;
38.  
39. procedure P;
40. var     U,V :REC;
41. begin
42. U:= 99;
43. V:= U;
44. write('U='); for N:=0 to IMax do write(U.I[N],' '); writeln;
45. write('V='); for N:=0 to IMax do write(V.I[N],' '); writeln;
46. TrashREC(U,V);
47. write('U='); for N:=0 to IMax do write(U.I[N],' '); writeln;
48. write('V='); for N:=0 to IMax do write(V.I[N],' '); writeln;
49. writeln;
50. end;
51.  
52. begin
53. P;
54. end.
55.  

Note the use of constref with the v1 parameter of the copy procedure. If you use const instead, you get a compile error "Impossible operator overload". If anyone can explain why this is, I would be curious to know.

[dbannon]

This qualifier informs the compiler that within the entire program there is no code that will change the value of the parameter while the procedure/function is executing.

That is so unbeliveable I had to test it. And it turns out to NOT be true. A variable CAN be modified while some other rountine is "using" the variable as a constref.

Fibonacci, you need to understand what has happened here, you have, by using constref, promised no other process will mess with that variable. That, as far as the compiler is concerned, was a solemn promise, by you, to your compiler.

And you went on to break that promise !  There is a matter of trust here, your compiler will will not, now trust you. It will forever be thinking you are lying to it ! "An integer ? I bet he is going to try and sneak a double in there when I am not watching". The compiler will be on edge, never comfortable, never trusting you again.

I suggest you uninstall the compiler and reinstall ! Its the only way forward.

Davo

[Martin_fr]

Code: Pascal  [Select][+][-]

1. V:= U;

The record contains a dyn-array. Which is a pointer. The above statement copies that pointer.

V.I  and U.I  are now the same array (with a ref count of 2).

[Fibonacci]

@dbannon

Code: Pascal  [Select][+][-]

1. V:= U;

The record contains a dyn-array. Which is a pointer. The above statement copies that pointer.

V.I  and U.I  are now the same array (with a ref count of 2).

Code: Pascal  [Select][+][-]

1.   writeln('@V = @U? ', pointer(V) = pointer(U));
2.   V:= U;
3.   writeln('@V = @U? ', pointer(V) = pointer(U));

Code: Text  [Select][+][-]

1. @V = @U? FALSE
2. @V = @U? TRUE

He is right.

[Bogen85]

I thought it might be for multi threaded programs because of

This qualifier informs the compiler that within the entire program there is no code that will change the value of the parameter while the procedure/function is executing.
This means that not only the parameter, but also the variable (in above example: x) passed by the caller (e.g. a global var) must not be changed until the call with the constref parameter has returned.

But why only for multi threaded programs? Why not just in general?

Many find it useful to limit the side effects and state mutations when calling functions, and want to make any side effects or state mutations obvious and explicit.

If the compiler can help us with that, that is very useful. And 440bx already explained it well.

[Martin_fr]

If the compiler can help us with that, that is very useful. And 440bx already explained it well.

You may perceive it as "help by the compiler" => but it is not.

Code: Pascal  [Select][+][-]

1. ...(const foo: TFooType)

Is you helping the compiler (helping to optimize). Because you are telling the compiler: "foo will not change. Go ahead optimize".

And that means not to change by any means.

Code: Pascal  [Select][+][-]

1. procedure somethnig(const foo: integer);
2. begin
3.   writeln(foo);
4.   CallSub;
5.   inc(foo);
6.   writeln(foo):
7. end;

If you call the above code with "x=1", and if "CallSub has a pointer to "x", and changes that value to -1 => then the above code has no defined behaviour.

If not optimized (as currently, since the compiler is not yet that clever) then it would print: 1 then 0 (after inc(-1))

But if optimized it would print: 1 then 2
Because the compiler would be allowed to keep a copy of the value 1 somewhere were CallSub could not reach it, and use that for optimization.

And this is exactly according to the documentation of const parameters.
And as you can see nothing to do with threads.

And btw, you do that with refcounted data (ansistring, dyn array) and you can get wrong data and crashes (with any current fpc version)


The fact that the compiler may give you an error if you try "foo := ... => is just that the compiler caught you lying. But, if you disguise that lie well enough, the compiler wont spot it. And then the result may be a program that does not behave (or may stop behaving in some future version of fpc, when the compiler gets better at optimizing)

Ok in this example "x" would be passed by value, so Callsub would need a pointer to foo.

[Bogen85]

You may perceive it as "help by the compiler" => but it is not.
...
Is you helping the compiler (helping to optimize). Because you are telling the compiler: "foo will not change. Go ahead optimize".

And that means not to change by any means.

I still take that as a win. (and a mutual helping of each other out). 

I take the approach that I by default I expect the items I pass to functions, that the function will not change the caller's view of those items (or that the function will modify what was passed in, even if it just it's own copy of those), unless I explicit want such a change to take place (or that the parameters are explicitly free to use for whatever local variables because of no const/constref). So const and constref helps clarify those assumptions.

Though I agree, how the compiler actually does it, might not be completely aligned with that, but so far those assumptions have not bitten me.

[Warfley]

After calling the TrashREC procedure, the first array element of U is also incorrectly overwritten, as follows:

U=99 99 99 99 99
V=99 99 99 99 99
TrashREC
U=100 99 99 99 99
V=100 99 99 99 99
While investigating this, I found this page in the wiki...
https://wiki.freepascal.org/management_operators
This gives an explanation of what is happening, and a solution to the problem. The fixed/working program is:

The reason you have this problem is quite simply is that dynamic arrays are (unlinke dynamic strings) not copy on write. Meaning if you have multiple references to the same array, it will not be made unique on access.
For strings there is the function UniqueString, but afaik nothing comparable exists for arrays yet.

For your purposes the most simple way of doing the deep copy would be to simply use setlength, as it ensures a refcount of 1, meaning it can be used to create a unique copy:

Code: Pascal  [Select][+][-]

1. class operator REC.copy(constref v1:REC; var V2:REC);
2. begin
3.   v2.I:=v1.I;
4.   SetLength(v2.I, Length(v2.I));
5. end;

Personally I think a deepcopy intrinisc utilizing RTTI would be a very useful addition to the compiler. All thats needed to implement it is already part of the InitRTTI table used for managed types anyway

[Joanna from IRC]

I read the constref wiki page French {using translator} And it’s quite a different story. It said it’s for interfacing with External code in other languages  ?

Les notes de version de la version 2.6 suggère que vous ne devriez utiliser ceci que pour l'interface avec des routines externes écrites dans d'autres langages, où ce type de passage de paramètre est requis.

I did sound a bit far fetched that passing a value as a constref could stop it being changed in any other part of the program. 

The only way I can think of to do something like that is to use a property with setter with Boolean variable inside to temporarily lock it so it can’t be changed...