| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I just pushed a commit to get stuff compiling. Excited to review this! |
Sorry, something went wrong.
|
One thing to note is that some things are directly translation from PyPy, others from CPython's. Also there's that difference I said on Gitter, regarding declaring a class like: class Custom(_SimpleCData):
pass
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: class must define a '_type_' attributewhich here will succeed. |
Sorry, something went wrong.
There was a problem hiding this comment.
Here's what I have so far; I think it would be better to use more of libffi's middle API in ctypes::function, just to avoid as much unsafe as possible for something like ctypes 🙃 . Now that the functions are copied from cpython, it should be easier to refactor into something more idiomatic.
Sorry, something went wrong.
| #[pyclass(module = "_ctypes", name = "_CData")] | ||
| pub struct PyCData { | ||
| _objects: AtomicCell<Vec<PyObjectRef>>, | ||
| _buffer: PyRwLock<Vec<u8>>, |
There was a problem hiding this comment.
I think the buffer is supposed to be a raw pointer and a length, so that you can mutate the original data it if you want to.
Sorry, something went wrong.
There was a problem hiding this comment.
Something like
struct RawBuffer{
inner: *mut u8,
size: usize
}?
Sorry, something went wrong.
| #[cfg(any(unix, windows, target_os = "wasi"))] | ||
| modules.insert("_ctypes".to_owned(), Box::new(ctypes::make_module)); |
There was a problem hiding this comment.
instead of duplicating same cfg, it can be grouped like:
#[cfg(any(unix, windows, target_os = "wasi"))]
{
modules.insert(os::MODULE_NAME.to_owned(), Box::new(os::make_module));
modules.insert("_ctypes".to_owned(), Box::new(ctypes::make_module));
}
Sorry, something went wrong.
There was a problem hiding this comment.
Although I don't know if ctypes would be available on wasi, I don't think it has dll functionality yet
Sorry, something went wrong.
There was a problem hiding this comment.
Would libffi work?
Sorry, something went wrong.
There was a problem hiding this comment.
No, wasm doesn't really have any sort of support for loading another wasm module at runtime
Sorry, something went wrong.
There was a problem hiding this comment.
Maybe interesting, Blazor does load DLLs at runtime from what I can see (from this blog). How feasible is something similar for Rust?
Sorry, something went wrong.
| } | ||
| void => { | ||
| ptr::null_mut() | ||
| arg(&ptr::null::<usize>()) |
There was a problem hiding this comment.
I've replace this for
ptr::null::<c_void>()since it seemed more "correct", which I'changed after committing this. So in a next change batch it will be replaced.
Sorry, something went wrong.
| fn obj_bytes(&self) -> BorrowedValue<[u8]> { | ||
| PyRwLockReadGuard::map(self.data.borrow_value(), |x| x.as_slice()).into() | ||
| PyRwLockReadGuard::map(self.data.borrow_value(), |x| unsafe { | ||
| slice::from_raw_parts(x.inner, x.size) |
There was a problem hiding this comment.
I'm not sure if this is copying things
Sorry, something went wrong.
There was a problem hiding this comment.
no, it isn't. slice is a sort of view
Sorry, something went wrong.
| &self.options | ||
| } | ||
| } | ||
| pub struct RawBuffer { |
There was a problem hiding this comment.
@coolreader18 is this what you have in mind?
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, it is.
Sorry, something went wrong.
There was a problem hiding this comment.
The safe expression of this type in Rust is Box<[u8]> which is exactly equivalent to this struct. Is this intended work to avoid somethiing like ownership check? I think there must be a better way.
Sorry, something went wrong.
| } | ||
| pointer => { | ||
| usize::try_from_object(vm, obj).unwrap() as *mut c_void | ||
| arg(&usize::try_from_object(vm, obj).unwrap()) |
There was a problem hiding this comment.
Also replace this with
arg(&(usize::try_from_object(vm, obj)? as *mut usize as *mut c_void))
Sorry, something went wrong.
| buffer | ||
| .obj_bytes() | ||
| .chunks(4) | ||
| .map(|c| NativeEndian::read_u32(c)) |
There was a problem hiding this comment.
You can use u32::from_ne_bytes for this; also, there's already a byteorder dependency in vm/Cargo.toml
Sorry, something went wrong.
There was a problem hiding this comment.
Also, what function is this supposed to be? CArray doesn't have a value property from what I can see
Sorry, something went wrong.
There was a problem hiding this comment.
You can use u32::from_ne_bytes for this; also, there's already a byteorder dependency in vm/Cargo.toml
Oh boy, I missed that one, I've found the btand le counterparts. I can simply ignore the whole ByteOrder crate, then.
Also, what function is this supposed to be? CArray doesn't have a value property from what I can see
These properties come from it's metaclass (PyCArrayType_Type), which are CharArray_getsets and WCharArray_getsets
Sorry, something went wrong.
| { | ||
| fn obj_bytes(&self) -> BorrowedValue<[u8]> { | ||
| // @TODO: This is broken | ||
| PyRwLockReadGuard::map(self.data.borrow_value(), |x| unsafe { |
There was a problem hiding this comment.
This need to be fixed, "casting" the associated type somehow
Sorry, something went wrong.
|
What are the major blockers for this PR? I think I want to give this a try :) |
Sorry, something went wrong.
Unfortunately there are several problems with the state of things and I haven't been able to continue working on this (and will not be able to do so for a while). The main problem is how it behaves differently from CPython's and PyPy's, a meta class should be added for Struct, CSimpleType, Pointer, ... (all but CFuntion), so that you can do c_int * 10 and return a proper array. In the current impl type(CSimpleType) == type, which does not has a __mul__ as required for a c_int, for instance. |
Sorry, something went wrong.
|
@youknowone I tried to sync with master a while ago, but there was a problem and I got too busy to look at that again, so thanks for that! Are you planning into work on this? If so, I think there are some tests I ported from CPython that I altered just to see if the implementation was working-ish. To be fully compatible to CPython and PyPy, the types must have a metaclass that's not PyType. I tried to work something there, but I couldn't find a solution. This happens because a base type has to implement some ops at type level, eg, c_int * 8 should call __mul__ from its meta (a yet non-existing class PrimitiveTypeMeta which is a subclass of PyType) and return a c_array. I am happy to help with discussion and pointers, but I'm afraid I don't have much time to code :/ |
Sorry, something went wrong.
|
@darleybarreto Hi, thank you for the comment. I am not planning for working on ctypes at the moment, but looking for a way to merge it as it is for future contributors - because you already worked really a lot of it - if it doesn't break things too much. How do you think about it? |
Sorry, something went wrong.
|
You mean merging as is? |
Sorry, something went wrong.
|
yes, if we can manage it not to break CI |
Sorry, something went wrong.
|
I think that we need to do a few things to help future contributions in this code before merging:
Undoubtedly 1 is the major blocker here. Since I don't know much of the internals of RustPython, I wasn't able to solve this problem. Perhaps you can give me pointers and I can work it out. PyPy does is basically this way: class _CDataMeta(type):
...
def __mul__(self, length):
return create_array_type(self, length)
...
class SimpleType(_CDataMeta):
...
class SimpleCData(..., metaclass=SimpleType):
...I couldn't find a way to make type(SimpleCData) be SimpleType instead of type. Point 2 is extremely important since this code is basically too experimental and not that much well written. I could gladly do it based on PyPy, CPython and documentations. |
Sorry, something went wrong.
|
Thank you for detailed explanation. The metaclass issue is recently solved. Here is the test: $ cat t.py
class _CDataMeta(type):
def __mul__(self, length):
return create_array_type(self, length)
class SimpleType(_CDataMeta):
pass
class SimpleCData(object, metaclass=SimpleType):
pass
print(type(SimpleCData))
assert type(SimpleCData) == SimpleType
$ cargo run t.py
Finished dev [unoptimized + debuginfo] target(s) in 0.06s
Running `target/debug/rustpython t.py`
<class '__main__.SimpleType'>I will look in the code for 3 |
Sorry, something went wrong.
|
Oh, I'm sorry, I meant to do the same thing in the Rust side. CPython does it in the C side of things by manually filling the type: #define MOD_ADD_TYPE(TYPE_EXPR, TP_TYPE, TP_BASE) \
do { \
PyTypeObject *type = (TYPE_EXPR); \
Py_SET_TYPE(type, TP_TYPE); \
type->tp_base = TP_BASE; \
if (PyModule_AddType(mod, type) < 0) { \
return -1; \
} \
} while (0)
MOD_ADD_TYPE(&Simple_Type, &PyCSimpleType_Type, &PyCData_Type);Here PyCSimpleType_Type is the meta and PyCData_Type is the base class. |
Sorry, something went wrong.
| } | ||
| } | ||
|
|
||
| fn slice_adjust_size(length: isize, start: &mut isize, stop: &mut isize, step: isize) -> isize { |
There was a problem hiding this comment.
I didn't compare precisely, but this function might be a duplication of inner_indices in slice.rs
Sorry, something went wrong.
There was a problem hiding this comment.
They have some resemblance, but are different nonetheless.
Sorry, something went wrong.
| _type_: new_simple_type(Either::A(&outer_type), vm)?.into_ref(vm), | ||
| _length_: length, | ||
| _buffer: PyRwLock::new(RawBuffer { | ||
| inner: Vec::with_capacity(length * itemsize).as_mut_ptr(), |
There was a problem hiding this comment.
I don't think this line is safe. The new Vec will be removed right after this line and the given pointer will be a dangling pointer.
Sorry, something went wrong.
| // @TODO: Is this copying? | ||
|
|
||
| let buffered = if copy { | ||
| unsafe { slice::from_raw_parts_mut(buffer.as_mut_ptr(), buffer.len()) } |
There was a problem hiding this comment.
this is not a copy
Sorry, something went wrong.
There was a problem hiding this comment.
Here
let buffered = if copy {
unsafe { slice::from_raw_parts_mut(buffer.as_mut_ptr(), buffer.len()) }
.as_mut_ptr()
} else {
buffer.as_mut_ptr()
};The idea is to copy the bytes from the buffer if copy, or return a view otherwise. Should I use to_contiguous to copy it?
Sorry, something went wrong.
| #[pyimpl] | ||
| pub trait PyCDataFunctions: PyValue { | ||
| #[pymethod] | ||
| fn size_of_instances(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<PyObjectRef>; |
There was a problem hiding this comment.
| fn size_of_instances(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<PyObjectRef>; | |
| fn size_of_instances(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult; |
PyResult == PyResult<PyObjectRef>
Sorry, something went wrong.
| fn obj_bytes(&self) -> BorrowedValue<[u8]> { | ||
| PyRwLockReadGuard::map(self.data.borrow_value(), |x| x.as_slice()).into() | ||
| PyRwLockReadGuard::map(self.data.borrow_value(), |x| unsafe { | ||
| slice::from_raw_parts(x.inner, x.size) |
There was a problem hiding this comment.
no, it isn't. slice is a sort of view
Sorry, something went wrong.
| PyCDataFunctions::alignment_of_instances(zelf.into_ref(vm), vm) | ||
| } | ||
| Either::B(obj) if obj.has_class_attr("alignment_of_instances") => { | ||
| let size_of = vm.get_attribute(obj, "alignment_of_instances").unwrap(); |
There was a problem hiding this comment.
| let size_of = vm.get_attribute(obj, "alignment_of_instances").unwrap(); | |
| let size_of = vm.get_attribute(obj, "alignment_of_instances")?; |
is this unwrap() intended?
Sorry, something went wrong.
| if vm.isinstance(&argtypes, &vm.ctx.types.list_type).is_ok() | ||
| || vm.isinstance(&argtypes, &vm.ctx.types.tuple_type).is_ok() |
There was a problem hiding this comment.
| if vm.isinstance(&argtypes, &vm.ctx.types.list_type).is_ok() | |
| || vm.isinstance(&argtypes, &vm.ctx.types.tuple_type).is_ok() | |
| if vm.isinstance(&argtypes, &vm.ctx.types.list_type).and_then(|_| vm.isinstance(&argtypes, &vm.ctx.types.tuple_type)).map_err(|e| vm.new_type_error(format!( | |
| "_argtypes_ must be a sequence of types, {} found.", | |
| argtypes.to_string() | |
| )))? { |
Sorry, something went wrong.
|
by watching rustpython_vm::builtins::pytype::new, I feel like that'd be simply done by replacing typ field in PyObject, but not sure it actually will be that easy. |
Sorry, something went wrong.
Let me get the CPython definitions as example MOD_ADD_TYPE(&Struct_Type, &PyCStructType_Type, &PyCData_Type);
MOD_ADD_TYPE(&Union_Type, &UnionType_Type, &PyCData_Type);
MOD_ADD_TYPE(&PyCPointer_Type, &PyCPointerType_Type, &PyCData_Type);
MOD_ADD_TYPE(&PyCArray_Type, &PyCArrayType_Type, &PyCData_Type);
MOD_ADD_TYPE(&Simple_Type, &PyCSimpleType_Type, &PyCData_Type);
MOD_ADD_TYPE(&PyCFuncPtr_Type, &PyCFuncPtrType_Type, &PyCData_Type);Here we have MOD_ADD_TYPE(&Class, &MetaClass, &BaseClass) right? Looking at Simple_Type (in RustPython this is PySimpleType in primitive.rs), we see that its metaclass implements the __mul__, so we can have c_int * 8 just fine, where c_int is subclass of Simple_Type. I used a trait PyCDataSequenceMethods in basics.rs to implement the __mul__, but doing impl PyCDataSequenceMethods for PySimpleTypeMeta {} in primitive.rs doesn't work because PySimpleTypeMeta is not the metaclass of PySimpleType. |
Sorry, something went wrong.
|
I'm not sure why, but I cannot comment on the following reply
The idea of RawBuffer is to be a "view" of the external buffer, so when needed, we copy the bytes from that buffer. I think there's one case we need to copy data and another where we need a view. |
Sorry, something went wrong.
|
What happens when the buffer is destroyed? Does it deletes only the pointer(as a reference) or also deletes the data it contains (as an owner)? If it does both, then does it need a flag to distinguish them? |
Sorry, something went wrong.
|
I forgot where these two functions are used, but based on the docs: |
Sorry, something went wrong.
|
@youknowone I basically patched all your comments, you wrote:
For this code fn obj_bytes(&self) -> BorrowedValue<[u8]> {
PyRwLockReadGuard::map(self.data.borrow_value(), |x| unsafe {
slice::from_raw_parts(x.inner, x.size)
})
.into()
}What did you meant there? |
Sorry, something went wrong.
|
There was a question asking if it does copy. The comment is an answer about it. Creating an slice object doesn't copy anything. |
Sorry, something went wrong.
|
I added a C file that should be compiled as a shared lib and bundled together with the interpreter. Its _ctypes_test.c. How would one make this happen? Would it be some sort of build script using cc? And we should be able to do this: import _ctypes_test |
Sorry, something went wrong.
|
build.rs file can contain any build script. Using cc crate will be helpful. |
Sorry, something went wrong.
Adding more meta impls.
|
@youknowone I haven't figured out a nice way to implement the from_buffer. from_buffer_copy is straight forward, you simply create a new instance (default args for both tp_new and init) and copy the bytes from the value to create an instance. For example: data = b'data'
ubyte = c_ubyte * len(data)
byteslike = ubyte.from_buffer_copy(data) # an instance of ubyte made from data by copying thingsfrom_buffer does a similar thing, but it doesn't copy things, it points to the source. So if you change byteslike, you change data. My implementation of from_buffer_copy is something in these lines #[pyclassmethod]
fn from_buffer_copy(
cls: PyRef<Self>,
obj: PyObjectRef,
offset: OptionalArg,
vm: &VirtualMachine,
) -> PyResult {
let buffer = try_buffer_from_object(vm, &obj)?;
let opts = buffer.get_options().clone();
let (size, offset) = Self::buffer_check(cls, opts, offset, vm); //ignore this for now
let src_buffer = buffer.obj_bytes();
let empty_instance = ... //creates a new empty instance
let dst_buffer = empty_instance.obj_bytes_mut();
dst_buffer.copy_from_slice(&src_buffer[offset..offset+size]);
Ok(empty_instance.as_object().clone())
}So the idea is simply get the bytes from obj (it should implement the buffer protocol) and point to them. How would I point to the data in obj instead of copying it? Should I use some refcounted type to avoid pointing to dangling things? obj should always be a buffer with write access. |
Sorry, something went wrong.
|
I don't know well about your requirements, but I guess you can do same way as how PyMemoryView does. Or even just use it. it just holds a PyBufferRef and copy the data only when it needs to be. It also locks PyBufferRef when the lock is required. |
Sorry, something went wrong.
|
I think this PR is (practically) not possible to rebase anymore in this state. Do you mind if we squash the commits into single commit for rebase? |
Sorry, something went wrong.
Not at all :) I'm afraid I can't contribute in the following months. I can help others eventually, tho. |
Sorry, something went wrong.
|
Is this effort still ongoing / has this ctypes implementation been abandoned for another? |
Sorry, something went wrong.
Co-authored-by: Jeong YunWon <jeong@youknowone.org> Co-authored-by: Rodrigo Oliveira <rodrigo.redcode@gmail.com> Co-authored-by: Darley Barreto <darleybarreto@gmail.com> Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
Co-authored-by: Jeong YunWon <jeong@youknowone.org> Co-authored-by: Rodrigo Oliveira <rodrigo.redcode@gmail.com> Co-authored-by: Darley Barreto <darleybarreto@gmail.com> Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
|
Hi folks, I think we should close this, it's too old for any merging attempt. |
Sorry, something went wrong.
|
This is still a best trial of ctypes now. I wouldn't close it unless we actually merge any ctypes implementation. For anyone who will try another ctypes implementation, I hope it can be gradually mergable, not to be suffered by huge size of rebase. |
Sorry, something went wrong.
|
Agreed, the rebase effort was slowed by refactors that have happened since this pr, I'm planning to start incremental work on this soon. |
Sorry, something went wrong.
|
Note that #5572 is now the tracking issue for further ctypes implementation. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
@darleybarreto and I are implementing the ctypes module. This an initial implementation for review. At this stage, we are focusing on Linux platforms to in the future extend to other platforms like ctypes from cpython does.