View Issue Details

IDProjectCategoryView StatusLast Update
0033939FPCCompilerpublic2019-11-07 08:34
ReporterC WesternAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Platformx86_64OSlinuxOS Version
Product Version3.1.1Product Build 
Target VersionFixed in Version 
Summary0033939: Unexpected choice of overloaded function to call involving Single and Double
Descriptionfunction Max(a, b: Double): Double; overload;
begin
  WriteLn('Double');
  if a > b then Result := a else Result := b;
end;

function Max(a, b: Single): Single; overload;
begin
  WriteLn('Single');
  if a > b then Result := a else Result := b;
end;

var
  v1: Double;
  v2: Single;
  v3: Integer;
begin
  v1 := Pi;
  v2 := 0;
  WriteLn(v1);
  WriteLn(Max(v1,0));
  WriteLn(Max(v1,0.0));
  WriteLn(Max(v1,v2));
  WriteLn(Max(v1,v3));
end.

Prints:

 3.1415926535897931E+000
Single
 3.141592741E+00
Double
 3.1415926535897931E+000
Double
 3.1415926535897931E+000
Single
 3.141592741E+00

Max(Single,Single) is called in two cases in preference to Max(Double,Double). This implies precision is lost in two of the cases
Steps To ReproduceCompile and run the pro9gram above.
Additional InformationDelphi prints "Double" on all 4 calls. This has been discussed in the fpc-pascal mailing list.
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files
  • 33939_htypechk.pas.patch (1,326 bytes)
    Index: compiler/htypechk.pas
    ===================================================================
    --- compiler/htypechk.pas	(revision 43390)
    +++ compiler/htypechk.pas	(working copy)
    @@ -3230,6 +3230,7 @@
         function is_better_candidate(currpd,bestpd:pcandidate):integer;
           var
             res : integer;
    +        cmpexact : boolean;
           begin
             {
               Return values:
    @@ -3247,6 +3248,7 @@
               - (Smaller) Total of ordinal distance. For example, the distance of a word
                 to a byte is 65535-255=65280.
             }
    +        cmpexact:=false;
             if bestpd^.invalid then
              begin
                if currpd^.invalid then
    @@ -3287,6 +3289,7 @@
                               res:=(bestpd^.cl1_count-currpd^.cl1_count);
                               if (res=0) then
                                begin
    +                             cmpexact:=true;
                                  { more exact parameters? }
                                  res:=(currpd^.exact_count-bestpd^.exact_count);
                                  if (res=0) then
    @@ -3312,6 +3315,8 @@
                      end;
                   end;
                 end;
    +           if (not cmpexact) and (res<>0) and (currpd^.exact_count-bestpd^.exact_count>0) then
    +            res:=1;
              end;
             is_better_candidate:=res;
           end;
    
    33939_htypechk.pas.patch (1,326 bytes)
  • 33939_v2_htypechk.pas.patch (2,685 bytes)
    Index: compiler/htypechk.pas
    ===================================================================
    --- compiler/htypechk.pas	(revision 43409)
    +++ compiler/htypechk.pas	(working copy)
    @@ -52,6 +52,7 @@
              data         : tprocdef;
              wrongparaidx,
              firstparaidx : integer;
    +         exact_count_noconst,
              exact_count,
              equal_count,
              cl1_count,
    @@ -3123,7 +3124,11 @@
                   { increase correct counter }
                   case eq of
                     te_exact :
    +                 begin
                       inc(hp^.exact_count);
    +                  if not is_constnode(currpt.left) then
    +                    inc(hp^.exact_count_noconst);
    +                 end;
                     te_equal :
                       inc(hp^.equal_count);
                     te_convert_l1 :
    @@ -3263,6 +3268,10 @@
                res:=(bestpd^.coper_count-currpd^.coper_count);
                if (res=0) then
                 begin
    +             { more no constant exact count? }
    +             res:=(currpd^.exact_count_noconst-bestpd^.exact_count_noconst);
    +             if (res=0) then
    +             begin
                  { less cl6 parameters? }
                  res:=(bestpd^.cl6_count-currpd^.cl6_count);
                  if (res=0) then
    @@ -3271,22 +3280,27 @@
                     res:=(bestpd^.cl5_count-currpd^.cl5_count);
                     if (res=0) then
                      begin
    +                  writeln('cl5');
                       { less cl4 parameters? }
                       res:=(bestpd^.cl4_count-currpd^.cl4_count);
                       if (res=0) then
                        begin
    +                    writeln('cl4');
                         { less cl3 parameters? }
                         res:=(bestpd^.cl3_count-currpd^.cl3_count);
                         if (res=0) then
                          begin
    +                       writeln('cl3');
                            { less cl2 parameters? }
                            res:=(bestpd^.cl2_count-currpd^.cl2_count);
                            if (res=0) then
                             begin
    +                          writeln('cl2');
                               { less cl1 parameters? }
                               res:=(bestpd^.cl1_count-currpd^.cl1_count);
                               if (res=0) then
                                begin
    +                             writeln('cl1');
                                  { more exact parameters? }
                                  res:=(currpd^.exact_count-bestpd^.exact_count);
                                  if (res=0) then
    @@ -3311,6 +3325,7 @@
                        end;
                      end;
                   end;
    +              end;
                 end;
              end;
             is_better_candidate:=res;
    
  • 33939_v2_writelnx_htypechk.pas.patch (1,428 bytes)
    Index: compiler/htypechk.pas
    ===================================================================
    --- compiler/htypechk.pas	(revision 43409)
    +++ compiler/htypechk.pas	(working copy)
    @@ -52,6 +52,7 @@
              data         : tprocdef;
              wrongparaidx,
              firstparaidx : integer;
    +         exact_count_noconst,
              exact_count,
              equal_count,
              cl1_count,
    @@ -3123,7 +3124,11 @@
                   { increase correct counter }
                   case eq of
                     te_exact :
    +                 begin
                       inc(hp^.exact_count);
    +                  if not is_constnode(currpt.left) then
    +                    inc(hp^.exact_count_noconst);
    +                 end;
                     te_equal :
                       inc(hp^.equal_count);
                     te_convert_l1 :
    @@ -3263,6 +3268,10 @@
                res:=(bestpd^.coper_count-currpd^.coper_count);
                if (res=0) then
                 begin
    +             { more no constant exact count? }
    +             res:=(currpd^.exact_count_noconst-bestpd^.exact_count_noconst);
    +             if (res=0) then
    +             begin
                  { less cl6 parameters? }
                  res:=(bestpd^.cl6_count-currpd^.cl6_count);
                  if (res=0) then
    @@ -3311,6 +3320,7 @@
                        end;
                      end;
                   end;
    +              end;
                 end;
              end;
             is_better_candidate:=res;
    

