jakt icon indicating copy to clipboard operation
jakt copied to clipboard

Can't pass array slice to function taking array

Open lanmonster opened this issue 3 years ago • 5 comments

The following program:

function foo(anon xs: [i64]) {}

function main() {
    let xs = [0; 5]
    foo(xs[2..4])
}

fails to compile in the c++ stage with the following error:

build/test.cpp:13:1: error: no matching function for call to 'foo'
foo(((xs).slice_range(static_cast<i64>(2LL), static_cast<i64>(4LL))));
^~~
build/test.cpp:5:13: note: candidate function not viable: no known conversion from 'ArraySlice<long long>' to 'const Array<i64>' (aka 'const Array<long long>') for 1st argument
static void foo(const Array<i64> xs) {
            ^
1 error generated.

lanmonster avatar Aug 08 '22 06:08 lanmonster

Yeah ideally that would be supported. Was having a look if it worked currently with a generic params, seems that there is an issue currently with a generic param determining the iterable_type in typecheck_for. I am also adding a to_array for a slice as well, for creating an array from it.

robryanx avatar Aug 08 '22 09:08 robryanx

hmm, it actually is uncertain that it should be possible. instead you should either pass a kind of iterator range (does jakt have that) or a copy into a straight array. not sure if the iterator range passing can be done safely though

maddanio avatar Aug 31 '22 20:08 maddanio

Looking at the actual implementation of an array it does not seem safe to pass around slices. for that to be safe you would some kind of copy-on-write semantics...

maddanio avatar Aug 31 '22 20:08 maddanio

ah, I was wrong it is quite safe. so yes, in theory at least all const array arguments could easily be turned into slice arguments.

maddanio avatar Aug 31 '22 20:08 maddanio

Is there a reason why passing a ArraySlice is preferred to a Jakt::Span? In serenity code we pass things as "ReadOnlyBytes" (an alias to Span<u8 const> a lot.

edit: Ah I see, ArraySlice keeps a RefPtr to the Array's storage. hmm

ADKaster avatar Sep 01 '22 02:09 ADKaster