Recent

Author Topic: Failure to assign record fields via setter  (Read 3041 times)

Nitorami

  • Sr. Member
  • ****
  • Posts: 368
Failure to assign record fields via setter
« on: August 20, 2019, 01:45:55 pm »
The code below defines a 2D Matrix of complex numbers. The elements are stored in linear array "data", and can be accessed via getters/setters.

The setter expects type complex, hence it is not possible to make an assignment to the real or imaginary part only, e.g.
Code: Pascal  [Select]
  1. A[0,0].re := 1;
throws a compiler error "Argument cannot be assigned to".

That is understandable. But when I encapsulate this in a "with" construct, the assignment still does not work, but there is no compiler warning. Rather dangerous, and I suspect a bug. Please confirm.

FPC 3.0.4, win32

Code: Pascal  [Select]
  1. {$mode objfpc}
  2. {$modeswitch advancedrecords}
  3.  
  4. type complex = record re,im: double; end;
  5.  
  6. type TMatrix = record
  7.   strict private
  8.     sizex, sizey: longint;
  9.     function  GetItem (const x,y: longint): complex;
  10.     procedure PutItem (const x,y: longint; a: complex);
  11.     property  Item    [const x,y:longint]: complex read GetItem write PutItem; default;
  12.     procedure SetSize (const x,y: longint);
  13.   private
  14.     Data: array of complex;
  15.   public
  16.     property  dimx: longint read sizex;
  17.     property  dimy: longint read sizey;
  18.     procedure init  (const x,y: longint);
  19.     procedure print;
  20.   end;
  21.  
  22.  
  23. function  TMatrix.GetItem (const x,y: longint): complex;
  24. begin
  25.   result := Data[x*sizey+y];
  26. end;
  27.  
  28. procedure TMatrix.PutItem (const x,y: longint; a: complex);
  29. begin
  30.   Data[x*sizey+y] := a;
  31. end;
  32.  
  33. procedure TMatrix.SetSize (const x,y: longint);
  34. begin
  35.   SetLength (Data,x*y);
  36.   sizex := x; sizey := y;
  37. end;
  38.  
  39. procedure TMatrix.init (const x,y: longint);
  40. begin
  41.   Data := nil;
  42.   SetSize (x,y);
  43. end;
  44.  
  45. procedure TMatrix.print;
  46. var x,y: longint;
  47. begin
  48.   for y := 0 to dimy-1 do  begin
  49.     for x := 0 to dimx-1 do write (self[x,y].re:6:2,'',self[x,y].im:6:2,'j ');
  50.     writeln;
  51.   end;
  52.   writeln;
  53. end;
  54.  
  55.  
  56. var A: TMatrix;
  57.     i: longint;
  58.  
  59. begin
  60.   A.Init (3,3);
  61.   writeln ('Direct array access works :');
  62.   for i := 0 to A.dimx-1 do with A.data[i*A.dimy+i] do begin
  63.     re := i;
  64.     im := i;
  65.   end;
  66.   A.print;
  67.  
  68.   A.Init (3,3);
  69.   writeln ('Access via setter fails - all assignments to A are ignored without a warning');
  70.   for i := 0 to A.dimx-1 do with A[i,i] do begin
  71.     re := i;
  72.     im := i;
  73.   end;
  74.   A.print;
  75.  
  76. end.
  77.  

julkas

  • Sr. Member
  • ****
  • Posts: 428
  • KISS principle / Lazarus 2.0.0 / FPC 3.0.4
Re: Failure to assign record fields via setter
« Reply #1 on: August 20, 2019, 02:51:07 pm »
The setter expects type complex, hence it is not possible to make an assignment to the real or imaginary part only, e.g.
Code: Pascal  [Select]
  1. A[0,0].re := 1;
throws a compiler error "Argument cannot be assigned to".

That is understandable. But when I encapsulate this in a "with" construct, the assignment still does not work, but there is no compiler warning. Rather dangerous, and I suspect a bug. Please confirm.
FPC 3.0.4, win32
Yes. FPC 3.0.4 - no compiler warnings using (dangerous) with.
See "Object Pascal Handbook" by Marco Cantu.

