View Issue Details

IDProjectCategoryView StatusLast Update
0025607FPCCompilerpublic2017-03-13 14:29
ReporterMaciej IzakAssigned ToJonas Maebe 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.7.1Product Build26466 
Target VersionFixed in Version3.1.1 
Summary0025607: Can't determine which overloaded function to call for override constructor
DescriptionCan't determine which overloaded function to call for override constructor. In Delphi all is ok. I see no reason for that error (now to compile example I need to reimplement all constructors from TA - in normal code this is a big problem!):
---program---
{$MODE DELPHI}

type
  TA = class
  public
    constructor Create(A: Integer = 0); virtual; overload;
    constructor Create(A: string); virtual; overload;
  end;

  TB = class(TA)
  public
    constructor Create(A: string); override;
  end;

constructor TA.Create(A: Integer);
begin
end;

constructor TA.Create(A: string);
begin
end;

constructor TB.Create(A: string);
begin
end;

var
  B: TB;
begin
  B := TB.Create; // Error: Can't determine which overloaded function to call
end.
TagsNo tags attached.
Fixed in Revision35089
FPCOldBugId
FPCTarget
Attached Files
  • r13.lpr (533 bytes)
  • Test_01.zip (73,733 bytes)
  • Test_01.pdf (68,997 bytes)
  • Test_02.zip (74,983 bytes)
  • Test_02.pdf (70,186 bytes)
  • FPC_BlackList.zip (66,997 bytes)
  • FPC_BlackList.pdf (54,842 bytes)
  • FPC_BlackList_No_Mem_Leak.zip (3,176 bytes)
  • compiler_patch_distance_for_black_list_19.09.2016.patch (4,080 bytes)
    Index: compiler/htypechk.pas
    ===================================================================
    --- compiler/htypechk.pas	(revision 34540)
    +++ compiler/htypechk.pas	(working copy)
    @@ -73,6 +73,7 @@
             procedure collect_overloads_in_struct(structdef:tabstractrecorddef;ProcdefOverloadList:TFPObjectList;searchhelpers,anoninherited:boolean;spezcontext:tspecializationcontext);
             procedure collect_overloads_in_units(ProcdefOverloadList:TFPObjectList; objcidcall,explicitunit: boolean;spezcontext:tspecializationcontext);
             procedure create_candidate_list(ignorevisibility,allowdefaultparas,objcidcall,explicitunit,searchhelpers,anoninherited:boolean;spezcontext:tspecializationcontext);
    +        procedure calc_distance(st_root:tsymtable;objcidcall: boolean);
             function  proc_add(st:tsymtable;pd:tprocdef;objcidcall: boolean):pcandidate;
             function  maybe_specialize(var pd:tprocdef;spezcontext:tspecializationcontext):boolean;
           public
    @@ -2517,10 +2518,73 @@
                   end;
               end;
     
    +        calc_distance(st,objcidcall);
    +
             ProcdefOverloadList.Free;
           end;
     
     
    +    procedure tcallcandidates.calc_distance(st_root: tsymtable; objcidcall: boolean);
    +      var
    +        pd:tprocdef;
    +        candidate:pcandidate;
    +        objdef: tobjectdef;
    +        st: tsymtable;
    +      begin
    +        st:=nil;
    +        if (st_root = nil) or (st_root.defowner = nil) or (st_root.defowner.typ <> objectdef) then
    +          st := st_root
    +        else
    +          repeat
    +            candidate := FCandidateProcs;
    +
    +            while candidate <> nil do
    +              begin
    +                pd:=candidate^.data;
    +                if pd.owner = st_root then
    +                begin
    +                  st:=st_root;
    +                  break;
    +                end;
    +                candidate := candidate^.next;
    +              end;
    +            if st=nil then
    +              begin
    +                if st_root.defowner=nil then
    +                  Internalerror(201605301);
    +
    +                if tobjectdef(st_root.defowner).childof = nil then
    +                  begin
    +                    st:=st_root;
    +                    break;
    +                  end;
    +
    +                st_root:=tobjectdef(st_root.defowner).childof.symtable;
    +              end;
    +          until st<>nil;
    +
    +        candidate:=FCandidateProcs;
    +        while candidate <> nil do
    +          begin
    +            pd:=candidate^.data;
    +            { Give a small penalty for overloaded methods not in
    +              defined the current class/unit }
    +            {  when calling Objective-C methods via id.method, then the found
    +               procsym will be inside an arbitrary ObjectSymtable, and we don't
    +               want togive the methods of that particular objcclass precedence over
    +               other methods, so instead check against the symtable in which this
    +               objcclass is defined }
    +            if objcidcall then
    +              st:=st.defowner.owner;
    +
    +            if (st<>pd.owner) then
    +              candidate^.ordinal_distance:=candidate^.ordinal_distance+1.0;
    +
    +          candidate := candidate^.next;
    +        end;
    +      end;
    +
    +
         function tcallcandidates.proc_add(st:tsymtable;pd:tprocdef;objcidcall: boolean):pcandidate;
           var
             defaultparacnt : integer;
    @@ -2548,17 +2612,6 @@
                    dec(result^.firstparaidx,defaultparacnt);
                  end;
              end;
    -        { Give a small penalty for overloaded methods not in
    -          defined the current class/unit }
    -        {  when calling Objective-C methods via id.method, then the found
    -           procsym will be inside an arbitrary ObjectSymtable, and we don't
    -           want togive the methods of that particular objcclass precedence over
    -           other methods, so instead check against the symtable in which this
    -           objcclass is defined }
    -        if objcidcall then
    -          st:=st.defowner.owner;
    -        if (st<>pd.owner) then
    -          result^.ordinal_distance:=result^.ordinal_distance+1.0;
           end;
     
     
    

