twinbasic / lang-design

Language Design for twinBASIC
MIT License
11 stars 1 forks source link

Custom enumerators #6

Open mansellan opened 3 years ago

mansellan commented 3 years ago

Is your feature request related to a problem? Implementing custom enumerators in VB6 is difficult. twinBASIC could make this easier with some language-level support.

Describe the solution you'd like Built-in interfaces (IEnumerable and IEnumerable(Of T)), which would contain a NewEnum function (which would be automatically assigned Procedure ID = -4) which returns a type which gets special-cased by the compiler to support IEnumVARIANT.

Describe alternatives you've considered Alternately, special keywords could be used, not sure what that would be though.

Additional context The For Each construct can be supported in a custom collection class by adding an IUnknown function called NewEnum and setting it's procedure Id to -4:

' @ProcedureId(-4)
Public Function NewEnum As IUnknown
  Set NewEnum = _collection.[_NewEnum]
End Function

The function must return a pointer to an IEnumVARIANT variant. And therein lies the problem - this interface is not implementable in VB6. There is a workaround, but it requires several hacks and some custom IDL.

It would be great if we could do this:

Public Class List(Of T)
  Implements IEnumerable(Of T)

  Dim _enumerator As Enumerator(Of T)

  Public Sub New()
     Set _enumerator = new Enumerator(Of T)(Me)
  End Sub

  Public Function NewEnum As IEnumerator(Of T) ' Automatically assigned Procedure ID -4
    Set NewEnum = _enumerator
  End Function
End Class

Public Class Enumerator(Of T)
  Implements IEnumerator(Of T) ' <------ Compiles to IEnumVARIANT

  ' Implementation skipped
End Class
bclothier commented 3 years ago

Just as a note... IEnumVARIANT can only return a Variant. There are several articles out there explaining how to create a custom collection class but ultimately it's possible to write a VB* code like this that compiles but will blow up runtime:

Dim notThatThing As NotThatThing

For Each notThatThing In ACollectionOfThings
Next

The non-generic IEnumerator should compile down to IEnumVARIANT, but the generic version will have to be a private implementation detail within tB code. The generic version of IEnumerator itself can even implement IEnumerator so that we do not need to explicitly implement both non-generic and generic forms, but it's something to be aware when using it in a VB* codebase.

mansellan commented 3 years ago

Ok cool, makes sense. Wasn't sure how it would play with tB generics.

FullValueRider commented 3 years ago

It might make more sense to allow initialisers for collections. If it were possible to pass arrays to a collection (one for the keys and one for the values) you getalmost everything @Mansellan has requested as well as making the collection object a much more usable object.

e.g.

Dim myColl as collection = new collection(array1,array2)

vbRichClient commented 3 years ago

In VB6 (and VBA in 32Bit-Office) we can already do this (directly in a Form-Class):

Option Explicit

Implements vbIEnumerable

Private Keys(1 To 3) As String, Vals(1 To 3) As Double, Member

Private Sub Form_Load()
  Keys(1) = "Prop1": Vals(1) = 1.1
  Keys(2) = "Prop2": Vals(2) = 2.2
  Keys(3) = "Prop3": Vals(3) = 3.3

  With New_c.ArrayList(vbString)
    For Each Member In New_c.VBI.EnumerateOn(Me, LBound(Keys))
       .Push Member
    Next
    Debug.Print "[" & .Join(", ") & "]" 'finally join the pushed JsonObj-parts into a JSON-array
  End With
End Sub

