View Issue Details

IDProjectCategoryView StatusLast Update
0034021FPCCompilerpublic2019-05-12 23:48
ReporterRyan JosephAssigned ToSven Barth 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.1.1Product Build 
Target VersionFixed in Version3.1.1 
Summary0034021: Implicit array in operator is treated like "set of"
DescriptionThe compiler treats implicit in operator overloads like sets thus giving an error that the operator is not overloaded. Happens on classes (as seen below) as well as records. I haven't tried all operators but at least + and := are affected.
Steps To Reproduceprogram test;

type
    TMyClass = class
    end;

operator + (left: TMyClass; right: array of integer): TMyClass; overload;
var
    i: integer;
begin
    for i in right do
        writeln('add ', i);
    result := left;
end;

var
    c: TMyClass;
begin
    c += [1, 2, 3]; // ERROR: Operator is not overloaded: "TMyClass" + "Set Of Byte"
end.
TagsNo tags attached.
Fixed in Revision39554
FPCOldBugId
FPCTarget
Attached Files
  • nadd.diff (858 bytes)
    1308c1308
    <          if not(is_dynamic_array(right.resultdef)) and is_array_constructor(left.resultdef) then
    ---
    >          if not(is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and is_array_constructor(left.resultdef) then
    1313c1313
    <          if not(is_dynamic_array(left.resultdef)) and is_array_constructor(right.resultdef) then
    ---
    >          if not(is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and is_array_constructor(right.resultdef) then
    1322c1322
    <          if is_dynamic_array(left.resultdef) and is_dynamic_array(right.resultdef) and
    ---
    >          if (is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and (is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and
    
    nadd.diff (858 bytes)
  • nadd.pas.patch (1,591 bytes)
    Index: nadd.pas
    ===================================================================
    --- nadd.pas	(revision 39512)
    +++ nadd.pas	(working copy)
    @@ -1305,12 +1305,12 @@
     
              { convert array constructors to sets, because there is no other operator
                possible for array constructors }
    -         if not(is_dynamic_array(right.resultdef)) and is_array_constructor(left.resultdef) then
    +         if not(is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and is_array_constructor(left.resultdef) then
               begin
                 arrayconstructor_to_set(left);
                 typecheckpass(left);
               end;
    -         if not(is_dynamic_array(left.resultdef)) and is_array_constructor(right.resultdef) then
    +         if not(is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and is_array_constructor(right.resultdef) then
               begin
                 arrayconstructor_to_set(right);
                 typecheckpass(right);
    @@ -1319,7 +1319,7 @@
              { allow operator overloading }
              hp:=self;
     
    -         if is_dynamic_array(left.resultdef) and is_dynamic_array(right.resultdef) and
    +         if (is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and (is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and
                  (nodetype=addn) and
                  (m_array_operators in current_settings.modeswitches) and
                  isbinaryoverloaded(hp,[ocf_check_non_overloadable,ocf_check_only]) then
    
    nadd.pas.patch (1,591 bytes)
  • nadd.pas.v2.patch (1,077 bytes)
    Index: nadd.pas
    ===================================================================
    --- nadd.pas	(revision 39512)
    +++ nadd.pas	(working copy)
    @@ -1305,12 +1305,12 @@
     
              { convert array constructors to sets, because there is no other operator
                possible for array constructors }
    -         if not(is_dynamic_array(right.resultdef)) and is_array_constructor(left.resultdef) then
    +         if not(is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef) or is_object(right.resultdef)) and is_array_constructor(left.resultdef) then
               begin
                 arrayconstructor_to_set(left);
                 typecheckpass(left);
               end;
    -         if not(is_dynamic_array(left.resultdef)) and is_array_constructor(right.resultdef) then
    +         if not(is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef) or is_object(left.resultdef)) and is_array_constructor(right.resultdef) then
               begin
                 arrayconstructor_to_set(right);
                 typecheckpass(right);
    
    nadd.pas.v2.patch (1,077 bytes)

Relationships

related to 0035061 resolvedSven Barth Operator overloads see literal arrays as "Set of ..." 

Activities

Thaddy de Koning

2018-07-23 18:23

reporter   ~0109641

Last edited: 2018-07-23 18:28

View 2 revisions

That syntax is unknown to the compiler.
  c +={this part-->} [1, 2, 3]
What lead you to believe this was possible in Pascal?

This is possible, though:
program test;
{$mode objfpc}
type
    TMyClass = integer; // CAN be a class but this is not JAVA!!!

operator + (left: TMyClass; right: array of integer): TMyClass; overload;
var
    i: integer;
begin
    for i in right do
        writeln('add ', i);
    result := left;
end;

var
    c: array of TMyClass;
    I:integer;
begin
    c := [100, 200, 300];
    for i in c do
      writeln(i);
end.

Akira1364

2018-07-25 21:27

reporter   ~0109674

Last edited: 2018-07-26 15:44

View 5 revisions

Appending to an array with "SomeArray += [blah, blah, blah]" actually did compile and work properly before the {arrayoperators} modeswitch was even introduced, for what it's worth. Obviously the "C" variable is a class in Ryan's example which is somewhat different, but that being said:

program test;

{$mode objfpc}
{$modeswitch arrayoperators}
{$coperators on}

type
    TMyClass = class
    end;

operator + (left: TMyClass; right: array of integer): TMyClass; overload;
var
    i: integer;
begin
    for i in right do
        writeln('add ', i);
    result := left;
end;

var
    c: TMyClass;
    X: array of integer = (1, 2, 3);
begin
    X := X + [1, 2, 3]; //Works fine!
    X += [1, 2, 3]; //Works fine!
    C += [1, 2, 3]; // ERROR: Operator is not overloaded: "TMyClass" + "Set Of Byte"
end.

Seems inconsistent to me. IMO they should all work.

Akira1364

2018-07-26 22:05

reporter  

nadd.diff (858 bytes)
1308c1308
<          if not(is_dynamic_array(right.resultdef)) and is_array_constructor(left.resultdef) then
---
>          if not(is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and is_array_constructor(left.resultdef) then
1313c1313
<          if not(is_dynamic_array(left.resultdef)) and is_array_constructor(right.resultdef) then
---
>          if not(is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and is_array_constructor(right.resultdef) then
1322c1322
<          if is_dynamic_array(left.resultdef) and is_dynamic_array(right.resultdef) and
---
>          if (is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and (is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and
nadd.diff (858 bytes)

Akira1364

2018-07-26 22:10

reporter   ~0109694

Last edited: 2018-07-26 22:11

View 2 revisions

I'm pretty sure I've figured this out. The issue was between line 1308 and 1326 of "nadd.pas." Basically it was only checking if the left and right hand parameters were arrays, and if not then converting them to sets.

I've attached a simple diff with the necessary changes (that also accounts for overloaded records, not only classes.) With it applied, Ryan's example works fine for me (although you'll probably want to add calls to create and free for the instance of TMyClass.)

Cyrax

2018-07-27 00:19

reporter   ~0109696

@Akira1364: Can you make a proper diff with using svn tools? And make your diff against the trunk version of FPC.

http://wiki.freepascal.org/Creating_A_Patch

Akira1364

2018-07-27 01:24

reporter  

nadd.pas.patch (1,591 bytes)
Index: nadd.pas
===================================================================
--- nadd.pas	(revision 39512)
+++ nadd.pas	(working copy)
@@ -1305,12 +1305,12 @@
 
          { convert array constructors to sets, because there is no other operator
            possible for array constructors }
-         if not(is_dynamic_array(right.resultdef)) and is_array_constructor(left.resultdef) then
+         if not(is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and is_array_constructor(left.resultdef) then
           begin
             arrayconstructor_to_set(left);
             typecheckpass(left);
           end;
-         if not(is_dynamic_array(left.resultdef)) and is_array_constructor(right.resultdef) then
+         if not(is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and is_array_constructor(right.resultdef) then
           begin
             arrayconstructor_to_set(right);
             typecheckpass(right);
@@ -1319,7 +1319,7 @@
          { allow operator overloading }
          hp:=self;
 
-         if is_dynamic_array(left.resultdef) and is_dynamic_array(right.resultdef) and
+         if (is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef)) and (is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef)) and
              (nodetype=addn) and
              (m_array_operators in current_settings.modeswitches) and
              isbinaryoverloaded(hp,[ocf_check_non_overloadable,ocf_check_only]) then
nadd.pas.patch (1,591 bytes)

Akira1364

2018-07-27 01:25

reporter   ~0109697

@Cyrax: I've just uploaded one.

Akira1364

2018-07-27 18:42

reporter   ~0109714

Ok, it turned out that only the changes I made beginning at line 1308 were actually necessary, and not the ones at line 1322.

I've uploaded a revised version of the patch (which additionally checks for TP Object types, something I overlooked before) that I'm confident is the correct, complete solution.

Akira1364

2018-07-27 18:42

reporter  

nadd.pas.v2.patch (1,077 bytes)
Index: nadd.pas
===================================================================
--- nadd.pas	(revision 39512)
+++ nadd.pas	(working copy)
@@ -1305,12 +1305,12 @@
 
          { convert array constructors to sets, because there is no other operator
            possible for array constructors }
-         if not(is_dynamic_array(right.resultdef)) and is_array_constructor(left.resultdef) then
+         if not(is_dynamic_array(right.resultdef) or is_class(right.resultdef) or is_record(right.resultdef) or is_object(right.resultdef)) and is_array_constructor(left.resultdef) then
           begin
             arrayconstructor_to_set(left);
             typecheckpass(left);
           end;
-         if not(is_dynamic_array(left.resultdef)) and is_array_constructor(right.resultdef) then
+         if not(is_dynamic_array(left.resultdef) or is_class(left.resultdef) or is_record(left.resultdef) or is_object(left.resultdef)) and is_array_constructor(right.resultdef) then
           begin
             arrayconstructor_to_set(right);
             typecheckpass(right);
nadd.pas.v2.patch (1,077 bytes)

Sven Barth

2018-08-03 17:26

manager   ~0109860

Thank you for the patch, but that would have failed with other types that could be used for the overload (e.g. Integer). Thus I fixed it in a more general way. ;)

Please test and close if okay.

Ryan Joseph

2019-05-12 23:48

reporter   ~0116152

I just realized I forgot to close this so I'm doing it now.

Issue History

Date Modified Username Field Change
2018-07-23 17:36 Ryan Joseph New Issue
2018-07-23 18:23 Thaddy de Koning Note Added: 0109641
2018-07-23 18:28 Thaddy de Koning Note Edited: 0109641 View Revisions
2018-07-25 21:27 Akira1364 Note Added: 0109674
2018-07-26 02:09 Akira1364 Note Edited: 0109674 View Revisions
2018-07-26 02:10 Akira1364 Note Edited: 0109674 View Revisions
2018-07-26 02:10 Akira1364 Note Edited: 0109674 View Revisions
2018-07-26 15:44 Akira1364 Note Edited: 0109674 View Revisions
2018-07-26 22:05 Akira1364 File Added: nadd.diff
2018-07-26 22:10 Akira1364 Note Added: 0109694
2018-07-26 22:11 Akira1364 Note Edited: 0109694 View Revisions
2018-07-27 00:19 Cyrax Note Added: 0109696
2018-07-27 01:24 Akira1364 File Added: nadd.pas.patch
2018-07-27 01:25 Akira1364 Note Added: 0109697
2018-07-27 18:42 Akira1364 Note Added: 0109714
2018-07-27 18:42 Akira1364 File Added: nadd.pas.v2.patch
2018-08-03 17:26 Sven Barth Fixed in Revision => 39554
2018-08-03 17:26 Sven Barth Note Added: 0109860
2018-08-03 17:26 Sven Barth Status new => resolved
2018-08-03 17:26 Sven Barth Fixed in Version => 3.1.1
2018-08-03 17:26 Sven Barth Resolution open => fixed
2018-08-03 17:26 Sven Barth Assigned To => Sven Barth
2019-02-15 15:12 Sven Barth Relationship added related to 0035061
2019-05-12 23:48 Ryan Joseph Status resolved => closed
2019-05-12 23:48 Ryan Joseph Note Added: 0116152