BTW This works -
Code: Pascal  [Select]
  1. {$mode objfpc}
  2. {$modeswitch advancedrecords}
  3.  
  4. type complex = record re,im: double; end;
  5.  
  6. type TMatrix = record
  7.   strict private
  8.     sizex, sizey: longint;
  9.     function  GetItem (const x,y: longint): complex;
  10.     procedure PutItem (const x,y: longint; a: complex);
  11.     property  Item    [const x,y:longint]: complex read GetItem write PutItem; default;
  12.     procedure SetSize (const x,y: longint);
  13.   private
  14.     Data: array of complex;
  15.   public
  16.     property  dimx: longint read sizex;
  17.     property  dimy: longint read sizey;
  18.     procedure init  (const x,y: longint);
  19.     procedure print;
  20.   end;
  21.  
  22.  
  23. function  TMatrix.GetItem (const x,y: longint): complex;
  24. begin
  25.   result := Data[x*sizey+y];
  26. end;
  27.  
  28. procedure TMatrix.PutItem (const x,y: longint; a: complex);
  29. begin
  30.   Data[x*sizey+y] := a;
  31. end;
  32.  
  33. procedure TMatrix.SetSize (const x,y: longint);
  34. begin
  35.   SetLength (Data,x*y);
  36.   sizex := x; sizey := y;
  37. end;
  38.  
  39. procedure TMatrix.init (const x,y: longint);
  40. begin
  41.   Data := nil;
  42.   SetSize (x,y);
  43. end;
  44.  
  45. procedure TMatrix.print;
  46. var x,y: longint;
  47. begin
  48.   for y := 0 to dimy-1 do  begin
  49.     for x := 0 to dimx-1 do write (self[x,y].re:6:2,'',self[x,y].im:6:2,'j ');
  50.     writeln;
  51.   end;
  52.   writeln;
  53. end;
  54.  
  55.  
  56. var A: TMatrix;
  57.     i: longint;
  58.     c: complex;
  59. begin
  60.   A.Init (3,3);
  61.   writeln ('Direct array access works :');
  62.   for i := 0 to A.dimx-1 do with A.data[i*A.dimy+i] do begin
  63.     re := i;
  64.     im := i;
  65.   end;
  66.   A.print;
  67.  
  68.   A.Init (3,3);
  69.   //writeln ('Access via setter fails - all assignments to A are ignored without a warning');
  70.   for i := 0 to A.dimx-1 do
  71.   begin
  72.     // c := A[i, i];
  73.     with c do begin re := i; im := i; end;
  74.     A[i, i] := c;
  75.   end;
  76.   A.print;
  77.  
  78. end.
« Last Edit: August 20, 2019, 05:46:50 pm by julkas »
procedure mulu64(a, b: QWORD; out clo, chi: QWORD); assembler;
asm
  mov rax, a
  mov rdx, b
  mul rdx
  mov [clo], rax
  mov [chi], rdx
end;

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #2 on: August 21, 2019, 12:44:23 am »
I the event that you are not aware, there is a UNIT called "ucomplex" which has the record define and a few other things to work with real and imaginary numbers.

 I don't see your USES clause so I can't tell.
Number 1 at blue screen app creations!

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #3 on: August 21, 2019, 01:30:23 am »
I had to read a little deep to understand the problem here.

The issue is that using WITH you are given a complete object/Record to work with so you can change the values but you are only changing them within the local scope and only with a temporary object.

 The Getter only gives you a copy of the object , it does not give you a direct root to it.

 I've run into this many of times and the fix is to have the Getter/Setters with pointers to the object.
 THe Getter should return a Pointer to the object so that you can directly modify it.
Number 1 at blue screen app creations!

Thaddy

  • Hero Member
  • *****
  • Posts: 9276
Re: Failure to assign record fields via setter
« Reply #4 on: August 21, 2019, 09:20:25 am »
@Nitorami
The problem with with is indeed a bug, but to use the setter you need an assignment to a  temp complex.
I usually write a utility function for that.
Code: Pascal  [Select]
  1. function MakeComplex(const i,j:double):complex;inline;
  2. begin
  3.   Result.re :=i;
  4.   Result.im :=j;
  5. end;
  6.  
  7.   for i := 0 to A.dimx-1 do begin
  8.     A[i,i] := MakeComplex(i,i);
  9.   end;

« Last Edit: August 21, 2019, 12:18:53 pm by Thaddy »
also related to equus asinus.

PascalDragon

  • Hero Member
  • *****
  • Posts: 700
  • Compiler Developer
Re: Failure to assign record fields via setter
« Reply #5 on: August 21, 2019, 09:32:53 am »
The code below defines a 2D Matrix of complex numbers. The elements are stored in linear array "data", and can be accessed via getters/setters.

The setter expects type complex, hence it is not possible to make an assignment to the real or imaginary part only, e.g.
Code: Pascal  [Select]
  1. A[0,0].re := 1;
throws a compiler error "Argument cannot be assigned to".

