Strange issue when overloading / overriding the constructor

Issue #33 resolved
Christian Budde created an issue

The below code behalves strange:

type
  TApp = class
  end;

type
  TCustomScene = class
  private
    FA: Integer;
    FB: Integer;
    FC: Integer;
    FApp: TApp;
  protected
    constructor Create(App: TApp); overload;

    procedure PA; virtual; abstract;
    procedure PB; virtual; abstract;
    procedure PC; virtual; abstract;
    procedure PD; virtual; abstract;
    procedure PE; virtual; abstract;
  public
    constructor Create; overload; virtual; abstract;

    property App: TApp read FApp; // <- rename to 'Owner' to make it work
    property A: Integer read FA; // <- comment one of these lines to make it work
    property B: Integer read FB;
    property C: Integer read FC; // <- rename to 'Creatf' to make it work again ('Creatd' fails!)
  end;

type
  TMyScene = class(TCustomScene)
  protected
    procedure PA; override;
    procedure PB; override;
    procedure PC; override;
    procedure PD; override;
    procedure PE; override;
  public
    constructor Create; override;
  end;  

constructor TCustomScene.Create(App: TApp);
begin
  FApp := App;
end;

procedure TMyScene.PA; begin end;
procedure TMyScene.PB; begin end;
procedure TMyScene.PC; begin end;
procedure TMyScene.PD; begin end;
procedure TMyScene.PE; begin end;
constructor TMyScene.Create; begin end;

The code results in the Syntax Error: "Inherited method "Create" isn't virtual. "override" not applicable"

The funny thing is that it seems to work as soon as you alter some of the lines in the code (see comments).

Comments (7)

  1. Anders Melander

    Reproduced.

    I believe the problem is that when the compiler looks for the base class symbol being overridden, it doesn't take parameters into account. It should perform the symbol lookup using a method signature that includes the parameters.

    The reason you can make the script compile by making small changes to the member names is that this alters the order of the sorted symbol table, thus changing which of the two symbols is found.
    You can also make it compile by changing the order in which the two Create symbols are added to the base class symbol table. E.g. this compiles :

      TCustomScene = class
      public
        constructor Create; overload; virtual; abstract;
      private
        ...etc...
    

    Anyway, the problem originates in the following line in TdwsCompiler.ReadMethodDecl:

       // Check if name is already used
       sym:=ownerSym.Members.FindSymbolFromScope(name, ownerSym);
    

    IMO the best solution would be to introduce an overload of FindSymbolFromScope (or an option) that performs the initial lookup using the name (as it does now) but then also compares the parameter list.

  2. Eric Grange repo owner

    The issue is that the compiler currently requires an explicit "overload" and does not infer it from the ancestor class, so

    constructor Create; override;
    

    fails because the compiler does not consider it as an overloaded method, while

    constructor Create; overload; override;
    

    passes. The Delphi compiler is not very consistent either in that regard, for instance the following code will trigger an error in Delphi:

       TBase = class
          constructor Create; overload; virtual;
          constructor Create(a : Integer); overload;
       end;
    
       TChild = class (TBase)
          constructor Create; override;
          constructor Create(a : Integer);   // error on this line
       end;
    

    So it seems that when "override" is used, an implicit overload is accepted (but it can be explicit).

    I guess I will add the implicit overload support, but add a hint or warning, because accepting the following code without a message does not look right (especially in a more complex declaration with stuff or scope differences between the implicit and the explicit overload)

       TChild = class (TBase)
          constructor Create; override;
          constructor Create(a : Integer); overload;
       end;
    
  3. Anders Melander

    I guess you already know the following is in the source:

    // this could actually be an override of an inherited method
    // and not just an overload of a local method
    // in that case 'overload' is optional
    

    I get your point with regard to "It doesn't look right" but I'm not that happy about it emitting a hint or warning. It is proper use of the language after all and one we've gotten used to from Delphi.

    I don't think your Delphi example shows an inconsistency in Delphi. TChild does declare two methods named Create and one of them isn't marked as overloaded (while the other is so implicitly).

  4. Eric Grange repo owner

    I think it is inconsistent because when declaring TBase, not having both "overloaded" is an error for the compiler, and in TChild having both constructors "overloaded" explicitly is not an error. It also makes the code self-explanatory without having to lookup the ancestor declaration, which makes it desirable in Delphi as well to always use an explicit "overloaded".

    So the implicit overloaded looks more like a bug/oversight in the compiler than a language design choice, as it breaks local consistency of a class declaration (being able to understand a declaration without having to lookup the ancestor)

    For instance if TBase no longer overloads Create, then the Delphi compiler will complain about a missing overloaded in TChild if the override is not preceded by an explicit "overloaded". If you explicitly "overloaded" both constructors in TChild, then the TChild declaration stays consistent and error-free in both scenarios.

  5. Anders Melander

    You are right. I actually thought Delphi allowed omission of the overload on the virtual base method.

  6. Log in to comment