-
Notifications
You must be signed in to change notification settings - Fork 171
Add list.count #1676
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
Add list.count #1676
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Please add an integration test as well for list.count
. See test_list_*.py
in https://github.com/lcompilers/lpython/tree/main/integration_tests for the pattern of testing lists and use your creativity for writing strong test for list.count
(for example l.count
where l
can be empty, check l.count
after every insertion/deletion in a loop, etc.)
I would prefer an error. For testing error see https://github.com/lcompilers/lpython/tree/main/tests/errors and following pattern for registering error tests, Lines 66 to 68 in b35e744
|
I think your commits are including previous changes (already merged/present in latest |
To remove extra commits (already present in For example, first update your git checkout main
git pull Now switch to your branch and rebase: git checkout scratch
git rebase main Now force push it git push your_remote scratch --force-with-lease |
Thanks @Shaikh-Ubaid. Will use this in future. |
integration_tests/test_list_count.py
Outdated
|
||
assert x == [-5, -4, -3, -2, -1, 0, 1, 2, 3, 4] | ||
|
||
while len(x)>0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while len(x)>0: | |
while len(x) > 0: |
|
||
llvm::Value* left_arg = read_item(list, LLVM::CreateLoad(*builder, i), | ||
false, module, LLVM::is_llvm_struct(item_type)); | ||
llvm::Value* cond = llvm_utils->is_equal_by_value(left_arg, item, module, item_type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good that you figured it out on your own.
llvm_utils->start_new_block(loopend); | ||
|
||
return LLVM::CreateLoad(*builder, cnt); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now this is good. Later we should lift this into ASR and implement there, it will be a lot easier to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, thanks!
Adds
list.count
method. Completes a part of #941.