Fix ASR Slice frontend

[Smit-create]

Fixes #332

[Smit-create]

The current PR works fine with this:

def main0():
    arr: i32[4]
    arr[0] = 1
    arr[1] = 0
    arr[2] = -4
    print(arr[:2])
    print(arr[1])
    print(arr[2])

main0()

Output:

1
0
-4

0
-4

The only issue is that the Array Section backend considers the right limit as inclusive whereas other Section backends (List/string) consider the right limit as exclusive. So we need to define some strict way of defining indexes, limits that is followed uniformly in all the backends.

[czgdp1807]

The only issue is that the Array Section backend considers the right limit as inclusive whereas other

Well. Array is a shared feature between Fortran and Python. Fortran includes end index while slicing an array. However Python excludes end index. I think the backend should always treat the end index inclusive. Its the AST->ASR transition's job to format the indices of an array slices correctly. For example, array[0:3] should be converted to ArraySlice(array, 0, 2) in LPython's AST->ASR transition and ArraySlice(array, 0, 3) in LFortran's AST->ASR transition.

[czgdp1807]

Please add a test for your fix.

[czgdp1807]

Use assert for verifying that the outputs are actually correct?

[Smit-create]

I am not sure if we can assign Array Section at present (I think only string is supported).

    p: i32[4]
    arr[0] = 1
    arr[1] = 0
    arr[2] = -4
    p = arr[:2]

Throws this:

LFORTRAN_ASSERT(ASR::is_a<ASR::Character_t>(*ASRUtils::expr_type(x.m_v)) &&
AssertFailed: ASR::is_a<ASR::Character_t>(*ASRUtils::expr_type(x.m_v)) && n_dims == 0

[czgdp1807]

I am surprised why tests didn't fail after this change. Can you try applying this specific change to LFortran to make sure this works there as well.

[Smit-create]

Just because the frontend subtracts one and in backend (internally) we add one again. So, logic remains the same. And it's expected to pass all the tests.

[Smit-create]

Can you try applying this specific change to LFortran to make sure this works there as well.

Yep, tried. All tests passed for me. On the latest main branch of LFortran:

diff --git a/src/libasr/runtime/lfortran_intrinsics.c b/src/libasr/runtime/lfortran_intrinsics.c
index 17399c784..b71bfdd95 100644
--- a/src/libasr/runtime/lfortran_intrinsics.c
+++ b/src/libasr/runtime/lfortran_intrinsics.c
@@ -784,6 +784,7 @@ LFORTRAN_API char* _lfortran_str_copy(char* s, int32_t idx1, int32_t idx2) {
 LFORTRAN_API char* _lfortran_str_slice(char* s, int32_t idx1, int32_t idx2, int32_t step,
                         bool idx1_present, bool idx2_present) {
     int s_len = strlen(s);
+    idx2++;
     if (step == 0) {

[czgdp1807]

Just make a PR there (in LFortran) as well with a similar test. We will merge both together.

[czgdp1807]

Will shepherd this later.

[Smit-create]

Will shepherd this later.

@czgdp1807 Should we merge this? Can we add assertion tests as the follow-up PR to keep this clean?

[certik]

@czgdp1807 let me know what needs to get done here.

[czgdp1807]

Just make a PR there (in LFortran) as well with a similar test. We will merge both together.

[Smit-create]

This is ready @czgdp1807

[czgdp1807]

Usage of assert possible here to check if the outputs are actually correct?

Suggested change

	print(arr[:2])
	print(arr[1])
	print(arr[2])
	print(arr[:2])
	print(arr[1])
	print(arr[2])
	arr_2: i32[2] = arr[:2]
	assert arr_2[0] == 1
	assert arr_2[1] == 0
	assert arr[1] == 0
	assert arr[2] == -4

[Smit-create]

Not yet:

File "/Users/thebigbool/repos/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 1658
    LFORTRAN_ASSERT(ASR::is_a<ASR::Character_t>(*ASRUtils::expr_type(x.m_v)) &&
AssertFailed: ASR::is_a<ASR::Character_t>(*ASRUtils::expr_type(x.m_v)) && n_dims == 0

[czgdp1807]

Makes sense. Can you try fixing the bug?

[Smit-create]

I think the LLVM backend currently only supports string-slicing assignments and not arrays because they can be n-dimensional. Is that intentional?

[czgdp1807]

Please fix #1051 (comment) and resolve the conflicts. Will merge it then.