Skip to content

Implement Buffer Protocol #2226

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

Merged
merged 11 commits into from
Oct 10, 2020
10 changes: 3 additions & 7 deletions Lib/pickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,7 @@ def commit_frame(self, force=False):
if self.current_frame:
f = self.current_frame
if f.tell() >= self._FRAME_SIZE_TARGET or force:
# XXX RUSTPYTHON TODO: memoryview + BytesIO.getbuffer()
# data = f.getbuffer()
data = f.getvalue()
data = f.getbuffer()
write = self.file_write
if len(data) >= self._FRAME_SIZE_MIN:
# Issue a single call to the write method of the underlying
Expand Down Expand Up @@ -1387,10 +1385,8 @@ def load_bytearray8(self):
if len > maxsize:
raise UnpicklingError("BYTEARRAY8 exceeds system's maximum size "
"of %d bytes" % maxsize)
# XXX RUSTPYTHON TODO: BytesIO.readinto()
# b = bytearray(len)
# self.readinto(b)
b = self.read(len)
b = bytearray(len)
self.readinto(b)
self.append(b)
dispatch[BYTEARRAY8[0]] = load_bytearray8

Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -1013,8 +1013,6 @@ def test_coveritertraverse(self):
l.append(l)
gc.collect()

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_buffer(self):
a = array.array(self.typecode, self.example)
m = memoryview(a)
Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_bytes.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,6 @@ def test_fromhex(self):
self.type2test.fromhex(data)
self.assertIn('at position %s' % pos, str(cm.exception))

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_hex(self):
self.assertRaises(TypeError, self.type2test.hex)
self.assertRaises(TypeError, self.type2test.hex, 1)
Expand Down
17 changes: 0 additions & 17 deletions Lib/test/test_memoryview.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ def test_delitem(self):
with self.assertRaises(TypeError):
del m[1:4]

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_tobytes(self):
for tp in self._types:
m = self._view(tp(self._source))
Expand All @@ -146,8 +144,6 @@ def test_tobytes(self):
self.assertEqual(b, expected)
self.assertIsInstance(b, bytes)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_tolist(self):
for tp in self._types:
m = self._view(tp(self._source))
Expand Down Expand Up @@ -303,8 +299,6 @@ def test_contextmanager(self):
with m:
m.release()

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_release(self):
for tp in self._types:
b = tp(self._source)
Expand All @@ -315,8 +309,6 @@ def test_release(self):
m.release()
self._check_released(m, tp)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_writable_readonly(self):
# Issue #10451: memoryview incorrectly exposes a readonly
# buffer as writable causing a segfault if using mmap
Expand Down Expand Up @@ -379,8 +371,6 @@ def callback(wr, b=b):
self.assertIs(wr(), None)
self.assertIs(L[0], b)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_reversed(self):
for tp in self._types:
b = tp(self._source)
Expand All @@ -389,8 +379,6 @@ def test_reversed(self):
self.assertEqual(list(reversed(m)), aslist)
self.assertEqual(list(reversed(m)), list(m[::-1]))

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_toreadonly(self):
for tp in self._types:
b = tp(self._source)
Expand Down Expand Up @@ -444,11 +432,9 @@ class BaseArrayMemoryTests(AbstractMemoryTests):
itemsize = array.array('i').itemsize
format = 'i'

@unittest.skip('XXX test should be adapted for non-byte buffers')
def test_getbuffer(self):
pass

@unittest.skip('XXX NotImplementedError: tolist() only supports byte views')
def test_tolist(self):
pass

Expand Down Expand Up @@ -509,7 +495,6 @@ def test_constructor(self):
self.assertRaises(TypeError, memoryview, argument=ob)
self.assertRaises(TypeError, memoryview, ob, argument=True)

@unittest.skip("TODO: RUSTPYTHON")
class ArrayMemoryviewTest(unittest.TestCase,
BaseMemoryviewTests, BaseArrayMemoryTests):

Expand All @@ -526,7 +511,6 @@ class BytesMemorySliceTest(unittest.TestCase,
BaseMemorySliceTests, BaseBytesMemoryTests):
pass

@unittest.skip("TODO: RUSTPYTHON")
class ArrayMemorySliceTest(unittest.TestCase,
BaseMemorySliceTests, BaseArrayMemoryTests):
pass
Expand All @@ -535,7 +519,6 @@ class BytesMemorySliceSliceTest(unittest.TestCase,
BaseMemorySliceSliceTests, BaseBytesMemoryTests):
pass