Relationships

related to 0027206 resolvedSven Barth [Patch] Christmas gift by FreeSparta : Generics.Collections 
related to 0031081 resolvedJonas Maebe When crosscompiling from Windows towards Darwin cocoa an exception occurs 

Activities

Maciej Izak

2014-01-25 13:40

reporter  

r13.lpr (533 bytes)

Jonas Maebe

2014-01-25 14:01

manager   ~0072657

That's because both "tobject.create()" and "ta.create(a: integer=0)" are in scope ("overload" tells the compiler to keep looking in parent classes for potential candidates). If Delphi compiles this, I think it's a bug in Delphi.

Maciej Izak

2014-01-25 14:55

reporter   ~0072658

Last edited: 2014-01-25 14:58

View 2 revisions

I think it's not a bug, but the desired behavior. Look at this scenario:
---code begin---
constructor Create(ACapacity: Integer = 0); overload;
constructor Create(ACapacity: Integer; const AComparer: IEqualityComparer<TKey>); virtual; overload;
constructor Create(const AComparer: IEqualityComparer<TKey>); overload;
constructor Create(ACollection: TEnumerable<TDictionaryPair>); virtual; overload;
constructor Create(ACollection: TEnumerable<TDictionaryPair>; const AComparer: IEqualityComparer<TKey>); virtual; overload;
---code end---

When I need override only:
---code begin---
constructor Create(ACapacity: Integer; const AComparer: IEqualityComparer<TKey>); virtual; overload;
---code end---

then I must reimplement all (!!!) constructors in descendant class (this unnecessarily complicates the code in many of my classes), what does not make sense... This one thing is well implemented in Delphi ;).

Jonas Maebe

2014-01-25 16:56

manager   ~0072663

I'm simply talking about the fact that "overload" in Delphi means:
a) you can have multiple methods with the same name in the current class
b) the compiler should also search for other methods with the same name in the parent class

Then all of these methods are grouped together and the compiler picks the most appropriate one. If there are two methods that are valid if you don't specify any parameter, how is the compiler supposed to pick the most appropriate one?

Or maybe there's a special rule in case the compiler has to choose between a method without parameters and one with a default parameter, or maybe something specific regarding methods in the current class and in the parent class. But then you should point to the explicit rules or write test programs that demonstrate these rules (also with regular functions, maybe overloaded across multiple units). Without that it's impossible to change anything, because it would just be a hack that may help your specific scenario, but break things in other cases.

Thaddy de Koning

2014-01-25 17:21

reporter   ~0072664

Last edited: 2014-01-25 17:25

View 3 revisions

That code does not compile in any delphi version upto and including Delphi 2007.
I have no access today to a XE set of compilers, but this example code does NOT compile in D4..D2007. I tested 7,2006,2007 of them.
If it does in newer versions, it's a bug because as Jonas said, the default constructor of Tobject is also in scope. So the code is ambiguous.

IF the code compiles in newer delphi's plz test both dcc32 and dcc64 because your results may very because of a completely different back-end.

Maciej Izak

2014-01-25 18:01

reporter   ~0072668

Last edited: 2014-01-25 18:02

View 2 revisions

@Jonas Maebe
ok I will create more tests (for XE2, XE4 and Delphi 2007). I don't want any hack. At this moment In Delphi XE2 constructors (I need to create test for methods too) with single parameter with optionally default value just reintroduce "Create" from TObject. It is logic. Different behavior compiler will create serious problems as that described in the report.

TA.Create; // will call TA.Create(0); There is no way to call Create from TObject in this way. "Create" from TObject should not be taken as possible option.