Activities

Thaddy de Koning

2018-07-04 07:44

reporter   ~0109212

Last edited: 2018-07-04 07:53

View 2 revisions

The literal 0 is actually integer.
It seems the compiler is looking for:
function Max(a: double; b:integer): double; overload;
begin
  Write('integer :');
  if a > b then Result := a else Result := b;
end;

And then doesn't expand b to double, but shrinks the a instead and expands b to single. Strange indeed. The logic seems reversed.

Do-wan Kim

2019-11-05 01:35

reporter   ~0119065

I got all double by this patch.

33939_htypechk.pas.patch (1,326 bytes)
Index: compiler/htypechk.pas
===================================================================
--- compiler/htypechk.pas	(revision 43390)
+++ compiler/htypechk.pas	(working copy)
@@ -3230,6 +3230,7 @@
     function is_better_candidate(currpd,bestpd:pcandidate):integer;
       var
         res : integer;
+        cmpexact : boolean;
       begin
         {
           Return values:
@@ -3247,6 +3248,7 @@
           - (Smaller) Total of ordinal distance. For example, the distance of a word
             to a byte is 65535-255=65280.
         }
+        cmpexact:=false;
         if bestpd^.invalid then
          begin
            if currpd^.invalid then
@@ -3287,6 +3289,7 @@
                           res:=(bestpd^.cl1_count-currpd^.cl1_count);
                           if (res=0) then
                            begin
+                             cmpexact:=true;
                              { more exact parameters? }
                              res:=(currpd^.exact_count-bestpd^.exact_count);
                              if (res=0) then
@@ -3312,6 +3315,8 @@
                  end;
               end;
             end;
+           if (not cmpexact) and (res<>0) and (currpd^.exact_count-bestpd^.exact_count>0) then
+            res:=1;
          end;
         is_better_candidate:=res;
       end;
33939_htypechk.pas.patch (1,326 bytes)

Thaddy de Koning

2019-11-05 12:40

reporter   ~0119070

Sure you ran that to the test suite for the compiler?
*Literals* are assumed to be signed, even for reals.

I think this will break things.

Do-wan Kim

2019-11-05 13:06

reporter   ~0119071

Last edited: 2019-11-07 08:26

View 2 revisions

First patch choose overload function by definition order. It has bug.

Do-wan Kim

2019-11-07 08:30

reporter  

33939_v2_htypechk.pas.patch (2,685 bytes)
Index: compiler/htypechk.pas
===================================================================
--- compiler/htypechk.pas	(revision 43409)
+++ compiler/htypechk.pas	(working copy)
@@ -52,6 +52,7 @@
          data         : tprocdef;
          wrongparaidx,
          firstparaidx : integer;
