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

stdLambda stack issue #68

Open
sancarn opened this issue Mar 10, 2023 · 0 comments
Open

stdLambda stack issue #68

sancarn opened this issue Mar 10, 2023 · 0 comments
Labels
enhancement New feature or request lib-stdLambda

Comments

@sancarn
Copy link
Owner

sancarn commented Mar 10, 2023

Currently in evaluate:

            Case iType.oAccess
                Select Case op.subType
                    Case ISubType.argument
                        Dim iArgIndex As Long: iArgIndex = val(mid(op.value, 2)) + LBound(vLastArgs) - 1
                        If iArgIndex <= UBound(vLastArgs) Then
                            Call pushV(stack, stackPtr, vLastArgs(iArgIndex))
                        Else
                            Call Throw("Argument " & iArgIndex & " not supplied to Lambda.")
                        End If
                    Case Else
                        Call pushV(stack, stackPtr, stack(stackPtr - op.value))

Call pushV(stack, stackPtr, stack(stackPtr - op.value)) can sometimes error when the variable at stack(stackPtr - op.value) is an object. See explanation for details. As a temporary work around we can do the following:

 '@FIX bug where if stack(stackPtr-op.value) was an object, then array will become locked. Array locking occurs by compiler to try to protect
'instances when re-allocation would move the array, and thus corrupt the pointers. By copying the variant we redivert the compiler's efforts,
'but we might actually open ourselves to errors...
Dim vAccessVar: Call CopyVariant(vAccessVar, stack(stackPtr - op.value))
Call pushV(stack, stackPtr, vAccessVar)

This may still break so we need to consider a better solution. Our suggestion is to use a Collection (i.e. linked list) instead of a dynamic array. This should have fewer issues but might sacrifice on some performance.


Explanation:

Minimal examples where error occurs:

Sub Example()
    'Initialize the array with 3 elements
    Dim myArray() As string
    ReDim myArray(0)
    Set myArray(0) = "yop"
    Call Push(myArray, myArray(0))
End Sub

Sub Push(ByRef myArray() As string, ByRef element As string)
    'Lock the array to prevent deallocation of memory for the reference parameter
    ReDim Preserve myArray(UBound(myArray) + 1)
    If IsObject(element) Then
      Set myArray(UBound(myArray)) = element
    Else
      myArray(UBound(myArray)) = element
    End If
End Sub

The issue is that element, in this case is a reference to an element within myArray... The solution to above is change element to ByVal:

Sub Push(ByRef myArray() As string, ByVal element As string)

When extended to the variant case however:

Sub Push(ByRef myArray() As variant, ByVal element As variant)

Objects are also pointers... so

Sub Example()
    'Initialize the array with 3 elements
    Dim myArray() As Variant
    ReDim myArray(0)
    Set myArray(0) = CreateObject("scripting.dictionary")
    Call Push(myArray, myArray(0))
End Sub

Sub Push(ByRef myArray() As Variant, ByVal element As Variant)
    'Lock the array to prevent deallocation of memory for the reference parameter
    ReDim Preserve myArray(UBound(myArray) + 1)
    If IsObject(element) Then
      Set myArray(UBound(myArray)) = element
    Else
      myArray(UBound(myArray)) = element
    End If
End Sub

Also errors because we have a pointer directly to myArray(0) object. So the real work around is

Sub Example()
    'Initialize the array with 3 elements
    Dim myArray() As Variant
    ReDim myArray(0)
    Set myArray(0) = CreateObject("scripting.dictionary")
    Dim v: CopyVariant(v, myArray(0))
    Call Push(myArray, v)
End Sub
@sancarn sancarn added enhancement New feature or request lib-stdLambda labels Oct 28, 2023
@sancarn sancarn added this to Roadmap Jun 3, 2024
@sancarn sancarn moved this to Feature Request/Unknown requirement in Roadmap Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lib-stdLambda
Projects
Status: Feature Request/Unknown requirement
Development

No branches or pull requests

1 participant