View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0035580 | FPC | FCL | public | 2019-05-13 22:13 | 2019-07-30 07:50 |
| Reporter | Ondrej Pokorny | Assigned To | Jonas Maebe | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | reopened | ||
| Product Version | 3.3.1 | Product Build | r42056 | ||
| Target Version | Fixed in Version | 3.3.1 | |||
| Summary | 0035580: Compiler picks up wrong overload for TMemoryStream.Write(TBytes, Longint); | ||||
| Description | FPC calls the "Write(const Buffer; Count: Longint)" overload even when the "Write(const Buffer: TBytes; Count: Longint)" should be called. This is because the TMemoryStream.Write declaration misses the "overload" keyword. See docs: https://www.freepascal.org/docs-html/ref/refsu79.html "There is only one case where the overload modifier is mandatory: if a function must be overloaded that resides in another unit." The docs should be enhanced as well - it is mandatory for object methods across different classes; not only functions across different units. | ||||
| Steps To Reproduce | program Project1; {$mode objfpc} uses SysUtils, Classes; var B1, B2: TBytes; S: TMemoryStream; I: Integer; begin B1 := [1, 2, 3, 4, 5]; S := TMemoryStream.Create; S.Write(B1, Length(B1)); // <<< BUG: wrong overload is called S.Position := 0; SetLength(B2, Length(B1)); S.ReadData(B2, Length(B2)); S.Free; if not(Length(B2) = Length(B1)) then Halt(999); for I := Low(B1) to High(B1) do if B1[I] <> B2[I] then Halt(I); Writeln('OK'); Readln; end. | ||||
| Additional Information | Patch attached. | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | 42058, 42118 | ||||
| FPCOldBugId | |||||
| FPCTarget | 3.2.0 | ||||
| Attached Files |
| ||||
| related to | 0035576 | closed | Michael Van Canneyt | r42042-r42049 prevent Lazarus (i386) from starting |
|
|
readwrite-overload-01.patch (1,587 bytes)
Index: rtl/objpas/classes/classesh.inc
===================================================================
--- rtl/objpas/classes/classesh.inc (revision 42056)
+++ rtl/objpas/classes/classesh.inc (working copy)
@@ -899,13 +899,13 @@
function WriteMaxSizeData(Const Buffer; aSize,aCount : NativeInt) : NativeInt;
Procedure WriteExactSizeData(Const Buffer; aSize,aCount : NativeInt);
public
- function Read(var Buffer; Count: Longint): Longint; virtual;
+ function Read(var Buffer; Count: Longint): Longint; virtual; overload;
function Read(Buffer: TBytes; Count: Longint): Longint; overload;
function Read(Buffer : TBytes; aOffset, Count: Longint): Longint; overload;
function Write(const Buffer: TBytes; Offset, Count: Longint): Longint; overload;
function Write(const Buffer: TBytes; Count: Longint): Longint; overload;
- function Write(const Buffer; Count: Longint): Longint; virtual;
+ function Write(const Buffer; Count: Longint): Longint; virtual; overload;
function Seek(Offset: Longint; Origin: Word): Longint; virtual; overload;
function Seek(const Offset: Int64; Origin: TSeekOrigin): Int64; virtual; overload;
@@ -1168,7 +1168,7 @@
procedure LoadFromStream(Stream: TStream);
procedure LoadFromFile(const FileName: string);
procedure SetSize({$ifdef CPU64}const NewSize: Int64{$else}NewSize: LongInt{$endif}); override;
- function Write(const Buffer; Count: LongInt): LongInt; override;
+ function Write(const Buffer; Count: LongInt): LongInt; override; overload;
end;
{ TBytesStream }
|
|
|
Added overload to all versions of Read/Write. Potentially all TStream descendents need the 'overload' now. Very strange, since I cannot imagine Delphi doing such an invasive change without tons of bugreports :( |
|
|
Oh, thanks - you added also overloads in other descendants, which I forgot about. > Very strange, since I cannot imagine Delphi doing such an invasive change without tons of bugreports :( To me it looks like an FPC compiler bug. I'll ask on the mailing list. |
|
|
OK, it's not that easy after all :/ FPC behaves differently within compiler modes. The objfpc mode calls the TBytes-overload if a PByteArray is dereferenced whereas the Delphi mode calls the const-overload: program Project1; {$ifdef FPC} {$mode objfpc} // {$mode delphi} works fine {$else} {$APPTYPE CONSOLE} {$endif} uses SysUtils, Classes; var B1: TBytes; PB: PByteArray; S: TMemoryStream; begin B1 := [1, 2, 3, 4, 5]; S := TMemoryStream.Create; S.Write(B1, Length(B1)); // <<< BUG (1) - fixed: wrong overload is called - Delphi calls Write(TBytes) GetMem(PB, Length(B1)); Move(B1[0], PB^, Length(B1)); S.Write(PB^, Length(B1)); // <<< BUG (2): wrong overload is called - Delphi calls Write(const), FPC calls Write(TBytes) end. That makes a difference :/ |
|
|
I have forwarded this to the compiler people. |
|
|
Thank you. I tested a little bit more and the overrides do not need the overload keyword if the original virtual method has it. The overrides inherit the overload automatically. That means the Write() virtual methods would need an overload and all the overrides don't need it. See readwrite-overload-02.patch for what I mean. The above doesn't change anything about the overload resolving differences between modes. I can imagine these differences are as-designed. |
|
|
readwrite-overload-02.patch (2,673 bytes)
Index: rtl/objpas/classes/classesh.inc
===================================================================
--- rtl/objpas/classes/classesh.inc (revision 42061)
+++ rtl/objpas/classes/classesh.inc (working copy)
@@ -905,7 +905,7 @@
function Write(const Buffer: TBytes; Offset, Count: Longint): Longint; overload;
function Write(const Buffer: TBytes; Count: Longint): Longint; overload;
- function Write(const Buffer; Count: Longint): Longint; virtual;
+ function Write(const Buffer; Count: Longint): Longint; virtual; overload;
function Seek(Offset: Longint; Origin: Word): Longint; virtual; overload;
function Seek(const Offset: Int64; Origin: TSeekOrigin): Int64; virtual; overload;
@@ -1081,8 +1081,8 @@
function GetIStream: IStream;
public
constructor Create(const Stream: IStream);
- function Read(var Buffer; Count: Longint): Longint; override; overload;
- function Write(const Buffer; Count: Longint): Longint; override; overload;
+ function Read(var Buffer; Count: Longint): Longint; override;
+ function Write(const Buffer; Count: Longint): Longint; override;
function Seek(const Offset: int64; Origin: TSeekOrigin): int64; override;
procedure Check(err:integer); virtual;
end;
@@ -1115,8 +1115,8 @@
procedure SetSize(const NewSize: Int64); override;
public
constructor Create(AHandle: THandle);
- function Read(var Buffer; Count: Longint): Longint; override; overload;
- function Write(const Buffer; Count: Longint): Longint; override; overload;
+ function Read(var Buffer; Count: Longint): Longint; override;
+ function Write(const Buffer; Count: Longint): Longint; override;
function Seek(const Offset: Int64; Origin: TSeekOrigin): Int64; override;
property Handle: THandle read FHandle;
end;
@@ -1145,7 +1145,7 @@
function GetPosition: Int64; Override;
procedure SetPointer(Ptr: Pointer; ASize: PtrInt);
public
- function Read(var Buffer; Count: LongInt): LongInt; override; overload;
+ function Read(var Buffer; Count: LongInt): LongInt; override;
function Seek(const Offset: Int64; Origin: TSeekOrigin): Int64; override;
procedure SaveToStream(Stream: TStream);
procedure SaveToFile(const FileName: string);
@@ -1168,7 +1168,7 @@
procedure LoadFromStream(Stream: TStream);
procedure LoadFromFile(const FileName: string);
procedure SetSize({$ifdef CPU64}const NewSize: Int64{$else}NewSize: LongInt{$endif}); override;
- function Write(const Buffer; Count: LongInt): LongInt; override; overload;
+ function Write(const Buffer; Count: LongInt): LongInt; override;
end;
{ TBytesStream }
|
|
|
I have applied the 2nd patch in 42062. I will wait for the compiler people to respond with some explanation of the 'why' before closing this issue. |
|
|
See the related bug report. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2019-05-13 22:13 | Ondrej Pokorny | New Issue | |
| 2019-05-13 22:13 | Ondrej Pokorny | File Added: readwrite-overload-01.patch | |
| 2019-05-13 22:27 | Michael Van Canneyt | Assigned To | => Michael Van Canneyt |
| 2019-05-13 22:27 | Michael Van Canneyt | Status | new => resolved |
| 2019-05-13 22:27 | Michael Van Canneyt | Resolution | open => fixed |
| 2019-05-13 22:27 | Michael Van Canneyt | Fixed in Version | => 3.3.1 |
| 2019-05-13 22:27 | Michael Van Canneyt | Fixed in Revision | => 42058 |
| 2019-05-13 22:27 | Michael Van Canneyt | FPCTarget | => 3.2.0 |
| 2019-05-13 22:27 | Michael Van Canneyt | Note Added: 0116177 | |
| 2019-05-13 22:57 | Ondrej Pokorny | Status | resolved => closed |
| 2019-05-13 22:57 | Ondrej Pokorny | Note Added: 0116179 | |
| 2019-05-14 07:42 | Ondrej Pokorny | Status | closed => feedback |
| 2019-05-14 07:42 | Ondrej Pokorny | Resolution | fixed => reopened |
| 2019-05-14 07:42 | Ondrej Pokorny | Note Added: 0116184 | |
| 2019-05-14 07:50 | Michael Van Canneyt | Note Added: 0116187 | |
| 2019-05-14 08:14 | Michael Van Canneyt | Relationship added | related to 0035576 |
| 2019-05-14 08:28 | Ondrej Pokorny | Note Added: 0116191 | |
| 2019-05-14 08:28 | Ondrej Pokorny | Status | feedback => assigned |
| 2019-05-14 08:28 | Ondrej Pokorny | File Added: readwrite-overload-02.patch | |
| 2019-05-14 10:37 | Michael Van Canneyt | Note Added: 0116196 | |
| 2019-05-25 14:33 | Jonas Maebe | Status | assigned => resolved |
| 2019-05-25 14:33 | Jonas Maebe | Fixed in Revision | 42058 => 42058, 42118 |
| 2019-05-25 14:33 | Jonas Maebe | Note Added: 0116411 | |
| 2019-05-25 14:33 | Jonas Maebe | Assigned To | Michael Van Canneyt => Jonas Maebe |
| 2019-05-25 14:33 | Jonas Maebe | Description Updated | View Revisions |
| 2019-05-25 14:33 | Jonas Maebe | Steps to Reproduce Updated | View Revisions |
| 2019-07-30 07:50 | Ondrej Pokorny | Status | resolved => closed |