Private Function vbIEnumerable_ElementForIndex(ByVal Index As Long, UserData, CancelEnumeration As Boolean)
  If Index > UBound(Keys) Then CancelEnumeration = True: Exit Function

  'return the members of 2 different arrays as a properly encoded JSON-Object-String
  vbIEnumerable_ElementForIndex = "{""" & Keys(Index) & """:" & Str(Vals(Index)) & "}"
End Function

Which then prints out the following, properly encoded JSON-array-string: [{"Prop1": 1.1}, {"Prop2": 2.2}, {"Prop3": 3.3}]

The only requirement for the above snippet to work, is a reference to vbRichClient5 or (the newer) RC6.

I guess what I'm trying to say is, that "pure" IEnumVariant-based Enumerations - only have to be properly supported at compiler-level (which twinBasic currently doesn't).

Advanced flexibility with Enumerations, can then be implemented at runtime-lib-level -or as in case of the referenced RC6-lib, at the level of any Helper-Lib which returns proper IEnumVariant-wrapping Objects (behind the In-Keyword of a For Each Loop).

Olaf

Kr00l commented 3 years ago

There are plenty of ways to achieve this in VB6. IMO a good way is to use lightweight-COM described by DEXWERX. (full .BAS only solution) So, I think there is no hurry for tB to have a "native" solution and it should work (at least first) with the current solutions in place.

Greedquest commented 3 years ago

@Kr00l I think language support is probably quite key (I know you aren't suggesting it isn't) but imo that com workaround is too buggy and slow for me to use even in proof of concept code, the unmanaged memory just risks crashes while debugging. Haven't come across vbRichClient before @vbRichClient but it looks good, although how does its performance compare to a native early evaluated object? I got 1 order of magnitude slower for the DEXWERX method (see comments on this answer) whereas python's range generator is as fast as iterating over a list of numbers, even though it is lazy (actually, I just checked, it's half as fast, but that's better than 10x slower).

Also does IEnumerable have lazy evaluation built in - you provide either an array of values or a generator function, or should I put in a separate feature request for something like a yield keyword to allow this?

mansellan commented 3 years ago

@Greedquest yes, IEnumerator has a callback for MoveNext, so can be evaluated lazily. That said, a Yield Return syntax would allow simpler implementation, so would be a nice-to-have IMO.

WaynePhillipsEA commented 3 years ago

I think this is a great idea. Tomorrow we'll have IDispatch support in tB classes, so we can then use the legacy DISPID_ENUM(-4) method to supply the enumerator. But going forward, I'll be adding this native version, or something very similar.

mansellan commented 3 years ago

As bclothier said, it makes more sense to base the IEnumerable (non-generic) interface on the IEnumVARIANT interface than on the .Net equivalent.

The IEnumVARIANT interface inherits IUnknown, and has the following members:

HRESULT Clone(IEnumVARIANT **ppEnum);
HRESULT Next(ULONG  celt, VARIANT *rgVar,  ULONG  *pCeltFetched);
HRESULT Reset();
HRESULT Skip(ULONG celt);

Note that these use an unsigned long, which is not yet supported in twinBASIC (seee twinbasic/lang-design#9).

If we could have an IEnumerable interface of the following:

Interface IEnumerable
  Public Function Clone(ByRef elements As IEnumerable) As Boolean
  Public Function Next(ByVal elementCount As ULong, ByRef elements As Variant(), ByRef returnedElementCount As ULong) As Boolean
  Public Function Reset() As Boolean
  Public Function Skip(ByVal elementCount As ULong) As Boolean
End Interface

Could twinBASIC handle the conversion to / from IEnumVariant? Ideally, IEnumerable and IEnumVARIANT should be freely interchangeable.

Note, I suspect that Clone and Skip are not used by the VB For Each construct, however having them available on the interface is probably best for interop.

mansellan commented 3 years ago

Also, I assumed the interface needed Boolean functions to handle the HRESULT, but actually maybe they are better as Subs?

WaynePhillipsEA commented 3 years ago

@mansellan The VB.NET IEnumerator internally implements IEnumVARIANT, and is simpler, having only 3 methods (prop-get Current / MoveNext / Reset).

I suggest that twinBASIC simply offers the same/equivalent IEnumerator interface, and handles the IEnumVARIANT for you behind the scenes.

Also, for a fully conformant implementation, the IEnumVARIANT::Next method requires support for returning a non-error HRESULT of S_FALSE (value 1), which is something that you can't ordinarily do in VBx or tB, so I do think the raw implementation of IEnumVARIANT is best handled by the compiler.

WaynePhillipsEA commented 3 years ago

Also, I assumed the interface needed Boolean functions to handle the HRESULT, but actually maybe they are better as Subs?

Class members in tB (and VBx) return HRESULTs for you, based on the local error handling. A return value defined on a class member is actually passed as an out-reference parameter, as the final parameter.

VB.NET gives more control over HRESULT handling, using the [PreserveSig] attribute, but we haven't yet got anything similar in tB.

Kr00l commented 3 years ago

Also, for a fully conformant implementation, the IEnumVARIANT::Next method requires support for returning a non-error HRESULT of S_FALSE (value 1), which is something that you can't ordinarily do in VBx or tB, so I do think the raw implementation of IEnumVARIANT is best handled by the compiler.

I know that lightweight COM can circumvent that HRESULT "problem" by just having a Function which returns Long. It's not a big deal to still do that.

However, would it be technically possible to somehow allow a return value for a "Sub" (=HRESULT) ? I can imagine that COM error-ing stuff could dissalow it.. It may be convenient here and there when dealing with type libraries..

mansellan commented 3 years ago

@Kr00l Reading up on [PreserveSig], it seems to do exactly that. I hadn't heard of it before Wayne mentioned it.

https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.preservesigattribute?view=net-5.0

FullValueRider commented 2 years ago

Has twinBasic achieved this goal yet, or is it still on the 'to do' list

WaynePhillipsEA commented 2 years ago

I'll look at this very soon

wqweto commented 2 years ago

Btw, VBx uses IEnumVARIANT::Next method only and in a limited fashion only (requesting a single element in celt).

Probably not a good idea to base TB's enumerators implementation on the complete COM interface as the rest of the methods of IEnumVARIANT are dead cruft already.

Kr00l commented 2 years ago

Any chance to get a safe handler for IEnumString as well? It's also useful for some API's. (e.g. the SHAutocomplete API)

So, for example: Implements IEnumerator(Of String) ' <- IEnumString Implements IEnumerator(Of Variant) ' <- IEnumVARIANT something else would cause a compile error.

bclothier commented 2 years ago

Apologies for piling on but I want to point out that one scenario we should consider is supporting enumerable collection for WithEvents. We currently have the support for using an array which is great when the members are known at the compile time but I think there are also scenarios where members are added/removed at runtime and therefore an array would be quite expensive. Thus, it would be nice to have an enumerator that can be used with the WithEvents to forward the events from the members.

mansellan commented 2 years ago

something else would cause a compile error

I respectfully disagree. IEnumerator (Of Integer) should be compilable in twinBASIC, and templated for enumerating through Integers from twnBASIC callers. At the COM boundary, it should be expressed as IEnumVARIANT, and possibly runtime checked for a subtype of Integer.

sancarn commented 2 years ago

Any chance to get a safe handler for IEnumString

Surely it'd be better if Variant's were just be a generic type. I.E.

Interface IEnumVARIANT<T>
  Public Function Clone(ByRef elements As IEnumerable) As Boolean
  Public Function Next(ByVal elementCount As ULong, ByRef elements As Variant<T>, ByRef returnedElementCount As ULong) As Boolean
  Public Function Reset() As Boolean
  Public Function Skip(ByVal elementCount As ULong) As Boolean
End Interface

At that point you could Identify the type of variants at compile time, helping massively with intellisense.

bclothier commented 2 years ago

With the implementation of Extends (#806) and Err.RerturnHRESULT (#25), we are able to implement our custom enumerator using pure TB. We still have to redefine the IEnumVARIANT due to unsupported data types used in the methods but a redefined interface works well enough. This isn't completely tested as the language will only request one element and I don't know of any caller that will request more than one elements.

[ InterfaceId ("00020404-0000-0000-C000-000000000046") ]
Private Interface IEnumVARIANT Extends stdole.IUnknown
    Sub Next(ByVal celt As Long, ByRef rgvar As Variant, ByRef pceltFetched As Long)
    Sub Skip(ByVal celt As Long)
    Sub Reset()
    Sub Clone(ByRef ppenum As IEnumVARIANT)
End Interface

Public Class TestCustomEnumeartor
    Implements IEnumVARIANT

    Private CurrentIndex As Long
    Private Const MinValue As Long = 65
    Private Const MaxValue As Long = 123
    Private Const E_INVALIDARGS As Long = &H80070057
    Private Const S_OK As Long = 0
    Private Const S_FALSE As Long = 1

    Private Sub Next(ByVal celt As Long, ByRef rgvar As Variant, ByRef pceltFetched As Long) Implements IEnumVARIANT.Next
        If VarPtr(rgvar) = 0 Then
            Err.ReturnHResult = E_INVALIDARGS
            Exit Sub
        End If

        Dim Index As Long
        If celt = 1 Then
            If CurrentIndex < MaxValue Then
                Index = 1
                rgvar = Chr$(CurrentIndex)
                CurrentIndex += 1
            End If
        ElseIf celt > 1 Then
            Dim Upper As Long
            If CurrentIndex + celt > MaxValue Then
                Upper = MaxValue - CurrentIndex
            Else
                Upper = celt - 1
            End If
            ReDim rgvar(0 To Upper)

            For Index = 0 To Upper
                rgvar(Index) = Chr$(CurrentIndex + Index)
            Next
        End If

        If VarPtr(pceltFetched) <> 0 Then
            pceltFetched = Index
        End If

        Err.ReturnHResult = If(celt > Index, S_FALSE, S_OK)
    End Sub

    Private Sub Skip(ByVal celt As Long) Implements IEnumVARIANT.Skip
        CurrentIndex += celt
        If CurrentIndex > MaxValue Then
            CurrentIndex = MaxValue + 1
        End If
    End Sub

    Private Sub Reset() Implements IEnumVARIANT.Reset
        CurrentIndex = MinValue
    End Sub

    Private Sub Clone(ByRef ppenum As IEnumVARIANT) Implements IEnumVARIANT.Clone
        Set ppenum = Me
    End Sub

    [ Enumerator ]
    Public Function _NewEnum() As IUnknown
        Return Me
    End Function
End Class

Public Module TestEnumerator
    Public Sub Test()
        Dim v As Variant
        Dim e As New TestCustomEnumeartor

        For Each v In e
            Debug.Print v
        Next
    End Sub
End Module
WaynePhillipsEA commented 2 years ago

Looks good @bclothier. I did something very similar in the WebView2Package. twinBASIC and VBx wont ever request more than one element at a time, so it will be fine for almost all uses.

WaynePhillipsEA commented 2 years ago

Actually I see that you're trying to handle the celt>1 case, but no that code is wrong as the rgvar input is technically a c-stlye array (not a SAFEARRAY) and so we can't define it properly in tB. I would just throw an error in the celt>1 case.

bclothier commented 2 years ago

Thanks for that note. I used this example code and was probably too hasty on the parts where they use SafeArrayGetElement, but reading it again, I see that it's meant to copy from an internal collection (m_psa which is not defined in the example) and returning a C-style array of Variants rather than a SAFEARRAY of Variants.

sancarn commented 2 years ago

@bclothier Should be weary of implementing IEnumVARIANT inside the same class as the _NewEnum function. In this case:

Debug.Print "Start"
Dim v1, v2
for each v1 in e
  for each v2 in e
    Debug.print v1,v2
  next
next
Debug.Print "End"

Will have unexpected behaviour:

Start
1,1
1,2
1,3
End

Where it should be:

Start
1,1
1,2
1,3
2,1
2,2
2,3
3,1
3,2
3,3
End
bclothier commented 2 years ago

I see what I did wrong. I shouldn't have called it TestCustomEnumeartor (sic). I should have called it LameHalfDonkeyedCustomEnumeratorOnlyGoodForDemostratingOneLevelOfEnumeration. 😄

That said, keep in mind that IEnumVARIANT::Clone is supposed to return a copy with the same state, so a full implementation would be more complicated as you'd need to share the state between the collection and the implementation of the IEnumVARIANT. Again, I don't know if anyone calls IEnumVARIANT::Clone but it's documented to return with the same state, rather than a reset state as in your example.

sancarn commented 2 years ago

I should have called it LameHalfDonkeyedCustomEnumeratorOnlyGoodForDemostratingOneLevelOfEnumeration

Haha apologies for being pedantic, i imagine you knew that already it was more for others reading this thread 😛

I don't know if anyone calls IEnumVARIANT::Clone

Most of the time it's not implemented 🤣 I call it a fair amount though in my custom classes. And indeed, correct that it should share the same state 😊

bclothier commented 2 years ago

NOTE: as noted above, we do have some capabilities to implement our own IEnumVARIANT, however, we still need to address the unresolved discussion about strong-typed & generic enumerators, which requires some language support to make more useful.

mburns08109 commented 2 years ago

Hey - just a question here - is there any really good reason why we couldn't add an EXISTS() method into any iEnumerable interface class definition (since this isn't necessarily a Vb6 backwards-compliant feature addition? - or is it?). If it could be made to work, then EVERY collection could get in an inherent EXISTS method if it were defined to be ienumerable.

Not sure if that would be a vb6-backwards-compatibility killer request or not.

After all, if VB6 code was always written without a .Exists() method, then there suddenly being one wouldn't necessarily break anything, would it (I mean, it'd be getting recompiled in tB anyway...so any binary compatibility issues shouldn't be a really big deal...should it?)

Greedquest commented 2 years ago

@mburns08109 If I understand the proposal correctly, then if you had been doing some reflection like IDispatch::GetTypeInfo then this would change. Also any code that hardcodes VTable offsets, the layout of the interface in memory would change. And any late bound code expecting Exists to fail at runtime only to find it succeeding would break.

EduardoVB commented 2 years ago

If I understand the proposal correctly, then if you had been doing some reflection like IDispatch::GetTypeInfo then this would change. Also any code that hardcodes VTable offsets, the layout of the interface in memory would change. And any late bound code expecting Exists to fail at runtime only to find it succeeding would break.

This comment makes me think that perhaps the backward compatibility should just be 99.999% and ask to adjust the other 0.001 of the existent codebase. More or less what VB6 did with regard to VB5 codebase.

Greedquest commented 2 years ago

@EduardoVB It's a question of how many implementation details that are not included in the VBA language specification do we want to keep, such as memory layout, interface hierarchy etc. 100% compatible could mean 100% in-keeping with the spec (I think this is a must) or 100% matching under-the hood implementation (probably impossible).

FullValueRider commented 2 years ago

Don't use 'Exists'. For Collections, twBasic could follow C#, but more succinctly, by using 'HoldsItem' and 'LacksItem' Methods. Similarly, and additionally, for Key/Value type collections, 'HoldsKey' and 'LacksKey'. This improves the readability whilst avoiding any conflict with any existing use of 'Exists'

sancarn commented 2 years ago

If I understand the proposal correctly, then if you had been doing some reflection like IDispatch::GetTypeInfo then this would change. Also any code that hardcodes VTable offsets, the layout of the interface in memory would change. And any late bound code expecting Exists to fail at runtime only to find it succeeding would break.

This comment makes me think that perhaps the backward compatibility should just be 99.999% and ask to adjust the other 0.001 of the existent codebase. More or less what VB6 did with regard to VB5 codebase.

There are alternative implementations which could be used to implement an Exists method tbf.

As for a good reason why exists maybe shouldn't be part of enumerable class - sometimes there are different specifications for exists e.g. Exists(callback) or Exists(key,value)... Etc.

EduardoVB commented 2 years ago

But that always happen. Also in VBx there are some methods, properties, etc with the same name and that has different parameters or value meanings in different objects. Since the Collection object never had the Exists method, how can that be a source of conflict (other than the VTable and whatnot)? And AFAIK there is no Exist in the intrinsic VBx language for any object. The dictionary object has it, like this:

Function Exists(Key) As Boolean

FullValueRider commented 2 years ago

As an aside, the Original Post states that custom enumerators are difficult in VBA, and I would agree that they are. However, as I have discovered, Custom Iterators are relatively straightforward and can be implemented using standard vanilla VBA (no need to resort to IENUMVariant). I've found that these iterators considerably simplify my code and a very flexible in use.

Consider the following contrived examples An IEnumVariant Based enumerator


Dim myDIctionary as Scripting.Dictionary
'<code>
Dim myKey as Variant
Dim myArray as Variant
Dim myIndex as long 
myIndex =0
Redim myArray(0 to myDictionary.count-1)
For Each myKey in myDictionary
    myArray(myIndex)=Cstr(myKey) & "," & Cstr(myDictionary(myKey)
    myIndex =myIndex+1
Next

And the Iterator version

Dim myDIctionary as Scripting.Dictionary
'<code>
Dim myArray as Variant
Redim myArray(0 to myDictionary.count-1)
Dim myI as IIterator=ItemsIter(myDictionary)
Do
    MyArray(myI.Index) =Cstr(myI.Key) & ',' & CStr(myI.Item)
Loop While MyI.MoveNext

The methods defined by the IIterator interface are (NB: Some of the methods exist purely as debugging aids)

    Function HasNext() As Boolean
    Function HasNoNext() As Boolean
    Function HasPrev() As Boolean
    Function HasNoPrev() As Boolean
    Function MoveNext() As Boolean
    Function MovePrev() As Boolean
    Function Item(Optional ipOffset As Long = 0) As Variant
    Function Index(Optional ipoffset As Long = 0) As Long
    Function Key(Optional ipOffset As Long = 0) As Variant
    Function SetSpan(Optional ByRef ipFromOrSpan As Variant = Empty, Optional ByRef ipTo As Variant = Empty, Optional ByRef ipstep As Variant = 1, Optional ByVal ipRank As Long = 1) As IIterator
    Function MoveToStart() As IIterator
    Function MoveToEnd() As IIterator
    Function Count() As Long
    Function TypeName() As String

I currently Have three Iterator classes working and two additional in development

Working IterConst - a sequence of a predefined value IterItems - iterates strings, Objects with an .Item Method, Objects with a .ToArray Method, Objects with a .Keys Method IterNum - a sequence of numbers defined by from/to/step

In development IterStrings - a sequence of strings composed of a defined set of characters e.g. aaaa,aaab,aaac,aaad etc IterArray - Iterates an array with more than one dimension

I'm happy to share code if anyone has an interest but please be advised that all of the above are 'in progress'.

mburns08109 commented 2 years ago

@mburns08109 If I understand the proposal correctly, then if you had been doing some reflection like IDispatch::GetTypeInfo then this would change. Also any code that hardcodes VTable offsets, the layout of the interface in memory would change. And any late bound code expecting Exists to fail at runtime only to find it succeeding would break.

I'd say that there would be an infintely close to 0.0 percent chance of that affecting the vast majority of VBA code in existence. So I'd be fairly comfortable with that risk profile.

bclothier commented 2 years ago

FYI, I have seen a number of such low-level hacks in VB6 forums and they most assuredly will break with such differences.

But then again, I'm inclined to say that can't fall under the compatibility purview because the hacks explicitly violates the abstractions (even if it's stupid) that is in the place. On the other hand, the hacks exist precisely because the original abstraction was stupid and getting in the way. I know Wayne already has put in some work to enable that certain classes of hacks will work (e.g. see inputbox which is often hacked) to avoid changing VB6 code that makes assumption about the inputbox being used.

mburns08109 commented 2 years ago

Yeah, I put this question in on this topic because iEnumerable was in scope of the topic. It's just always bothered me that VBx collections didn't have any .Exists(key) equivalent methods, and patching that glaring functionality hole while we're at this would be a really nice improvement to make (and for the record I'm not hung up on "Exists" as the method name if there is a better, more intuitive semantic name to be used - but everyone here understands what .Exists(x) does, which is really my point).

As for the question of whether adding any sort .Exists(x) functionality being a fit for the iEnumerable interface, I considered that, but where would be better to define it? Keep in mind that for iEnumerable-implementing classes, just because a .Exists(x) is defined in the interface, doesn't mean you have to actually implement it (other than having the method sig. defined so it matches the interface, of course). Naturally, though, having to have the method signature in your code would "help" the developer to remember to consider actually implementing it for custom collection classes. Having the method would just be so much better for using collections than not having it, though.

mburns08109 commented 2 years ago

So, do we have consensus enough that I should post this thought as an actual feature request?

bclothier commented 2 years ago

I see lot of ideas but no concrete proposal that can be acted upon.

To summarize the pain points:

1) We want to make it easy to implement custom collections by implementing IEnumerable, custom enumerators with IEnumerator and providing language support. That is, For Each will work natively with a class that implements IEnumerable.

2) We want to be able to make a strong-typed collections. In VBx, For Each will always involve casting because it uses non-generic IEnumVARIANT internally.

3) But, we want the ability to specify alternatives such as IEnumString instead of IEnumVARIANT for use with APIs.

It's not clear yet if we want base collection & generic collection class to be provided as part of the standard library. I'm inclined to think that it ought to be, offering opportunities for additional optimizations for most typical enumeration scenarios. Of course that doesn't prevent us from making a custom collection / enumerator for our specific needs. However, it does become much easier when you simply can extend one of such existing built-in collection classes instead of implementing it from scratch.

I would prefer an approach that introduces the minimal black magic; I don't want to be surprised with an IEnumString if I used an IEnumerable(Of String) when I was expecting a IEnumVARIANT. Requiring the class to specify the enumerator would remove any doubts over what enumerators it'll use, I think. However, I would prefer that the For Each works transparently with all the implementations and I do not have to worry about optimizing that particular code path.

mburns08109 commented 2 years ago

I see lot of ideas but no concrete proposal that can be acted upon.

To summarize the pain points:

  1. We want to make it easy to implement custom collections by implementing IEnumerable, custom enumerators with IEnumerator and providing language support. That is, For Each will work natively with a class that implements IEnumerable.
  2. We want to be able to make a strong-typed collections. In VBx, For Each will always involve casting because it uses non-generic IEnumVARIANT internally.
  3. But, we want the ability to specify alternatives such as IEnumString instead of IEnumVARIANT for use with APIs.

It's not clear yet if we want base collection & generic collection class to be provided as part of the standard library. I'm inclined to think that it ought to be, offering opportunities for additional optimizations for most typical enumeration scenarios. Of course that doesn't prevent us from making a custom collection / enumerator for our specific needs. However, it does become much easier when you simply can extend one of such existing built-in collection classes instead of implementing it from scratch.

I would prefer an approach that introduces the minimal black magic; I don't want to be surprised with an IEnumString if I used an IEnumerable(Of String) when I was expecting a IEnumVARIANT. Requiring the class to specify the enumerator would remove any doubts over what enumerators it'll use, I think. However, I would prefer that the For Each works transparently with all the implementations and I do not have to worry about optimizing that particular code path.

How about if For Each were able to use ANY of the iEumVARIANT, iEnumSTRING, iEnumLONG, etc. types generically? Would that not make the type of the iteration variable a largely moot point? - even if we state that iEnumVARIANT is the Default? ...or are you thinking about this from a code reflection POV that I'm not groking yet?

mansellan commented 2 years ago

Linking https://github.com/twinbasic/lang-design/issues/43.

Bear in mind that in .Net, the .Contains() method isn't part of the IEnumerable interface, it's an extension method (part of Linq) that can be "added" to it. This allows IEnumerable to focus strictly on only what's needed for enumeration, and have convenience methods loaded on top if you want. This is close to the ideal solution IMO.

mansellan commented 2 years ago

So, Clone and Skip are not part of .net's IEnumerable, but honestly I'm wondering if that's a mistake and those things are in fact fundamentally useful to enumerables (if rarely used).

An enumerable tracks where it is in a forward-only sequence. It's not inconceivable that you'd want to clone that state, so that you can advance at independent rates in multiple contexts (maybe different threads?). For skip, I can imagine that you might have a sequence that's expensive to read, but easy to skip ahead (for example a disk stream of fixed-length records).

Did .net over-simplify here, and if so can/should tB avoid doing the same?

mburns08109 commented 2 years ago

Hmm...interesting - Hadn't heard of Extension methods until just now. How would those be used in a tB/COM/ActiveX context? Have Extension methods been discussed for tB as a concept yet (and I missed it)?

Ah! wait, I see your link to #43 now

FullValueRider commented 2 years ago

For idle curiosity I tried the following

Class Collection

    Attribute VB_GlobalNameSpace = False
    Attribute VB_Creatable = True
    Attribute VB_PredeclaredId = True
    Attribute VB_Exposed = True

    Implements vba.collection Via Host = New vba.collection

    Public Function Exists(ByRef ipItem As Variant) As Boolean

        Dim myItem  As Long
        For myItem = 1 To Host.Count

            If Host(myItem) = ipItem Then
                Return True
            End If
        Next

        Return False

    End Function

End Class

which would appear to perfectly legal and functioning twinBasic

Sub TestCollectionOverride()

    Dim myC As Collection = New Collection
    With myC

        .Add 10
        .Add 20
        .Add 30
        .Add 50
        .Add 60

    End With

    Debug.Print myC.Exists(40)
    Debug.Print myC.Exists(60)

End Sub

CollectionOverride

mburns08109 commented 2 years ago

well damn with the collection name overloading, that literally does the trick, doesn't it? ...well, almost.

If Host(myItem) = ipItem Then

that's going to be problematic, but I still see what you did. ...and actually, there's a semantic issue here too...does exists(x) mean "does the value exist" or "does the key exist"?

wqweto commented 2 years ago

What I tried to do was to redeclare IVbCollection with the same [InterfaceId("A4C46780-499F-101B-BB78-00AA00383CBB")] and the same methods and then Implements IVbCollection Via Host = New VBA.Collection so that to be able to add Exists which just checks HRESULT as returned by Item method.

Unfortunately I'm not aware how to modify methods in TB from HRESULT to return simple Long i.e. non-raising errors like it's done with this IDL

    [
      odl,
      uuid(A4C46780-499F-101B-BB78-00AA00383CBB),
    ]
    interface IVbCollection  : IUnknown {
        HRESULT GetTypeInfoCount(
            [out, retval] int* pctinfo);

        LONG GetTypeInfo(
            [in] int itinfo,
            [in] long lcid,
            [out] void *pptinfo);

        LONG GetIDsOfNames(
            [in] void *riid,
            [in] LPWSTR *rgszNames,
            [in] int cNames,
            [in] long lcid,
            [out] long *rgdispid);

        LONG Invoke(
            [in] long dispidMember,
            [in] void *riid,
            [in] long lcid,
            [in] short wFlags,
            [in] void *pdispparams,
            [out] VARIANT *pvarResult,
            [out] void *pexcepinfo,
            [out] int *puArgErr);

        long Item(
            [in] VARIANT* Index, 
            [out, optional] VARIANT* pvarRet);

        long Add(
            [in] VARIANT* Item, 
            [in, optional] VARIANT* Key, 
            [in, optional] VARIANT* Before, 
            [in, optional] VARIANT* After);

        long Count([out] long* pcRet);
        long Remove([in] VARIANT* Index);
        long NewEnum([out] IUnknown** ppunk);
    };

All I need is Item method not to raise errors when the key is not found but return the HRESULT as a Long result like AddRef/Release return Long (which is normal for non-automation interfaces).

Unfortunately while concocting all this TB IDE choked so badly, froze for several seconds and then closed silently, in the mean time it deleted my .twinproj file which was so disappointing I had to immediately switch subject as I couldn't believe TB IDE is still so early alpha version. . . Anyway

FullValueRider commented 2 years ago

...and actually, there's a semantic issue here too...does exists(x) mean "does the value exist" or "does the key exist"?

Well I only posted a demonstrator not a keenly honed snippet of code guaranteed to be correct in all cases.