@Thaddy de Koning
Ok. Ouh. With XE2 it is compile with small change (for 64 and 32 bits). I just forgotten that in Delphi "overload" is before "virtual". I will create test for Delphi 2007 too. Since I wrote the library Generics.*, I do not use Delphi anymore :).

Maciej Izak

2014-01-26 12:04

reporter   ~0072686

Last edited: 2014-01-26 12:04

View 2 revisions

Done. ~ 700 tests. Detailed information in the "test_01.pdf" and "test_01.zip". For regular functions all works as expected - no special rules. But for constructors, methods and class methods there is special rule. That is definitely (!) not a Delphi bug! Code works the same for each version of Delphi (2007, XE2 32 and 64, XE4 32 and 64).

At first I was angry that I was doing pointless tests, but I discovered an interesting thing. There are two groups of rules:

1. For constructors and class methods
2. For methods

------rule1 "constructors"------
  TA = class public
    constructor Create(A: Integer = 0); overload; virtual;
  end;

  TB = class(TA) public
    constructor Create(A: Integer); overload; override;
  end;
...
  TB.Create(); // will call TA.Create(A: Integer = 0)
------end------

------rule1 "class methods"------
  TA = class public
    class procedure Foo(A: Integer = 0); overload; virtual;
  end;

  TB = class(TA) public
    class procedure Foo(A: Integer); overload; override;
  end;
...
  TB.Foo(); // will call TA.Foo(A: Integer = 0)
------end------

------rule2 "methods"------
  TA = class public
    procedure Foo(A: Integer = 0); overload; virtual;
  end;

  TB = class(TA) public
    procedure Foo(A: Integer); overload; override;
  end;
...
var
  B: TB;
begin
  B := TB.Create();
  B.Foo(); // will call TB.Foo(A: Integer)
------end------

Important note: there is no side effect (no code stops compiling, will work the same way), if we allow the compilation of this code in the FPC.

Maciej Izak

2014-01-26 12:05

reporter  

Test_01.zip (73,733 bytes)

Maciej Izak

2014-01-26 12:05

reporter  

Test_01.pdf (68,997 bytes)

Maciej Izak

2014-01-26 13:57

reporter   ~0072690

Last edited: 2014-01-26 14:01

View 2 revisions

Warning: If normal method is without override then method works as is described in rule1.

Maciej Izak

2014-01-26 15:09

reporter  

Test_02.zip (74,983 bytes)

Maciej Izak

2014-01-26 15:09

reporter  

Test_02.pdf (70,186 bytes)

Maciej Izak

2014-01-26 15:09

reporter   ~0072692

Test_02 dedicated for "no override" version.

Maciej Izak

2014-01-26 18:31

reporter  

FPC_BlackList.zip (66,997 bytes)

Maciej Izak

2014-01-26 18:32

reporter  

FPC_BlackList.pdf (54,842 bytes)

Maciej Izak

2014-01-26 18:36

reporter   ~0072710

Last edited: 2014-01-26 19:04

View 3 revisions

Now I am sure that in Delphi is no bug, but in FreePascal. What came out of my tests is included in FPC_BlackList.zip & FPC_BlackList.pdf. Pure object oriented design and VMT operations. :)

Regards,
HNB

Thaddy de Koning

2014-02-02 01:18

reporter   ~0072801

Last edited: 2014-02-02 01:39

View 7 revisions

You write code like this (E01.dpr, E02.dpr):

  B := TB.Create; // TA.Create (VMT is not used
                  // compiler can determine)

  B.Create; // call TB.Create because of VMT rules

That can't possibly be right?
Calling a constructor twice, once (2th) on an already instantiated object? Without any code in between? What happens if B.Create actually gives back a different pointer to object ? Now you make that second instance potentially disappear.

Interesting problem, but I am not convinced based on these tests.
Can you write some proper unit tests?
I tested your code again on 7,2006 and 2007. They compile! and also leak memory.
B(1) isn't B(2) You are still working B(1) if you were to make a call on B's methods. You dropped the second instance pointer. Create means create....

What I am trying to say is that the code is wrong in the first place.
There may very well be a bug but it can be related to your code in which case there should be e.g. a warning or something, or there is a real bug in the compiler.
First we have to know if real syntactically and semantically correct code gives the same problems.
In this case you may have oversimplied the examples.

Maciej Izak

2014-02-02 11:02

reporter   ~0072803

Last edited: 2014-02-02 11:53

View 3 revisions

Memory leak is expected. I do not want to obfuscate code by "Free" (I made minimal code to present the essence of the problem). Memory leak exist in line:

B := TB.Create;

and

ClassB.Create;

but not in line:

B.Create;

