View Issue Details

IDProjectCategoryView StatusLast Update
0035580FPCFCLpublic2019-07-30 07:50
ReporterOndrej PokornyAssigned ToJonas Maebe 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionreopened 
Product Version3.3.1Product Buildr42056 
Target VersionFixed in Version3.3.1 
Summary0035580: Compiler picks up wrong overload for TMemoryStream.Write(TBytes, Longint);
DescriptionFPC 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 Reproduceprogram 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 InformationPatch attached.
TagsNo tags attached.
Fixed in Revision42058, 42118
FPCOldBugId
FPCTarget3.2.0
Attached Files
  • 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 }
    
  • 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 }
    

Relationships

related to 0035576 closedMichael Van Canneyt r42042-r42049 prevent Lazarus (i386) from starting 

Activities

Ondrej Pokorny

2019-05-13 22:13

reporter  

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 }

Michael Van Canneyt

2019-05-13 22:27

administrator   ~0116177

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 :(

Ondrej Pokorny

2019-05-13 22:57

reporter   ~0116179

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.

Ondrej Pokorny

2019-05-14 07:42

reporter   ~0116184

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 :/

Michael Van Canneyt

2019-05-14 07:50

administrator   ~0116187

I have forwarded this to the compiler people.

Ondrej Pokorny

2019-05-14 08:28

reporter   ~0116191

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.

Ondrej Pokorny

2019-05-14 08:28

reporter  

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 }

Michael Van Canneyt

2019-05-14 10:37

administrator   ~0116196

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.

Jonas Maebe

2019-05-25 14:33

manager   ~0116411

See the related bug report.

Issue History

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