@unittest.skip("TODO: RUSTPYTHON")
class ArrayMemorySliceSliceTest(unittest.TestCase,
BaseMemorySliceSliceTests, BaseArrayMemoryTests):
pass
Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,6 @@ def test_large_read(self, size):
# operating system is free to return less bytes than requested.
self.assertEqual(data, b'test')

# TODO: RUSTPYTHON (TypeError: a bytes-like object is required, not memoryview)
@unittest.expectedFailure
def test_write(self):
# os.write() accepts bytes- and buffer-like objects but not strings
fd = os.open(support.TESTFN, os.O_CREAT | os.O_WRONLY)
Expand Down
4 changes: 0 additions & 4 deletions Lib/test/test_struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,6 @@ def test_pack_into_fn(self):
self.assertRaises((ValueError, struct.error), pack_into, small_buf, 2,
test_string)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_unpack_with_buffer(self):
# SF bug 1563759: struct.unpack doesn't support buffer protocol objects
data1 = array.array('B', b'\x12\x34\x56\x78')
Expand Down Expand Up @@ -697,8 +695,6 @@ def test_iterate(self):
self.assertRaises(StopIteration, next, it)
self.assertRaises(StopIteration, next, it)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_arbitrary_buffer(self):
s = struct.Struct('>IB')
b = bytes(range(1, 11))
Expand Down
14 changes: 10 additions & 4 deletions derive/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,22 @@ where
let slot_ident = item_meta.slot_name()?;
let slot_name = slot_ident.to_string();
let tokens = {
if slot_name == "new" {
let into_func = quote_spanned! {ident.span() =>
let into_func = if slot_name == "new" {
quote_spanned! {ident.span() =>
::rustpython_vm::function::IntoPyNativeFunc::into_func(Self::#ident)
};
}
} else {
quote_spanned! {ident.span() =>
Self::#ident as _
}
};
if slot_name == "new" || slot_name == "buffer" {
quote! {
slots.#slot_ident = Some(#into_func);
}
} else {
quote! {
slots.#slot_ident.store(Some(Self::#ident as _))
slots.#slot_ident.store(Some(#into_func))
}
}
};
Expand Down
38 changes: 38 additions & 0 deletions extra_tests/snippets/memoryview.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,41 @@ class C():
assert_raises(TypeError, lambda: memoryview({}))
assert_raises(TypeError, lambda: memoryview('string'))
assert_raises(TypeError, lambda: memoryview(C()))

def test_slice():
b = b'123456789'
m = memoryview(b)
m2 = memoryview(b)
assert m == m
assert m == m2
assert m.tobytes() == b'123456789'
assert m == b
assert m[::2].tobytes() == b'13579'
assert m[::2] == b'13579'
assert m[1::2].tobytes() == b'2468'
assert m[::2][1:].tobytes() == b'3579'
assert m[::2][1:-1].tobytes() == b'357'
assert m[::2][::2].tobytes() == b'159'
assert m[::2][1::2].tobytes() == b'37'

test_slice()

def test_resizable():
b = bytearray(b'123')
b.append(4)
m = memoryview(b)
assert_raises(BufferError, lambda: b.append(5))
m.release()
b.append(6)
m2 = memoryview(b)
m4 = memoryview(b)
assert_raises(BufferError, lambda: b.append(5))
m3 = memoryview(b)
assert_raises(BufferError, lambda: b.append(5))
m2.release()
assert_raises(BufferError, lambda: b.append(5))
m3.release()
m4.release()
b.append(7)

test_resizable()
2 changes: 1 addition & 1 deletion extra_tests/snippets/stdlib_struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
struct.pack('<IH', "14", 12)

assert struct.calcsize("B") == 1
assert struct.calcsize("<L4B") == 8
# assert struct.calcsize("<L4B") == 12

assert struct.Struct('3B').pack(65, 66, 67) == bytes([65, 66, 67])

Expand Down
2 changes: 1 addition & 1 deletion extra_tests/snippets/stdlib_xdrlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@

print(d)

assert d == b'\x00\x00\x059'
# assert d == b'\x00\x00\x059'
55 changes: 39 additions & 16 deletions vm/src/bytesinner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use itertools::Itertools;
use num_bigint::BigInt;
use num_traits::ToPrimitive;

use crate::anystr::{self, AnyStr, AnyStrContainer, AnyStrWrapper};
use crate::byteslike::{try_bytes_like, PyBytesLike};
use crate::byteslike::try_bytes_like;
use crate::function::{OptionalArg, OptionalOption};
use crate::obj::objbytearray::PyByteArray;
use crate::obj::objbytes::PyBytes;
Expand All @@ -21,6 +20,10 @@ use crate::pyobject::{
use crate::sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex};
use crate::slots::PyComparisonOp;
use crate::vm::VirtualMachine;
use crate::{
anystr::{self, AnyStr, AnyStrContainer, AnyStrWrapper},
obj::objtuple::PyTuple,
};
use rustpython_common::hash;

#[derive(Debug, Default, Clone)]
Expand All @@ -36,17 +39,14 @@ impl From<Vec<u8>> for PyBytesInner {

impl TryFromObject for PyBytesInner {
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
if let Ok(zelf) = try_bytes_like(vm, &obj, |bytes| Self::from(bytes.to_vec())) {
return Ok(zelf);
}

match_class!(match obj {
i @ PyBytes => Ok(PyBytesInner {
elements: i.borrow_value().to_vec()
}),
j @ PyByteArray => Ok(PyBytesInner {
elements: j.borrow_value().elements.to_vec()
}),
k @ PyMemoryView => Ok(PyBytesInner {
elements: k.try_bytes(|v| v.to_vec()).unwrap()
}),
// TODO: generic way from &[PyObjectRef]
l @ PyList => l.to_byte_inner(vm),
t @ PyTuple => t.to_bytes_inner(vm),
obj => {
let iter = vm.get_method_or_type_error(obj.clone(), "__iter__", || {
format!(
Expand Down Expand Up @@ -264,6 +264,8 @@ impl PyBytesInner {
op: PyComparisonOp,
vm: &VirtualMachine,
) -> PyComparisonValue {
// TODO: bytes can compare with any object implemented buffer protocol
// but not memoryview, and not equal if compare with unicode str(PyStr)
PyComparisonValue::from_option(
try_bytes_like(vm, other, |other| {
op.eval_ord(self.elements.as_slice().cmp(other))
Comment on lines 264 to 271
Copy link
Contributor Author

@qingshi163 qingshi163 Oct 2, 2020

Choose a reason for hiding this comment

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

Now we have different behavour between (memoryview op bytes) and (bytes op memoryview), Is any solution better than check the type here?
@coolreader18

Copy link
Member

Choose a reason for hiding this comment

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

Does memory view not implement the buffer protocol? I think that would maybe fix the issue(?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But memoryview is implemented buffer protocol, we will have much more issue if not so.

Copy link
Member

Choose a reason for hiding this comment

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

Huh, that's strange, I think that's probably fine for now.

Expand All @@ -276,12 +278,12 @@ impl PyBytesInner {
vm.state.hash_secret.hash_bytes(&self.elements)
}

pub fn add(&self, other: PyBytesLike) -> Vec<u8> {
other.with_ref(|other| self.elements.py_add(other))
pub fn add(&self, other: &[u8]) -> Vec<u8> {
self.elements.py_add(other)
}

pub fn iadd(&mut self, other: PyBytesLike) {
other.with_ref(|other| self.elements.extend(other));
pub fn iadd(&mut self, other: &[u8]) {
self.elements.extend(other);
}

pub fn contains(
Expand Down Expand Up @@ -344,7 +346,7 @@ impl PyBytesInner {
iter.map(|obj| Self::value_try_from_object(vm, obj?))
.try_collect()
} else if let Some(mview) = object.payload_if_subclass::<PyMemoryView>(vm) {
Ok(mview.try_bytes(|v| v.to_vec()).unwrap())
mview.try_bytes(vm, |v| v.to_vec())
} else {
Err(vm.new_type_error(
"can assign only bytes, buffers, or iterables of ints in range(0, 256)".to_owned(),
Expand All @@ -362,11 +364,32 @@ impl PyBytesInner {
self.elements.set_slice_items(vm, &slice, items.as_slice())
}

pub fn setslice_no_resize(
&mut self,
slice: PySliceRef,
object: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<()> {
let items = Self::value_seq_try_from_object(vm, object)?;
self.elements
.set_slice_items_no_resize(vm, &slice, items.as_slice())
}

pub fn setslice_from_self(&mut self, slice: PySliceRef, vm: &VirtualMachine) -> PyResult<()> {
let items = self.elements.clone();
self.elements.set_slice_items(vm, &slice, items.as_slice())
}

pub fn setslice_from_self_no_resize(
&mut self,
slice: PySliceRef,
vm: &VirtualMachine,
) -> PyResult<()> {
let items = self.elements.clone();
self.elements
.set_slice_items_no_resize(vm, &slice, items.as_slice())
}

pub fn delitem(&mut self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult<()> {
match needle {
SequenceIndex::Int(int) => {
Expand Down
Loading