Create not always means create :). Constructor called from object instance is called as normal method. Same for FPC and Delphi.

by Barry Kelly:
"Delphi constructors take a hidden extra parameter which indicates two things: whether NewInstance needs to be called, and what the type of the implicit first parameter (Self) is. When you call a constructor from a class or class reference, you actually want to construct a new object, and the type of the Self parameter will be the actual class type. When you call a constructor from another constructor, or when you're calling the inherited constructor, then the object instance has already been created and is passed as the Self parameter. The hidden extra parameter acts as a Boolean flag which is True for allocating a new instance but False for method-style calls of the constructor."

http://stackoverflow.com/questions/6321603/problems-passing-a-pointer-to-a-constructor-as-a-parameter/6321723#6321723

PS. I attach examples without memory leak: "FPC_BlackList_No_Mem_Leak.zip"

Maciej Izak

2014-02-02 11:30

reporter  

FPC_BlackList_No_Mem_Leak.zip (3,176 bytes)

Sven Barth

2014-02-02 15:59

manager   ~0072806

@Thaddy: it's about whether the compiler calls the correct method (or any at all if its ambigous) and not about potential memory leaks. Also the code that Maciej Izak presented is technically correct, because it's totally legal to call constructors as a normal method (explicitely explained in the quote of Barry Kelly). It might be worth to mention this in the language reference...

Regards,
Sven

Thaddy de Koning

2014-02-03 21:27

reporter   ~0072840

I know that,Sven.

BTW: Someone made some changes this Sunday which possibly refers to this bug but isn't mentioned in this context.
I suggest to test with trunk of today.

Sven Barth

2014-02-05 11:17

manager   ~0072856

Well, you were the one who wrote "That can't possibly be right?" :)

Also the changes from Sunday are not related to this bug. That fixed a bug for constructors that contain an "exit"-statement.

Regards,
Sven

Jonas Maebe

2014-06-15 16:12

manager   ~0075703

To me it makes no sense at all to change the overload searching logic for constructors and class methods compared to regular methods. This should be consistent for any kind of call, and I don't see anything in the Delphi documentation that suggests this is done on purpose. They probably have a bug in their overload selection logic for class methods (constructors are a special kind of class method) that triggers this special behaviour.

In fact, the Delphi documentation explicitly says not to do this at all: "If you use default parameters in overloaded routines, be careful not to introduce ambiguous parameter signatures." (second to last paragraph of http://docwiki.embarcadero.com/RADStudio/XE5/en/Procedures_and_Functions#Overloading_Procedures_and_Functions )

If the behaviour had been consistent for all kinds of routines, then I would have considered implementing it. But given that it's not and the fact that the Delphi documentation itself warns against using this kind of constructions, I prefer not to change anything and keep things consistent.

Maciej Izak

2014-06-15 17:44

reporter   ~0075707

@Jonas Maebe: You are wrong. The overload searching logic don't work correctly even for regular methods. Check out attached "FPC_BlackList.pdf" (example 5 and example 6). I agree that Delphi is buggy but this one thing is implemented very well :). It works in the same way for very long time (checked for all versions between Delphi 6 to XE6 !!!). FPC is missing this one special overload case for constructors, class methods AND regular methods.

Regards,
hnb

Jonas Maebe

2014-06-15 18:01

manager   ~0075708

Example 5 and 6 are exactly the same as the others, as far as I can tell: you have two overloaded methods named Foo, one without parameters and one with a default parameter. This is a textbook example of creating ambiguous signatures by combining overloading and default parameters. FPC correctly gives the error that it cannot determine which overloaded function to call.

There is no "VMT" rule. Whether you call a virtual method or not is irrelevant in your examples, because before the compiler can determine which VMT entry to call, it still has to match the parameters to all possible method signatures. Virtual or not virtual does not matter, only the number of (default) parameters, their types and the presence of "overload" specifiers matters.

So afaics, FPC's behaviour here is consistent with how it handles your original constructor case.

Maciej Izak

2014-06-15 18:51

reporter   ~0075709

Last edited: 2014-06-15 20:32

View 2 revisions

1. My english skills <> pascal skills, please be more patient for this bug report.

2. VMT rules comments are only to explain how it works for example 1-4 and please do not look at the example in the bug report, it obscures the picture. You need to focus on the FPC_BlackList.pdf (all of examples can’t be compiled with FPC, but they works with no problem in any Delphi version). This is consistent for any kind of call, and there is no special rules. Foo from T0 is still considered, despite the fact that in TA is unreachable (for any instance of TA)! For some reason if you override foo from TA in TB during calling foo for TB instance, foo from T0 is still (!) considered (and this is bug), but T0.foo will never be used/called.

T0 = class
  procedure Foo;
end;

TA = class(T0)
  procedure Foo (A : Integer = 0 ) ; overload; virtual;
end;

TB = class (TA)
  procedure Foo (A: Integer ) ; overload ; override ;
end ;

var b: TB;
begin
  a.Foo; // if you comment Foo in T0 it will compile!
end;

Regards,
hnb

Jonas Maebe

2014-06-15 19:35

manager   ~0075711

Foo from T0 is not unreachable. Your "overload" directive for TA.Foo (and TB.Foo) explicitly tells the compiler it must add all methods called "Foo" from the inherited classes to the list of methods to search whenever you call "Foo".

Maciej Izak

2014-06-15 20:53

reporter   ~0075714

Last edited: 2014-06-15 20:54

View 3 revisions

So show me how to call T0.Foo from TA instance or TB instance :). T0.Foo is reintroduced in TA so what is sense of consideration T0.Foo for:

