Skip to content

LLVM: Fix list.insert() when given index is greater than length of the list #1565

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

LLVM: Fix list.insert() when given index is greater than length of the list #1565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b9e666
fix list insert
faze-geek Mar 6, 2023
db42e12
change location of snippet
faze-geek Mar 6, 2023
063fb67
add a test
faze-geek Apr 9, 2023
7325e2a
update test
faze-geek Apr 10, 2023
7b751e7
investigate segmentation fault in list_03.py
faze-geek Apr 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions integration_tests/test_list_03.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,23 @@ def test_list_insert_02(x: list[i32], n: i32) -> list[i32]:

return x

def test_list_insert_03():
l1:list[str] = ["a", "b", "c", "d"]
l2:list[str] = []
l1.insert(5,"e")
s:str = "a"
i:i32 = 0
for i in range(len(l1)):
assert l1[i] == chr(ord(s)+i)

def test_list_insert_04():
l:list[i32] = [1, 2, 3, 4, 5]
i:i32
l.insert(6,6)
l.insert(100,7)
for i in range(1,8):
assert l[i-1] == i

def test_list_02(n: i32) -> i32:
x: list[i32] = [50, 1]
acc: i32 = 0
Expand Down Expand Up @@ -60,5 +77,7 @@ def verify():
assert test_list_01(11) == 55
assert test_list_02(50) == 3628
test_list_02_string()
test_list_insert_03()
test_list_insert_04()

verify()
16 changes: 13 additions & 3 deletions src/libasr/codegen/llvm_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2344,7 +2344,17 @@ namespace LCompilers {
llvm::Type* el_type = std::get<2>(typecode2listtype[type_code]);
resize_if_needed(list, current_end_point, current_capacity,
type_size, el_type, module);

llvm::Value* adjusted_pos = builder->CreateSelect(
builder->CreateICmpSGT(pos, current_end_point),
current_end_point,
pos
);
adjusted_pos = builder->CreateSelect(
builder->CreateICmpSGE(adjusted_pos, llvm::ConstantInt::get(context, llvm::APInt(32, 0))),
adjusted_pos,
llvm::ConstantInt::get(context, llvm::APInt(32, 0))
);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an issue with edge case. Note that current_end_point equals len(list) all the times.

lpython/src/libasr/codegen/llvm_utils.cpp

Lines 2252 to 2254 in 57ebbc8

llvm::Value* LLVMList::len(llvm::Value* list) {
return LLVM::CreateLoad(*builder, get_pointer_to_current_end_point(list));
}

/* While loop equivalent in C++:
* end_point // nth index of list
* pos // ith index to insert the element
Expand All @@ -2366,7 +2376,7 @@ namespace LCompilers {
// LLVMList should treat them as data members and create them
// only if they are NULL
llvm::AllocaInst *tmp_ptr = builder->CreateAlloca(el_type, nullptr);
LLVM::CreateStore(*builder, read_item(list, pos, false, *module, false), tmp_ptr);
LLVM::CreateStore(*builder, read_item(list, adjusted_pos, false, *module, false), tmp_ptr);
llvm::Value* tmp = nullptr;

// TODO: Should be created outside the user loop and not here.
Expand Down Expand Up @@ -2409,7 +2419,7 @@ namespace LCompilers {
// end
llvm_utils->start_new_block(loopend);

write_item(list, pos, item, asr_type, false, module, name2memidx);
write_item(list, adjusted_pos, item, asr_type, false, module, name2memidx);
shift_end_point_by_one(list);
}

Expand Down