Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements Via not allowing member override #1840

Closed
tdimitriou opened this issue May 1, 2024 · 12 comments
Closed

Implements Via not allowing member override #1840

tdimitriou opened this issue May 1, 2024 · 12 comments

Comments

@tdimitriou
Copy link

tdimitriou commented May 1, 2024

Describe the bug
When Implements Via was first introduced, it was possible to override any member of the base class directly (https://github.com/twinbasic/lang-design/issues/10#issuecomment-1194965243)., which was really useful, and made it very easy to mimic inheritance, without having to re-implement every single member and forward it to the base class manually. The possibility was removed after some beta update (I cannot recall when, but it should be more than one year ago), and has never came back.

To Reproduce
Use the following code to reproduce the behavior:


Class MyCollection
    Implements Collection Via Base = New Collection
        
    Private Sub Collection_Add(ByRef Item As Variant, _    
                                    Optional ByRef Key As Variant, _
                                    Optional ByRef Before As Variant, _
                                    Optional ByRef After As Variant) Implements Collection.Add

        Base.Add(Item, Key, Before, After)
        Debug.Print "Items in collection:" ; Base.Count
    End Sub
            
End Class

Module MyApp

    Sub Main()
        Dim MyCol As Collection = New MyCollection

        Debug.Cls
        Debug.Print "=========================================================="
        Debug.Print "Test MyCollection.Add"
        Debug.Print "----------------------------------------------------------"
        Debug.Print "Should print the Collection.Count after each function call"
        Debug.Print "----------------------------------------------------------"
        MyCol.Add("Element 1")
        MyCol.Add("Element 2")
        Debug.Print "=========================================================="
    End Sub
End Module

Expected behavior
Implements Via should allow the override of any member, either directly, or by introducing an Overrides keyword. Direct overrides were enough, for most cases and should be re-enabled, except if the removal was a necessary part of of the plan to support full inheritance in the future.

Screenshots

Desktop (please complete the following information):

  • OS: Windows 11 Pro
  • twinBASIC compiler version BETA 527 (30/04/2024)

Additional context
Add any other context about the problem here.

@FullValueRider
Copy link

FullValueRider commented May 1, 2024

I think you are misunderstanding how Implements via works. The way in which you define your method for Add is also incorrect as they Key, Before and After parameters should be Optional. I've updated your code below and it works fine. I leave it to you to define how to handle the optional parameters.

Class MyCollection

    Implements Collection Via Base = New Collection

    Public Sub Add(Item As Variant, Optional Key As Variant, Optional Before As Variant, Optional After As Variant)
        Debug.Print "MyCollection.Add"
        Base.Add Item  'line corrected ater first post
    End Sub

End Class

Module MyApp

    Sub Main()
        Dim MyCol As MyCollection = New MyCollection

        Debug.Cls
        Debug.Print "========================================================"
        Debug.Print "Test MyCollection.Add"
        Debug.Print "--------------------------------------------------------"
        Debug.Print "(Should print 'MyCollection.Add' for each function call)"
        Debug.Print "--------------------------------------------------------"
        MyCol.Add "Element 1"
        MyCol.Add "Element 2"
        Debug.Print "========================================================"
    End Sub

End Module

@tdimitriou
Copy link
Author

I think you are misunderstanding how Implements via works. The way in which you define your method for Add is also incorrect as they Key, Before and After parameters should be Optional. I've updated your code below and it works fine. I leave it to you to define how to handle the optional parameters.

Class MyCollection

    Implements Collection Via Base = New Collection

    Public Sub Add(Item As Variant, Optional Key As Variant, Optional Before As Variant, Optional After As Variant)
        Debug.Print "MyCollection.Add"
        Base.Add(Item, Key, Before, After)
    End Sub

End Class

Module MyApp

    Sub Main()
        Dim MyCol As MyCollection = New MyCollection

        Debug.Cls
        Debug.Print "========================================================"
        Debug.Print "Test MyCollection.Add"
        Debug.Print "--------------------------------------------------------"
        Debug.Print "(Should print 'MyCollection.Add' for each function call)"
        Debug.Print "--------------------------------------------------------"
        MyCol.Add "Element 1"
        MyCol.Add "Element 2"
        Debug.Print "========================================================"
    End Sub

End Module

You are absolutely correct about the Add declaration and the optional parameters. I have already edited the post and corrected it. By the way, the declaration was automatically generated by the IDE (Implements without the Via ...), so this is probably an issue to report.

As far as the the code correction you suggested, with all my respect, I do not agree that this is the correct way to go, nor does it follow the original specification that was given from @WaynePhillipsEA during the initial discussion about inheritance twinbasic/lang-design#10 (comment). The example code in the above link has the DoSomething sub declared as private just to replace the original implementation and not be exposed, according to the code comments, as well. I can also recall that, during the early stages of Implements Via, member Overrides were allowed the way I suggested. Maybe @WaynePhillipsEA could shed some light on it.

@FullValueRider
Copy link

FullValueRider commented May 1, 2024

Its probably not best to pick your definition from the middle of a discussion but to look at what was finaly implemented. Fafalone does a great job in documenting the new features of twinBasic.

https://github.com/twinbasic/documentation/wiki/twinBASIC-Features

Although I'd have to admit that the example on 'implements via' likely needs updating to differentiate overriding methods in an interface vs overriding methods of a class.

But, as ever, Wayne is the only person who can provide a definitive answer.

@FullValueRider
Copy link

In your updated question this line is still problematical as myCol refers to the native tb collection and not your myCollection class.

Dim MyCol As Collection = New MyCollection

In my response you can see I updated it to

Dim MyCol As MyCollection = New MyCollection

@wqweto
Copy link

wqweto commented May 1, 2024

IMO Dim MyCol As Collection = New MyCollection is the original indent of the issue and not supporting Implements Interface.Method on private methods for Implements Via interfaces (the way it is for Implements interfaces) for "selective" method override is unfortunate as it would round up the Implements Via feature wholesomely.

I tried to hack it declaring both types of Implements simultaneously

Class MyCollection
    Implements Collection Via Base = New Collection
    Implements Collection

    Private Sub Connection_Add(ByRef Item As Variant, _    
...

... and it compiles and intercepts Add but bombs on Count for instance, probably because the VTable is full of nullptrs.

Compiler error reporting is currently very lax on Implements apparently e.g. warns but does not stop on missing methods, allows same interface being implemented twice, etc.

@tdimitriou
Copy link
Author

tdimitriou commented May 1, 2024

In your updated question this line is still problematical as myCol refers to the native tb collection and not your myCollection class.

Dim MyCol As Collection = New MyCollection

In my response you can see I updated it to

Dim MyCol As MyCollection = New MyCollection

This is exactly what I want to achieve. A custom collection like MyCollection is still a Collection. It exposes all the methods of the native VB Collection, plus any other additional methods. The Add method in question (as well as any other method of the native collection) should allow custom implementation (override).

To make may point clear, consider the following 2 examples, the first with Implements, and the second with Implements Via

Class MyBaseClass

    Function Foo() As String
        Return "Foo"
    End Function

    Function Boo() As String
        Return "Boo"
    End Function
    
End Class

' Example 1: Implements
Class MyClass1
    Implements MyBaseClass
    ' Declare an instance of the base class to mimic inheritance manually
    Private Base As MyBaseClass = New MyBaseClass

    ' We need to implement all the members of the base class
    ' The names "MyBaseClass_Foo" and "MyBaseClass_Foo" could have been changed,
    ' since the "Implements MyBaseClass.XXX" statements ensure the result
    
    ' Return the string "FooFoo" instead of "Foo" which the base class would return
    Private Function MyBaseClass_Foo() As String Implements MyBaseClass.Foo
        Return Base.Foo & Base.Foo
        ' Works Perfectly
    End Function

    ' Inherit the exact functionality of the base class
    Private Function MyBaseClass_Boo() As String Implements MyBaseClass.Boo
        Return Base.Boo()
    End Function
        
    ' Extend the MyBaseClass by adding a ne member
    ' The member is not accessible if we declare the
    ' variable as "Dim MyVar As MyBaseClass = New MyClass2"
    ' but it is accessible when we declare the variable as
    ' "Dim MyVar As MyClass2 = New MyClass2"
    
    Public Function SayHello() As String
        Return "Hello!"
    End Function
    
End Class

' Example 2: Implements Via
Class MyClass2
    Implements MyBaseClass Via Base = New MyBaseClass
    
    ' The derived class inherits all the members of the base class, so we do not have to re-implement them
    
    ' We should be able to override the std implementation by replacing it using the following syntax
    ' The function name "MyBaseClass_Foo" could have been anything since the 
    ' "Implements MyBaseClass.Foo" statement ensures what its purpose is
    Private Function MyBaseClass_Foo() As String Implements MyBaseClass.Foo
        ' Override the std implementation of the base class (MyBaseClass)
        ' to return the string "FooFoo" instead of "Foo"
        Return Base.Foo & Base.Foo
        
        ' However this is not functional and this function is not 
        ' called when calling the following code
        ' Dim x As MyBaseClass = New MyClass2
        ' Debug.Print x.Foo()
    End Function

    ' Extend the MyBaseClass by adding a ne member
    ' The member is not accessible if we declare the
    ' variable as "Dim MyVar As MyBaseClass = New MyClass2"
    ' but it is accessible when we declare the variable as
    ' "Dim MyVar As MyClass2 = New MyClass2"
    
    Public Function SayHello() As String
        Return "Hello!"
    End Function
            
End Class
 

The above examples show the following:

If we use the Implements without the Via syntax

  1. We must write code for all of the members to implement the interface of the base class (very time consuming if our base class has tons of members).
  2. We can achieve (mimic) inheritance by using the *Private MyBaseClassInstance as MyBaseClass" variable and forward all the members to this variable (eg. Return MyBaseClassInstance.Foo).
  3. We can use custom implementation for any member (mimic Overrides befaviour).
  4. We can even extend our class by adding new members which are, of cource accessible from its default interface and not the inherited one.
  5. The implemented members MyBaseClass.Foo and MyBaseClass.Boo are not accessible from the default interface of the class (e.g. Dim c as MyClass1 shoulf not expose Foo and Boo). This is the std VB6 behaviour.

If we use the Implements Via syntax

  1. We do not need to write code to implement the members of the interface of the base class because it is inherited by default.
  2. We cannot override any of the members of the base class because the statement Private Sub MySub() Implements Base.Foo is not recognized as explained.
  3. We can extend our class by adding new members which are, of cource accessible from its default interface and not the inherited one.
  4. The implemented members MyBaseClass.Foo and MyBaseClass.Boo are also accessible from the default interface of the derived class, which is the expected and desired behaviour of inheritance.

My conclusion is that Implements Via provides a really smart and useful solution for simple inheritance. If the direct override of any member is achieved, it would make it even more powerfull.

@tdimitriou
Copy link
Author

IMO Dim MyCol As Collection = New MyCollection is the original indent of the issue and not supporting Implements Interface.Method on private methods for Implements Via interfaces (the way it is for Implements interfaces) for "selective" method override is unfortunate as it would round up the Implements Via feature wholesomely.

I could not have agreed more... This is exactly my point.

I tried both

Class MyCollection
    Implements Collection Via Base = New Collection
    Implements Collection

    Private Sub Connection_Add(ByRef Item As Variant, _    
...

... and it compiles and intercepts Add but bombs on Count for instance, probably because the VTable is full of nullptrs.

Compiler error reporting is currently very lax on Implements apparently e.g. warns but does not stop on missing methods, allows same interface being implemented twice, etc.

I have not checked the VTable yet, but what I guess is done, in the case of Implements Via at the moment, is that the function pointers of the Base Class members are put to both the primary interface and the implemented interface positions, to achieve both interface implementation and inheritance. It would be absolutely possible to instruct the compiler to recognize the Implements Interface.Method syntax for selective overrides and replace the specific original pointers.

Of cource, I understand that this is not a top priority at the moment, since the primary goal is to achieve 100% VBA/VBx compatibility.

@FullValueRider
Copy link

I must admit that I'm struggling to follow your arguments

MyCollection using 'Implements via' with no additional methods is essentially aliasing a type, which can be useful. All methods of collection appear as methods of myCollection instances and all usage is referred back to the Base collection instance.

If I want to override the add method from collection in the mycollection class then my understanding is that I need to add a public add method to MyCollection. Then any instances of mycollection will use the new add method and if required I simply use Base.add to access the add method of the Base instance. e.g. me.add vs base.add.

I've not encountered any significant issues with using the inheritance model provided by implements via.

I've not yet come across an instance where I need to define a private class as this, I think, is just a VBx workaround for hiding Interface methods, hence the need for _ in Interface method names.

Given that the discussion is moving beyond my limited understanding, this is the last I'll say on this subject.

@WaynePhillipsEA
Copy link
Collaborator

Unfortunately, Implements... Via just wasn't designed to support overrides. QueryInterface requests are passed on to the Base field directly, as such it's not easy to change this behaviour to accommodate overrides. Implements... Via certainly has its uses, but it does have limitations too.

You'll need to wait for proper inheritance support for this feature.

@WaynePhillipsEA WaynePhillipsEA closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2024
@tdimitriou
Copy link
Author

tdimitriou commented May 1, 2024

I must admit that I'm struggling to follow your arguments

MyCollection using 'Implements via' with no additional methods is essentially aliasing a type, which can be useful. All methods of collection appear as methods of myCollection instances and all usage is referred back to the Base collection instance.

Nobody said that it's not useful. It has many uses and, I think, we are all more than grateful to have this feature in tb.

If I want to override the add method from collection in the mycollection class then my understanding is that I need to add a public add method to MyCollection. Then any instances of mycollection will use the new add method and if required I simply use Base.add to access the add method of the Base instance. e.g. me.add vs base.add.

This works, under the condition that the declaration is Dim Col As MyCollection and not Dim Col As Collection.

The following example, will show a case where the As Collection declaration cannot be avoided, just to make the situation more clear. Assume that we call the method ProcessCollection, which comes from a third party dll, and takes a Collection as an argument:

    Public Sub ProcessCollection(Col As Collection)

Because the method signature explicitly declares the Col argument As Collection, any call to the Col.Add(Item, ...) inside ProcessCollection, will be passed directly to the Add Method of the base Collection, because the declaration will force to query for the _Collection and not the _MyCollection interface.

I've not encountered any significant issues with using the inheritance model provided by implements via.

As already said, Implements Via works like a charm and saves hundreds of lines of code, which are mandatory in VBx, to achieve simple inheritance. I was just talking about a missing part, which I thought that would be quite easy to implement and would make things even more smooth.

I've not yet come across an instance where I need to define a private class as this, I think, is just a VBx workaround for hiding Interface methods, hence the need for _ in Interface method names.

Given that the discussion is moving beyond my limited understanding, this is the last I'll say on this subject.

As Wayne has already stated, member overrides are not easy and will not be accomodated using the Implements ... Via syntax, but we need to wait for proper inheritance support in tb instead.

@wqweto
Copy link

wqweto commented May 2, 2024

QueryInterface requests are passed on to the Base field directly. . .

Btw, this seems like a pretty opportunistic implementation without proper aggregation :-))

For instance this is failing a basic COM invariant:

Class Form1

    Sub New()
        Dim pCol As Collection = New MyCollection
        Dim MyCol As MyCollection = pCol        '<--- No such interface supported
    End Sub
    
End Class

Class MyCollection
    Implements Collection Via Base = New Collection
End Class

... because "inside" ICollection::QueryInterface doesn't know how to cast to "outside" MyCollection interface.

Probably known limitation but still should I file this as a separate issue?

On a second note, do TB produced coclasses support aggregation per se? This might help Implements Via esp. if built-in classes like Collection support it too.

@WaynePhillipsEA
Copy link
Collaborator

Yes, a known limitation. Implements Via fulfils a specific purpose (mostly for internal AX support), not general inheritance.

No, aggregation is not yet supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants