View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0038122 | FPC | Compiler | public | 2020-11-23 18:20 | 2021-01-01 17:04 |
Reporter | Nitorami | Assigned To | Sven Barth | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | feedback | Resolution | open | ||
Platform | PC | OS | Win 10 | ||
Product Version | 3.2.0 | ||||
Fixed in Version | 3.3.1 | ||||
Summary | 0038122: Type Helper only modifies stack copy instead of self | ||||
Description | Under some circumstances, a type helper on a simple type such as integer or double seems to modify a stack object rather than self. | ||||
Steps To Reproduce | Run the attached program. It should subtract 1000 from element (1,1) of the Matrix but it doesn't. It reminds me of the behaviour of getters, which return a stack copy of the object, and modifications to the object are not written back. But this is different and should work IMHO. Same with 3.0.4. Also tried in the office where I have Lazarus with, I believe, FPC 3.0.2. Same behaviour. FPUtype does not seem to matter, neither other settings. | ||||
Tags | type helpers | ||||
Fixed in Revision | 47625,47747 | ||||
FPCOldBugId | |||||
FPCTarget | - | ||||
Attached Files |
|
parent of | 0038207 | new | Recent type helper fixes introduce bug with "with" syntax | |
Not all the children of this issue are yet resolved or closed. |
|
floathelper_issue.pas (1,442 bytes)
{$modeswitch advancedrecords} {$modeswitch typehelpers} type float = double; pfloat = ^float; type TFloatHelper = type helper for float procedure sub (const a: float); end; type TMatrix = record sx,sy: sizeint; procedure Init (x,y: sizeint; content: array of float); function GetAdr (x,y: sizeint): pfloat; procedure print; private data: array of float; end; procedure TFloatHelper.sub (const a: float); begin self := self-a; end; function TMatrix.GetAdr (x,y: sizeint): pfloat; begin result := @data[x*sy+y]; end; procedure TMatrix.Init (x,y: sizeint; content: array of float); var i: sizeint; begin sx :=x; sy :=y; Data := nil; SetLength (data, sx*sy); for i := 0 to sx*sy-1 do data[i] := content[i]; end; procedure TMatrix.print; var x,y: sizeint; begin for y := 0 to sy-1 do begin writeln; for x := 0 to sx-1 do begin write (GetAdr(x,y)^:2:2,' '); end; end; writeln; end; var A: TMatrix; px: pfloat; begin A.Init (2,2,[1,2,3,4]); A.print; A.GetAdr(1,1)^ := 0; //I can set an element like this... A.Print; px := A.GetAdr(1,1); px^.sub(100); //and this works as well. A.Print; A.GetAdr(1,1)^.sub(1000); //but that does not change the Matrix !?! A.print; end. |
|
As far as I remember, this is not regulated and depends on the implementation (the same behavior in Delphi). For functions that return records, a temporary copy is created first, and if it is not assigned, subsequent functions will work with it. Type helper has nothing to do with this. |
|
I can accept that for a getter. But if A.GetAdr()^ provides a temporary copy instead of the original content (which is only a double in this case), I would find that truly disturbing. Edit: The issue is not actually what the function (GetAdr) returns but that function sub "self := self-a" does not modify self. I cannot make sense of that. |
|
I was able to configure the Helper to do a PFloat instead of the Float. with this there is no need for the use of the "^" caret.. But without doing this using the Pointer dereference the value is getting lost on the stack with no recourse.. Although my altered version fixes this by eliminating the ^ in the operation I still think it should work if the helper is a PROCEDURE and not a function. Please look here for the modifications I made that seems to work, or maybe it does not … https://forum.lazarus.freepascal.org/index.php/topic,52248.0.html |
|
@Serge Anvarov: for temporary copies you're right, but we're talking about dereferencing a pointer here. And in my opinion the following two should behave the same:px := A.GetAdr(1,1); px^.sub(100); A.Print; A.GetAdr(1,1)^.sub(1000); A.print; |
|
It's the same here. The "sub" function works with a value, not a pointer, so the compiler first allocates a temporary variable, copies the data from the pointer to it, and then calls the function for it, |
|
That may well be the case but is hardly an excuse. How is the programmer supposed to know that a helper's "self" is a temporary variable, when working on a dereferenced typed pointer returned by a function, but not when working on a dereferenced typed pointer to a static variable ? |
|
Type helper. It's need Self. You provide a pointer only. |
|
No, I provide a dereferenced pointer. px^ is exactly the same variable as GetAdr(1,1)^, at the same memory location. Sub() receives this variable in both cases. What is the difference ? And what is the rationale behind that behaviour ? |
|
The types to which type helper is applied can be complex. The compiler uses a single safe strategy to avoid side effects (this is possible with managed types). |
|
Delphi 10.2 adjusts Self in all three cases. But even if Delphi would not, I would fix this for the sake of consistency. |
|
Please test and close if okay. |
|
Tested with svn revision 1:47629 and it works. Great, thank you ! |
|
It has just been reported on the forum that the code below does not print 20 as it should, but some arbitrary number. I can confirm this with fpc3.3.1 as of 29.11. (rev. 47629), so it looks as though a bug was introduced when fixing the original issue of this report, so I re-opened this. program strhlp; {$mode delphi} uses sysutils; type trec=record i:integer; end; var rec:trec; prec:^trec; begin rec.i:=20; prec:=@rec; writeln(prec.i.tostring); end. |
|
Info: It doesn't print some arbitrary number but it prints the memory location of rec. |
|
The regression should be fixed now in r47747. Please test and close if okay. |
|
Works now. |
|
Now, compiling the following with fpc3.3.1 for win32 gives an error Incompatible types: got "PInteger" expected "LongInt" (win32). I am not sure what the correct behaviour should be but it compiled ok in 3.2.0 and 3.0.4 program strhlp2; {$mode delphi} uses sysutils; var j:integer; begin j:=22; writeln(pinteger(@j)^.tostring); end. |
Date Modified | Username | Field | Change |
---|---|---|---|
2020-11-23 18:20 | Nitorami | New Issue | |
2020-11-23 18:20 | Nitorami | File Added: floathelper_issue.pas | |
2020-11-23 19:25 | Serge Anvarov | Note Added: 0127143 | |
2020-11-23 20:05 | Nitorami | Note Added: 0127144 | |
2020-11-23 20:15 | Nitorami | Note Edited: 0127144 | View Revisions |
2020-11-24 00:27 | jamie philbrook | Note Added: 0127148 | |
2020-11-24 13:49 | Sven Barth | Note Added: 0127156 | |
2020-11-24 21:10 | Serge Anvarov | Note Added: 0127160 | |
2020-11-25 17:24 | Nitorami | Note Added: 0127178 | |
2020-11-25 18:56 | Serge Anvarov | Note Added: 0127180 | |
2020-11-25 19:33 | Nitorami | Note Added: 0127181 | |
2020-11-25 21:05 | Serge Anvarov | Note Added: 0127187 | |
2020-11-25 22:59 | Sven Barth | Note Added: 0127190 | |
2020-11-25 23:01 | Sven Barth | Status | new => confirmed |
2020-11-25 23:01 | Sven Barth | FPCTarget | => - |
2020-11-25 23:01 | Sven Barth | Tag Attached: type helpers | |
2020-11-28 19:33 | Sven Barth | Assigned To | => Sven Barth |
2020-11-28 19:33 | Sven Barth | Status | confirmed => resolved |
2020-11-28 19:33 | Sven Barth | Resolution | open => fixed |
2020-11-28 19:33 | Sven Barth | Fixed in Version | => 3.3.1 |
2020-11-28 19:33 | Sven Barth | Fixed in Revision | => 47625 |
2020-11-28 19:33 | Sven Barth | Note Added: 0127247 | |
2020-11-29 17:15 | Nitorami | Note Added: 0127257 | |
2020-11-29 17:16 | Nitorami | Status | resolved => closed |
2020-12-09 17:36 | Nitorami | Status | closed => feedback |
2020-12-09 17:36 | Nitorami | Resolution | fixed => open |
2020-12-09 17:36 | Nitorami | Note Added: 0127493 | |
2020-12-09 19:03 | Rik van Kekem | Note Added: 0127497 | |
2020-12-10 07:13 | Sven Barth | Status | feedback => resolved |
2020-12-10 07:13 | Sven Barth | Resolution | open => fixed |
2020-12-10 07:13 | Sven Barth | Fixed in Revision | 47625 => 47625,47747 |
2020-12-10 07:13 | Sven Barth | Note Added: 0127503 | |
2020-12-10 18:04 | Nitorami | Status | resolved => closed |
2020-12-10 18:04 | Nitorami | Note Added: 0127510 | |
2020-12-14 18:51 | Nitorami | Status | closed => feedback |
2020-12-14 18:51 | Nitorami | Resolution | fixed => open |
2020-12-14 18:51 | Nitorami | Note Added: 0127611 | |
2021-01-01 17:04 | Sven Barth | Relationship added | parent of 0038207 |