var
  b: TB;

b.Foo; // T0.Foo is unreachable for calling but for perform compiler error is reachable >.<.

What is sense of this behavior? It brings a lot of problems for object oriented design. In descendant class you always need to redefine (redundant) a lot of methods to omit this artificial error message...

Jonas Maebe

2014-06-15 23:00

manager   ~0075716

> So show me how to call T0.Foo from TA instance or TB instance :)

It is indeed hard/impossible because you are ignoring Embarcadero's "If you use default parameters in overloaded routines, be careful not to introduce ambiguous parameter signatures." You are introducing an ambiguous parameter signature here, and that is what is causing the problems.

> T0.Foo is reintroduced in TA

It is not reintroduced in TA. "overload" means "add this declaration to all declarations of methods with this name that were already present in the inherited class, and also allow other declarations with this name in the current class".

> What is sense of this behavior?

It just implements the defined behaviour for the "overload" directive. This is clearly defined by both Embarcadero and by FPC, except in ambiguous cases (which you are supposed not to create, and if you do it anyway then you are on your own).

> It brings a lot of problems for object oriented design.

What causes problems is your introduction of ambiguous declarations that clash. It has nothing to do with object-oriented design.

Maciej Izak

2014-06-16 00:06

reporter   ~0075717

> It is indeed hard/impossible because you are ignoring Embarcadero's (...)

Nope :). It's not my idea. Most of my reported bugs are related to Generics.* (like this bug). This construction is important part of Delphi RTL (maybe not 1:1 but for similar construction in TDictionary and TObjectDictionary - in this bug you have only simplified examples - if you don't believe me then check it yourself). I think this construction is BIG lack of Delphi compatibility.

> It is not reintroduced in TA. "overload" means "add this declaration to all declarations of methods with this name that were already present in the inherited class, and also allow other declarations with this name in the current class".

If this is true then for this code, FPC compiler should also raise error:

// (T0->TA)
// type T0 = class ... type TA = class(T0) ...
var
  a: TA;

a.Foo;

For this construction FPC compiler should also raise error but it works:
// (TA->TB)
// type TA = class ... type TB = class(TA) ...
var
  b: TB;

b.Foo;

Probably I will repeat it again but this also should work
// (T0->TA->TB)
// type T0 = class ... TA = class(T0) ... type TB = class(TA) ...
var
  b2: TB;

b2.Foo;

So if T0->TA works and TA->TB works then T0->TA->TB should also work. My last arguments and then you can ban my account in freepascal bugtracker :P

1. By fixing this bug we will get better Delphi compatibility
2. All existing FPC code base will be unaffected, and will work as always
3. This construction exist for long time in Delphi and this is definitly not Delphi bug -> It's part of RTL.
4. If 1, 2, 3 is not important then please ban my account. Workaround for FPC Generics.Collections is full of redundant code and I have really big Delphi code base > 8 MLOC, I have a lot of this constructions in my code base since D6...

PS. Please don't close this bugreport. If you think that this is not important, then I will try to fix it myself, in near future (after first full Generics.* release for FPC + few RTL Delphi compatible functions for FPC RTL)...

Jonas Maebe

2014-06-16 11:15

manager   ~0075724

Please stop the nonsense about banning your account, nobody has ever been banned from the FPC bug tracker.

The issue is actually caused by not repeating the default value in the overridden routine. If you repeat it, it will work as you expect. I still have to test whether we should automatically inherit the default value, or whether Delphi in fact changes the declaration of the overriding routine.

Maciej Izak

2014-06-18 09:28

reporter   ~0075770

Last edited: 2014-06-18 09:29

View 2 revisions

@Jonas "Please stop the nonsense about banning your account"

