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

Some Builtins expect Array type which is not compatible with ArrayOverBuffer #4843

Closed
wdanilo opened this issue Feb 5, 2023 · 0 comments
Closed
Assignees
Labels
--bug Type: bug -syntax p-low Low priority

Comments

@wdanilo
Copy link
Member

wdanilo commented Feb 5, 2023

This task is automatically imported from the old Task Issue Board and it was originally created by Radosław Waśko.
Original issue is here.


Builtins like Polyglot.invoke expect an Array which works only with the base Enso Array type, but it won't work with ArrayOverBuffer which is returned by File.read_last_bytes. Similarly, ArrayOverBuffer matches the Array type but does not expose the sort method.

ArrayOverBuffer was created as an optimization to avoid copying from the buffer when reading the file bytes. However, it is not implemented fully, as when implementing it originally we were not aware of these additional issues.

We should either remove ArrayOverBuffer or make it compatible with Array builtins so that it can be used interchangeably. Possibly by creating some common array superclass.

This issue may very likely be fixed by #183000876, but we track it separately to make sure that it indeed gets fixed.

Steps to reproduce

Create a file aob.enso with following contents and then run it:

from Standard.Base import all

run a =
    IO.println "Running with "+a.to_text
    case a of
         _ : Array -> IO.println "Matches Array"
         _ -> IO.println "Does not match array :("

    Panic.catch Any a.sort caught_panic->
        IO.println "Error "+caught_panic.to_text
    IO.println "After sort: "+a.to_text

    obj = mk_object IO.println
    Panic.catch Any (Polyglot.invoke obj "foo" a) caught_panic->
        IO.println "Invoke Error "+caught_panic.to_text

    IO.println ""

foreign js mk_object printer = """
    return {"foo": function(x, y) { printer("x=" + x + "; y=" + y); }}

main =
    arr = [3,2,1].to_array
    aob = (File.new "aob.enso" . read_last_bytes 10).to_array
 
    obj = mk_object IO.println
    obj.foo "main" "XYZ"
   
    run arr
    run aob

Actual

x=main; y=XYZ
Running with [3, 2, 1]
Matches Array
After sort: [3, 2, 1]
x=3; y=2

Running with org.enso.interpreter.runtime.data.ArrayOverBuffer@7074da1d
Matches Array
Error (Caught_Panic (No_Such_Method_Error org.enso.interpreter.runtime.data.ArrayOverBuffer@7074da1d UnresolvedSymbol<sort>) (No_Such_Method_Error org.enso.interpreter.runtime.data.ArrayOverBuffer@7074da1d UnresolvedSymbol<sort>))
After sort: org.enso.interpreter.runtime.data.ArrayOverBuffer@7074da1d
Invoke Error (Caught_Panic (Type_Error Array org.enso.interpreter.runtime.data.ArrayOverBuffer@7074da1d 'arg3') (Type_Error Array org.enso.interpreter.runtime.data.ArrayOverBuffer@7074da1d 'arg3'))

Expected

x=main; y=XYZ
Running with [3, 2, 1]
Matches Array
After sort: [3, 2, 1]
x=3; y=2

Running with [32, 32, 114, 117, 110, 32, 97, 111, 98, 10]
Matches Array
After sort: [10, 32, 32, 32, 97, 98, 110, 111, 114, 117]
x=10; y=32

Blockers:

#183266964 resolved

Comments:

There is #183266964 talking about Polyglot.invoke as well. Yes, we need a general solution to make our builtins work with any type of array. (jaroslavtulach - Sep 19, 2022)


We no longer throw an exception due to https://github.com//pull/3817 (Hubert Plociniczak - Jan 4, 2023)
**Hubert Plociniczak** reports a new **STANDUP** for yesterday (2023-01-04):

Progress: Investigating issues with ArrayOverBuffer. Looking at Array/Vector consolidation. It should be finished by 2023-01-05.

Next Day: Next day I will be working on the #183058716 task. Address PR reviews. (Enso Bot - Jan 5, 2023)


**Hubert Plociniczak** reports a new **STANDUP** for yesterday (2023-01-05):

Progress: Extended the scope of the ticket to also tackle Array.sort mutation. If we insisted on mutation in place, supporting sort for polyglot arrays would become impractical so decided to bring it in line with Vector.sort. It should be finished by 2023-01-05.

Next Day: Next day I will be working on the #183217534 task. Catching up on what's happening with widgets and continuing on consolidation. (Enso Bot - Jan 6, 2023)


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -syntax p-low Low priority
Projects
None yet
Development

No branches or pull requests

2 participants