That is understandable. But when I encapsulate this in a "with" construct, the assignment still does not work, but there is no compiler warning. Rather dangerous, and I suspect a bug. Please confirm.

FPC 3.0.4, win32
You can report it, but I don't know whether we can get the compiler to error out in that case as well...

Anyway: you need to assign record properties as a whole, not field-wise.

Thaddy

  • Hero Member
  • *****
  • Posts: 9276
Re: Failure to assign record fields via setter
« Reply #6 on: August 21, 2019, 01:09:07 pm »
Delphi also does not allow it on properties. Use something like my utility function. But the with thingy looks like bug, so I would report it.
also related to equus asinus.

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #7 on: August 21, 2019, 10:53:59 pm »
I don't see anything wrong with it, it's just a miss use and miss understanding of WITH.

A Getter is a function in the background, so no matter what you'll never be able to modify the source of the Record only the given from the local heap.

  Using WITH in this manner is exactly what it is designed to do, why should you expect compiler warnings or errors? The WITH has open the name space to that of the RECORD so now all members of that RECORD returned is available and of course in this case it's working with the currently local Heap version of it.

 Like I said, you can use a POITNER type return and get what you are looking for.

 If I have time I'll see if I can modify that code..

Number 1 at blue screen app creations!

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #8 on: August 21, 2019, 11:09:54 pm »
Code: Pascal  [Select]
  1.  
  2. program Project1;
  3.  
  4. {$mode objfpc}{$H+}
  5. {$modeswitch advancedrecords}
  6. uses
  7.   {$IFDEF UNIX}{$IFDEF UseCThreads}
  8.   cthreads,
  9.   {$ENDIF}{$ENDIF}
  10.   Classes
  11.   { you can add units after this };
  12.  
  13. type
  14.   pcomplex = ^complex;
  15.   complex = record re,im: double; end;
  16.  
  17. type TMatrix = record
  18.   strict private
  19.     sizex, sizey: longint;
  20.     function  GetItem (const x,y: longint): Pcomplex;
  21.     procedure PutItem (const x,y: longint; a: pcomplex);
  22.     property  Item    [const x,y:longint]: pcomplex read GetItem write PutItem; default;
  23.     procedure SetSize (const x,y: longint);
  24.   private
  25.     Data: array of complex;
  26.   public
  27.     property  dimx: longint read sizex;
  28.     property  dimy: longint read sizey;
  29.     procedure init  (const x,y: longint);
  30.     procedure print;
  31.   end;
  32.  
  33.  
  34. function  TMatrix.GetItem (const x,y: longint): pcomplex;
  35. begin
  36.   result := @Data[x*sizey+y];
  37. end;
  38.  
  39. procedure TMatrix.PutItem (const x,y: longint; a: pcomplex);
  40. begin
  41.   Data[x*sizey+y] := a^;
  42. end;
  43.  
  44. procedure TMatrix.SetSize (const x,y: longint);
  45. begin
  46.   SetLength (Data,x*y);
  47.   sizex := x; sizey := y;
  48. end;
  49.  
  50. procedure TMatrix.init (const x,y: longint);
  51. begin
  52.   Data := nil;
  53.   SetSize (x,y);
  54. end;
  55.  
  56. procedure TMatrix.print;
  57. var x,y: longint;
  58. begin
  59.   for y := 0 to dimy-1 do  begin
  60.     for x := 0 to dimx-1 do write (self[x,y]^.re:6:2,'',self[x,y]^.im:6:2,'j ');
  61.     writeln;
  62.   end;
  63.   writeln;
  64. end;
  65.  
  66.  
  67. var A: TMatrix;
  68.     i: longint;
  69.  
  70. begin
  71.   A.Init (3,3);
  72.   writeln ('Direct array access works :');
  73.   for i := 0 to A.dimx-1 do with A.data[i*A.dimy+i] do begin
  74.     re := i;
  75.     im := i;
  76.   end;
  77.   A.print;
  78.  
  79.   A.Init (3,3);
  80.   writeln ('Access via setter fails - all assignments to A are ignored without a warning');
  81.   for i := 0 to A.dimx-1 do with A[i,i]^ do begin
  82.     re := i;
  83.     im := i;
  84.   end;
  85.   A.print;
  86. Readln;
  87. end.
  88.  

Look At that code, you'll now get direct access to the array via a property.
Number 1 at blue screen app creations!

PascalDragon

  • Hero Member
  • *****
  • Posts: 700
  • Compiler Developer