Maybe you are angry with me. I opened this report few times. Thanks for your consideration. I hope we can fix it.

Sven Barth

2014-06-19 15:15

manager   ~0075794

@Maciej: Nobody gets angry at you for reopening a bug report multiple times, because you have a different view than one of the developers. You're not the first and certainly not the last. ;) Though it might be better to discuss such things on the mailing list than on the bug tracker...

Regards,
Sven

Maciej Izak

2014-06-21 18:16

reporter   ~0075834

@Sven: Nice to know. I can finally feel safe ;). This bug is already posted on the mailing list some time ago as "Black List of examples that FPC won't compile."

Sven Barth

2014-07-05 10:21

manager   ~0076075

Yes, you posted it there, but I don't see any discussion involving Jonas like it did here...

Regards,
Sven

Maciej Izak

2016-09-19 12:27

reporter  

compiler_patch_distance_for_black_list_19.09.2016.patch (4,080 bytes)
Index: compiler/htypechk.pas
===================================================================
--- compiler/htypechk.pas	(revision 34540)
+++ compiler/htypechk.pas	(working copy)
@@ -73,6 +73,7 @@
         procedure collect_overloads_in_struct(structdef:tabstractrecorddef;ProcdefOverloadList:TFPObjectList;searchhelpers,anoninherited:boolean;spezcontext:tspecializationcontext);
         procedure collect_overloads_in_units(ProcdefOverloadList:TFPObjectList; objcidcall,explicitunit: boolean;spezcontext:tspecializationcontext);
         procedure create_candidate_list(ignorevisibility,allowdefaultparas,objcidcall,explicitunit,searchhelpers,anoninherited:boolean;spezcontext:tspecializationcontext);
+        procedure calc_distance(st_root:tsymtable;objcidcall: boolean);
         function  proc_add(st:tsymtable;pd:tprocdef;objcidcall: boolean):pcandidate;
         function  maybe_specialize(var pd:tprocdef;spezcontext:tspecializationcontext):boolean;
       public
@@ -2517,10 +2518,73 @@
               end;
           end;
 
+        calc_distance(st,objcidcall);
+
         ProcdefOverloadList.Free;
       end;
 
 
+    procedure tcallcandidates.calc_distance(st_root: tsymtable; objcidcall: boolean);
+      var
+        pd:tprocdef;
+        candidate:pcandidate;
+        objdef: tobjectdef;
+        st: tsymtable;
+      begin
+        st:=nil;
+        if (st_root = nil) or (st_root.defowner = nil) or (st_root.defowner.typ <> objectdef) then
+          st := st_root
+        else
+          repeat
+            candidate := FCandidateProcs;
+
+            while candidate <> nil do
+              begin
+                pd:=candidate^.data;
+                if pd.owner = st_root then
+                begin
+                  st:=st_root;
+                  break;
+                end;
+                candidate := candidate^.next;
+              end;
+            if st=nil then
+              begin
+                if st_root.defowner=nil then
+                  Internalerror(201605301);
+
+                if tobjectdef(st_root.defowner).childof = nil then
+                  begin
+                    st:=st_root;
+                    break;
+                  end;
+
+                st_root:=tobjectdef(st_root.defowner).childof.symtable;
+              end;
+          until st<>nil;
+
+        candidate:=FCandidateProcs;
+        while candidate <> nil do
+          begin
+            pd:=candidate^.data;
+            { Give a small penalty for overloaded methods not in
+              defined the current class/unit }
+            {  when calling Objective-C methods via id.method, then the found
+               procsym will be inside an arbitrary ObjectSymtable, and we don't
+               want togive the methods of that particular objcclass precedence over
+               other methods, so instead check against the symtable in which this
+               objcclass is defined }
+            if objcidcall then
+              st:=st.defowner.owner;
+
+            if (st<>pd.owner) then
+              candidate^.ordinal_distance:=candidate^.ordinal_distance+1.0;
+
+          candidate := candidate^.next;
+        end;
+      end;
+
+
     function tcallcandidates.proc_add(st:tsymtable;pd:tprocdef;objcidcall: boolean):pcandidate;
       var
         defaultparacnt : integer;
@@ -2548,17 +2612,6 @@
                dec(result^.firstparaidx,defaultparacnt);
              end;
          end;
-        { Give a small penalty for overloaded methods not in
-          defined the current class/unit }
-        {  when calling Objective-C methods via id.method, then the found
-           procsym will be inside an arbitrary ObjectSymtable, and we don't
-           want togive the methods of that particular objcclass precedence over
-           other methods, so instead check against the symtable in which this
-           objcclass is defined }
-        if objcidcall then
-          st:=st.defowner.owner;
-        if (st<>pd.owner) then
-          result^.ordinal_distance:=result^.ordinal_distance+1.0;
       end;
 
 

