Conversation
|
|
|
related to #3742 |
ChJR
force-pushed
the
feature/fix_decorators
branch
from
a77ae32 to
f2e62cf
Compare
vm/src/builtins/classmethod.rs
Outdated
|
|
||
| fn py_new(cls: PyTypeRef, callable: Self::Args, vm: &VirtualMachine) -> PyResult { | ||
| PyClassMethod { | ||
| let _callable = callable.clone(); |
There was a problem hiding this comment.
usually _ prefix means unused variable.
vm/src/builtins/classmethod.rs
Outdated
| match obj.set_attr("__doc__", doc, vm) { | ||
| Err(e) => Err(e), | ||
| Ok(_) => result, | ||
| } |
There was a problem hiding this comment.
| match obj.set_attr("__doc__", doc, vm) { | |
| Err(e) => Err(e), | |
| Ok(_) => result, | |
| } | |
| obj.set_attr("__doc__", doc, vm)?; | |
| result |
vm/src/builtins/staticmethod.rs
Outdated
|
|
||
| fn py_new(cls: PyTypeRef, callable: Self::Args, vm: &VirtualMachine) -> PyResult { | ||
| PyStaticMethod { callable } | ||
| let _callable = callable.clone(); |
|
Good points. I will change the codes. |
vm/src/builtins/classmethod.rs
Outdated
| let _descr_get: PyResult<PyObjectRef> = zelf.callable.lock().get_attr("__get__", vm); | ||
| match _descr_get { | ||
| Err(_) => Ok(PyBoundMethod::new_ref(cls, zelf.callable.lock().clone(), &vm.ctx).into()), | ||
| Ok(_descr_get) => vm.invoke(&_descr_get, (cls.clone(), cls)), | ||
| } |
There was a problem hiding this comment.
_descr_get is unnecessarily _ prefixed
vm/src/builtins/classmethod.rs
Outdated
| let doc = vm.unwrap_pyresult(doc); | ||
| let obj = vm.unwrap_pyresult(result.clone()); |
There was a problem hiding this comment.
I don't think this is a correct error handling.
Both of them can be Err and they will panic.
vm/src/builtins/classmethod.rs
Outdated
| fn py_new(cls: PyTypeRef, callable: Self::Args, vm: &VirtualMachine) -> PyResult { | ||
| PyClassMethod { | ||
| callable: PyMutex::new(callable), | ||
| let result: PyResult<PyObjectRef> = PyClassMethod { |
There was a problem hiding this comment.
because PyResult = PyResult<PyObjectRef>, this is redundant.
vm/src/builtins/classmethod.rs
Outdated
| PyClassMethod { | ||
| callable: PyMutex::new(callable), | ||
| let result: PyResult<PyObjectRef> = PyClassMethod { | ||
| callable: PyMutex::new(callable.clone()), |
There was a problem hiding this comment.
try to get __doc__ before making PyClassMethod to avoid clone.
vm/src/builtins/classmethod.rs
Outdated
| let result: PyResult<PyObjectRef> = PyClassMethod { | ||
| callable: PyMutex::new(callable.clone()), | ||
| } | ||
| .into_ref_with_type(vm, cls) |
There was a problem hiding this comment.
i'd like to propagate error here before later handling
| .into_ref_with_type(vm, cls) | |
| .into_ref_with_type(vm, cls)? |
|
dependency of #3862 |
vm/src/builtins/classmethod.rs
Outdated
| match doc { | ||
| Err(_) => None, | ||
| Ok(doc) => Some(obj.set_attr("__doc__", doc, vm)), | ||
| }; |
There was a problem hiding this comment.
| match doc { | |
| Err(_) => None, | |
| Ok(doc) => Some(obj.set_attr("__doc__", doc, vm)), | |
| }; | |
| if let Ok(doc) = doc { | |
| obj.set_attr("__doc__", doc, vm)?; | |
| } |
set_attr result need to be checked with ?
Check 'Named Expressions Need Not Be Parenthesized' Section
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
looks good, thank you for contributing!
|
|
3 participants
Except test_expressions in test_decorators.py file.
Now it is working with full test_decorators.py test file.