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

[mypyc] Implement list insert primitive #9741

Merged
merged 5 commits into from
Nov 27, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions mypyc/lib-rt/CPy.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ bool CPyList_SetItem(PyObject *list, CPyTagged index, PyObject *value);
PyObject *CPyList_PopLast(PyObject *obj);
PyObject *CPyList_Pop(PyObject *obj, CPyTagged index);
CPyTagged CPyList_Count(PyObject *obj, PyObject *value);
CPyTagged CPyList_Insert(PyObject *list, CPyTagged index, PyObject *value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

return int here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

PyObject *CPyList_Extend(PyObject *o1, PyObject *o2);
PyObject *CPySequence_Multiply(PyObject *seq, CPyTagged t_size);
PyObject *CPySequence_RMultiply(CPyTagged t_size, PyObject *seq);
Expand Down
9 changes: 9 additions & 0 deletions mypyc/lib-rt/list_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ CPyTagged CPyList_Count(PyObject *obj, PyObject *value)
return list_count((PyListObject *)obj, value);
}

CPyTagged CPyList_Insert(PyObject *list, CPyTagged index, PyObject *value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

{
if (CPyTagged_CheckShort(index)) {
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
return PyList_Insert(list, n, value);
}
return -1;
}

PyObject *CPyList_Extend(PyObject *o1, PyObject *o2) {
return _PyList_Extend((PyListObject *)o1, o2);
}
Expand Down
8 changes: 8 additions & 0 deletions mypyc/primitives/list_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@
c_function_name='CPyList_Count',
error_kind=ERR_MAGIC)

# list.insert(index, obj)
method_op(
name='insert',
arg_types=[list_rprimitive, int_rprimitive, object_rprimitive],
return_type=int_rprimitive,
Copy link
Collaborator

Choose a reason for hiding this comment

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

the return type should be c_int_rprimitive since it corresponds to the low-level int.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense. Should be ok now.

c_function_name='CPyList_Insert',
error_kind=ERR_MAGIC)
Copy link
Collaborator

Choose a reason for hiding this comment

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

since the function PyList_Insert returns an int here, it is most reasonable to define the op with ERR_NEG_INT. See the append op in list_ops.py for an example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


# list * int
binary_op(
name='*',
Expand Down
15 changes: 15 additions & 0 deletions mypyc/test-data/irbuild-lists.test
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,18 @@ L0:
r2 = r1 >= 0 :: signed
r3 = truncate r1: int32 to builtins.bool
return r3

[case testListInsert]
from typing import List
def f(x: List[int], y: int) -> None:
x.insert(0, y)
[out]
def f(x, y):
x :: list
y :: int
r0 :: object
r1 :: int
L0:
r0 = box(int, y)
r1 = CPyList_Insert(x, 0, r0)
return 1
6 changes: 6 additions & 0 deletions mypyc/test-data/run-lists.test
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ pop(l, -2)
assert l == [1, 3]
assert count(l, 1) == 1
assert count(l, 2) == 0
l.insert(0, 0)
assert l == [0, 1, 3]
l.insert(2, 2)
assert l == [0, 1, 2, 3]
l.insert(4, 4)
assert l == [0, 1, 2, 3, 4]
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vsakkas This doesn't test the new primitive, since the code is in driver.py, which doesn't get compiled. Would you mind updating the test case? Also, it would be good to check negative indexes and index underflow and overflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course. I'll look into updating the tests in a new PR.


[case testListOfUserDefinedClass]
class C:
Expand Down