Maciej Izak

2016-09-19 20:43

reporter   ~0094724

Opus, I forgot to remove "objdef: tobjectdef;" local variable from calc_distance function in attached patch.

Maciej Izak

2016-12-08 12:04

reporter   ~0096597

The bug is caused by improper "calculation of distance" for methods with default parameter (methods with default parameter are ignored for few scenarios so compiler can't determine).

During my detailed research I discovered that Delphi implementation has small bug (VMT is ignored for scenario E01 and E03 which is totally improper).

I have made patch for this bug (compiler_patch_distance_for_black_list_19.09.2016.patch).

FPC black list details:

E01 source: https://gist.github.com/maciej-izak/f312ff683406e2b16003ee67ee1a95d1#file-e01-pas
{E01 Delphi | FPC with calc_distance }
TA.Create | TB.Create
TB.Create | TB.Create
TB.Create | TB.Create

E02 source: https://gist.github.com/maciej-izak/f312ff683406e2b16003ee67ee1a95d1#file-e02-pas
{E02 Delphi | FPC with calc_distance }
TA.Create | TA.Create
TA.Create | TA.Create
TA.Create | TA.Create

E03 source: https://gist.github.com/maciej-izak/f312ff683406e2b16003ee67ee1a95d1#file-e03-pas
{E03 Delphi | FPC with calc_distance }
TA.Foo | TB.Foo
TB.Foo | TB.Foo
TB.Foo | TB.Foo

E04 source: https://gist.github.com/maciej-izak/f312ff683406e2b16003ee67ee1a95d1#file-e04-pas
{E04 Delphi | FPC with calc_distance }
TA.Foo | TA.Foo
TA.Foo | TA.Foo
TA.Foo | TA.Foo

E05 source: https://gist.github.com/maciej-izak/f312ff683406e2b16003ee67ee1a95d1#file-e05-pas
{E05 Delphi | FPC with calc_distance }
TB.Foo | TB.Foo

E06 source: https://gist.github.com/maciej-izak/f312ff683406e2b16003ee67ee1a95d1#file-e06-pas
{E06 Delphi | FPC with calc_distance }
TA.Foo | TA.Foo

Thaddy de Koning

2016-12-09 09:23

reporter   ~0096613

Aside: did you report this Delphi bug? I can't find a QC for it.

Maciej Izak

2016-12-09 09:38

reporter   ~0096614

No, I am rather passive in Delphi QC for many reasons

Jonas Maebe

2016-12-09 14:40

manager   ~0096621

Thanks for the patch and the tests.

Issue History

Date Modified Username Field Change
2014-01-25 13:40 Maciej Izak New Issue
2014-01-25 13:40 Maciej Izak File Added: r13.lpr
2014-01-25 14:01 Jonas Maebe Note Added: 0072657
2014-01-25 14:55 Maciej Izak Note Added: 0072658
2014-01-25 14:58 Maciej Izak Note Edited: 0072658 View Revisions
2014-01-25 16:56 Jonas Maebe Note Added: 0072663
2014-01-25 17:21 Thaddy de Koning Note Added: 0072664
2014-01-25 17:21 Thaddy de Koning Note Edited: 0072664 View Revisions
2014-01-25 17:25 Thaddy de Koning Note Edited: 0072664 View Revisions
2014-01-25 18:01 Maciej Izak Note Added: 0072668
2014-01-25 18:02 Maciej Izak Note Edited: 0072668 View Revisions
2014-01-26 12:04 Maciej Izak Note Added: 0072686
2014-01-26 12:04 Maciej Izak Note Edited: 0072686 View Revisions
2014-01-26 12:05 Maciej Izak File Added: Test_01.zip
2014-01-26 12:05 Maciej Izak File Added: Test_01.pdf
2014-01-26 13:57 Maciej Izak Note Added: 0072690
2014-01-26 14:01 Maciej Izak Note Edited: 0072690 View Revisions
2014-01-26 15:09 Maciej Izak File Added: Test_02.zip
2014-01-26 15:09 Maciej Izak File Added: Test_02.pdf
2014-01-26 15:09 Maciej Izak Note Added: 0072692
2014-01-26 18:31 Maciej Izak File Added: FPC_BlackList.zip
2014-01-26 18:32 Maciej Izak File Added: FPC_BlackList.pdf
2014-01-26 18:36 Maciej Izak Note Added: 0072710
2014-01-26 18:37 Maciej Izak Note Edited: 0072710 View Revisions
2014-01-26 19:04 Maciej Izak Note Edited: 0072710 View Revisions
2014-02-02 01:18 Thaddy de Koning Note Added: 0072801
2014-02-02 01:19 Thaddy de Koning Note Edited: 0072801 View Revisions
2014-02-02 01:23 Thaddy de Koning Note Edited: 0072801 View Revisions
2014-02-02 01:24 Thaddy de Koning Note Edited: 0072801 View Revisions
2014-02-02 01:29 Thaddy de Koning Note Edited: 0072801 View Revisions
2014-02-02 01:37 Thaddy de Koning Note Edited: 0072801 View Revisions
2014-02-02 01:39 Thaddy de Koning Note Edited: 0072801 View Revisions
2014-02-02 11:02 Maciej Izak Note Added: 0072803
2014-02-02 11:30 Maciej Izak File Added: FPC_BlackList_No_Mem_Leak.zip
2014-02-02 11:36 Maciej Izak Note Edited: 0072803 View Revisions
2014-02-02 11:53 Maciej Izak Note Edited: 0072803 View Revisions
2014-02-02 15:59 Sven Barth Note Added: 0072806
2014-02-03 21:27 Thaddy de Koning Note Added: 0072840
2014-02-05 11:17 Sven Barth Note Added: 0072856
2014-06-15 16:12 Jonas Maebe Note Added: 0075703
2014-06-15 16:12 Jonas Maebe Status new => resolved
2014-06-15 16:12 Jonas Maebe Resolution open => won't fix
2014-06-15 16:12 Jonas Maebe Assigned To => Jonas Maebe
2014-06-15 17:44 Maciej Izak Note Added: 0075707
2014-06-15 17:44 Maciej Izak Status resolved => feedback
2014-06-15 17:44 Maciej Izak Resolution won't fix => reopened
2014-06-15 18:01 Jonas Maebe Note Added: 0075708
2014-06-15 18:01 Jonas Maebe Status feedback => resolved
2014-06-15 18:01 Jonas Maebe Resolution reopened => won't fix
2014-06-15 18:51 Maciej Izak Note Added: 0075709
2014-06-15 18:51 Maciej Izak Status resolved => feedback
2014-06-15 18:51 Maciej Izak Resolution won't fix => reopened
2014-06-15 19:35 Jonas Maebe Note Added: 0075711
2014-06-15 20:32 Maciej Izak Note Edited: 0075709 View Revisions
2014-06-15 20:53 Maciej Izak Note Added: 0075714
2014-06-15 20:53 Maciej Izak Status feedback => assigned
2014-06-15 20:54 Maciej Izak Note Edited: 0075714 View Revisions
2014-06-15 20:54 Maciej Izak Note Edited: 0075714 View Revisions
2014-06-15 23:00 Jonas Maebe Note Added: 0075716
2014-06-16 00:06 Maciej Izak Note Added: 0075717
2014-06-16 11:15 Jonas Maebe Note Added: 0075724
2014-06-18 09:28 Maciej Izak Note Added: 0075770
2014-06-18 09:29 Maciej Izak Note Edited: 0075770 View Revisions
2014-06-19 15:15 Sven Barth Note Added: 0075794
2014-06-21 18:16 Maciej Izak Note Added: 0075834
2014-07-02 19:14 Jonas Maebe Assigned To Jonas Maebe =>
2014-07-02 19:14 Jonas Maebe Status assigned => new
2014-07-05 10:21 Sven Barth Note Added: 0076075
2015-01-06 13:55 Sven Barth Relationship added parent of 0027206
2015-01-06 13:57 Sven Barth Relationship deleted parent of 0027206
2015-01-06 13:57 Sven Barth Relationship added related to 0027206
2016-09-19 12:27 Maciej Izak File Added: compiler_patch_distance_for_black_list_19.09.2016.patch
2016-09-19 20:43 Maciej Izak Note Added: 0094724
2016-12-08 11:45 Jonas Maebe Relationship added related to 0031081
2016-12-08 12:04 Maciej Izak Note Added: 0096597
2016-12-08 23:09 Jonas Maebe Assigned To => Jonas Maebe
2016-12-08 23:09 Jonas Maebe Status new => assigned
2016-12-09 09:23 Thaddy de Koning Note Added: 0096613
2016-12-09 09:38 Maciej Izak Note Added: 0096614
2016-12-09 14:40 Jonas Maebe Fixed in Revision => 35089
2016-12-09 14:40 Jonas Maebe Note Added: 0096621
2016-12-09 14:40 Jonas Maebe Status assigned => resolved
2016-12-09 14:40 Jonas Maebe Fixed in Version => 3.1.1
2016-12-09 14:40 Jonas Maebe Resolution reopened => fixed
2017-03-13 14:29 Maciej Izak Status resolved => closed