+         exact_count_noconst,
          exact_count,
          equal_count,
          cl1_count,
@@ -3123,7 +3124,11 @@
               { increase correct counter }
               case eq of
                 te_exact :
+                 begin
                   inc(hp^.exact_count);
+                  if not is_constnode(currpt.left) then
+                    inc(hp^.exact_count_noconst);
+                 end;
                 te_equal :
                   inc(hp^.equal_count);
                 te_convert_l1 :
@@ -3263,6 +3268,10 @@
            res:=(bestpd^.coper_count-currpd^.coper_count);
            if (res=0) then
             begin
+             { more no constant exact count? }
+             res:=(currpd^.exact_count_noconst-bestpd^.exact_count_noconst);
+             if (res=0) then
+             begin
              { less cl6 parameters? }
              res:=(bestpd^.cl6_count-currpd^.cl6_count);
              if (res=0) then
@@ -3271,22 +3280,27 @@
                 res:=(bestpd^.cl5_count-currpd^.cl5_count);
                 if (res=0) then
                  begin
+                  writeln('cl5');
                   { less cl4 parameters? }
                   res:=(bestpd^.cl4_count-currpd^.cl4_count);
                   if (res=0) then
                    begin
+                    writeln('cl4');
                     { less cl3 parameters? }
                     res:=(bestpd^.cl3_count-currpd^.cl3_count);
                     if (res=0) then
                      begin
+                       writeln('cl3');
                        { less cl2 parameters? }
                        res:=(bestpd^.cl2_count-currpd^.cl2_count);
                        if (res=0) then
                         begin
+                          writeln('cl2');
                           { less cl1 parameters? }
                           res:=(bestpd^.cl1_count-currpd^.cl1_count);
                           if (res=0) then
                            begin
+                             writeln('cl1');
                              { more exact parameters? }
                              res:=(currpd^.exact_count-bestpd^.exact_count);
                              if (res=0) then
@@ -3311,6 +3325,7 @@
                    end;
                  end;
               end;
+              end;
             end;
          end;
         is_better_candidate:=res;

Do-wan Kim

2019-11-07 08:34

reporter   ~0119126

Sorry, I forget removing 'writeln'.

33939_v2_writelnx_htypechk.pas.patch (1,428 bytes)
Index: compiler/htypechk.pas
===================================================================
--- compiler/htypechk.pas	(revision 43409)
+++ compiler/htypechk.pas	(working copy)
@@ -52,6 +52,7 @@
          data         : tprocdef;
          wrongparaidx,
          firstparaidx : integer;
+         exact_count_noconst,
          exact_count,
          equal_count,
          cl1_count,
@@ -3123,7 +3124,11 @@
               { increase correct counter }
               case eq of
                 te_exact :
+                 begin
                   inc(hp^.exact_count);
+                  if not is_constnode(currpt.left) then
+                    inc(hp^.exact_count_noconst);
+                 end;
                 te_equal :
                   inc(hp^.equal_count);
                 te_convert_l1 :
@@ -3263,6 +3268,10 @@
            res:=(bestpd^.coper_count-currpd^.coper_count);
            if (res=0) then
             begin
+             { more no constant exact count? }
+             res:=(currpd^.exact_count_noconst-bestpd^.exact_count_noconst);
+             if (res=0) then
+             begin
              { less cl6 parameters? }
              res:=(bestpd^.cl6_count-currpd^.cl6_count);
              if (res=0) then
@@ -3311,6 +3320,7 @@
                    end;
                  end;
               end;
+              end;
             end;
          end;
         is_better_candidate:=res;

Issue History

Date Modified Username Field Change
2018-07-02 22:08 C Western New Issue
2018-07-04 07:44 Thaddy de Koning Note Added: 0109212
2018-07-04 07:53 Thaddy de Koning Note Edited: 0109212 View Revisions
2019-11-05 01:35 Do-wan Kim File Added: 33939_htypechk.pas.patch
2019-11-05 01:35 Do-wan Kim Note Added: 0119065
2019-11-05 12:40 Thaddy de Koning Note Added: 0119070
2019-11-05 13:06 Do-wan Kim Note Added: 0119071
2019-11-07 08:26 Do-wan Kim Note Edited: 0119071 View Revisions
2019-11-07 08:30 Do-wan Kim File Added: 33939_v2_htypechk.pas.patch
2019-11-07 08:34 Do-wan Kim File Added: 33939_v2_writelnx_htypechk.pas.patch
2019-11-07 08:34 Do-wan Kim Note Added: 0119126