Re: Failure to assign record fields via setter
« Reply #9 on: August 23, 2019, 09:33:14 am »
I don't see anything wrong with it, it's just a miss use and miss understanding of WITH.

A Getter is a function in the background, so no matter what you'll never be able to modify the source of the Record only the given from the local heap.

  Using WITH in this manner is exactly what it is designed to do, why should you expect compiler warnings or errors? The WITH has open the name space to that of the RECORD so now all members of that RECORD returned is available and of course in this case it's working with the currently local Heap version of it.
But it's misleading. SomeVar.SomeRecProp.x := 42 fails already, so it would be logical that with SomeVar.SomeRecProp do x := 42 fails as well. Otherwise this can lead to subtle bugs that are hard to catch as only the temporary variable is modified.

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #10 on: August 23, 2019, 11:38:25 pm »
I think a WARNING would be sufficient but it would need to be specific to this case.

 In all fairness I don't think you'd want the Setter to be called each time you set a member because the setter may assume all fields are set and then process it as though. So with two members for example, the setter may do something twice, first with incomplete data and the next
time completed data.

 


Number 1 at blue screen app creations!

PascalDragon

  • Hero Member
  • *****
  • Posts: 700
  • Compiler Developer
Re: Failure to assign record fields via setter
« Reply #11 on: August 24, 2019, 10:48:48 am »
I think a WARNING would be sufficient but it would need to be specific to this case.

 In all fairness I don't think you'd want the Setter to be called each time you set a member because the setter may assume all fields are set and then process it as though. So with two members for example, the setter may do something twice, first with incomplete data and the next
time completed data.
FPC is not automatically calling the setter anyway. Where do you get that from? FPC actively forbids changing the value of a record's field that has been returned by a property. Same should be true if the result is used in a with as that will result in consistent behavior.

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #12 on: August 24, 2019, 04:18:15 pm »
I didn't suggest that it was working that way, I was merely saying that  you would not want it to work that way in case someone suggested it.

 But..

 A compiler enhancement which I am sure does not work in Delphi could really move FPC to the present...
Code: Pascal  [Select]
  1.  SomeProperty := With SomeOtherProperty do
  2.   begin
  3.     // make all the changes here to the property body given.
  4.    //  do other unrelated things.
  5.   End;
  6.  
That would allow a visible entity to be assigned to a sub entity when the block completes.

 Now that would move FPC to the space age!  :)
« Last Edit: August 24, 2019, 04:21:33 pm by jamie »
Number 1 at blue screen app creations!

lucamar

  • Hero Member
  • *****
  • Posts: 2120
Re: Failure to assign record fields via setter
« Reply #13 on: August 24, 2019, 08:58:23 pm »
Code: Pascal  [Select]
  1.  SomeProperty := With SomeOtherProperty do
  2.   begin
  3.     // make all the changes here to the property body given.
  4.    //  do other unrelated things.
  5.   End;
  6.  


IMHO, a function works just as well and is more readable:

Code: Pascal  [Select]
  1. function DoSomethingWith(var AProperty: TWhateverType): TSomeOtherType;
  2. begin
  3.   with AProperty do begin
  4.     // make all the changes here to the property body given.
  5.     //  do other unrelated things.
  6.   end;
  7. end;
  8.  
  9.   {... lots of other code ...}
  10.  
  11.   SomeProperty := DoSomethingWith(SomeOtherProperty);

It'll also isolate in the function's body any side-effects the block of code has, which is nice for debugging.
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.2/2.0.4  - FPC 3.0.4 on:
(K|L)Ubuntu 12..16, Windows XP SP3, various DOSes.

jamie

  • Hero Member
  • *****
  • Posts: 2140
Re: Failure to assign record fields via setter
« Reply #14 on: August 24, 2019, 09:43:34 pm »
That just bloats the program, now you have to write a function..

This idea is not restricted to Properties,  you can also do this with OneREcord to another where
you make the changes in one record within the block and when you exit the block the complete
contents will be copied over to the first record, too...

Example
Code: Pascal  [Select]
  1.  RecordOneOfTypeA  := With RecordTwoOFTypeA do
  2.    begin
  3.       //Make all the changes to RecordTwo here
  4.      // do what ever else needed, like call methods etc...
  5.   End;
  6.  // RecordTwo is now edited and RecordOne now = RecordTwo;
  7.  
You don't need to use a block ether for a single…

ReocrdOneofTypeA := With RecordTwoOfTypeA do SomeMemberName := Whatever;

That will change only one item but update both.

Oh well, its a nice idea but you know what they say, "You can't always get what you ask for!"
Number 1 at blue screen app creations!