remove type_is_pymodule
#3594
remove type_is_pymodule
#3594
Conversation
| @@ -3,10 +3,10 @@ use pyo3::prelude::*; | |||
| #[pymodule] | |||
| fn module(_py: Python<'_>, m: &PyModule) -> PyResult<()> { | |||
| #[pyfn(m, pass_module)] | |||
| fn fail(string: &str, module: &PyModule) -> PyResult<&str> { | |||
| fn fail<'py>(string: &str, module: &'py PyModule) -> PyResult<&'py str> { | |||
There was a problem hiding this comment.
The original check was masking an ambiguous lifetime error; I needed to add lifetime hints to avoid having this error also included in the trybuild output.
|
cc @wyfo |
|
Ah I'll push a test for the empty arguments branch in the morning. |
Good point, but now it's my turn to tell you that I think you should do the same for |
|
Thanks to the structure of your modification, I've realized that I forgot to update the line 380 in method.rs. Could you do that in this PR? |
davidhewitt
force-pushed
the
remove_type_is_pymodule
branch
from
c5fca3c
to
5ac56b8
Compare
There was a problem hiding this comment.
Moved this test into invalid_pyfunctions.rs
| #[allow(clippy::useless_conversion)] | ||
| ::std::convert::Into::into(_pyo3::types::PyType::from_type_ptr(py, _slf as *mut _pyo3::ffi::PyTypeObject)), | ||
| ::std::convert::Into::into(_pyo3::types::PyType::from_type_ptr(#py, #slf.cast())), | ||
| } | ||
| } | ||
| FnType::FnModule(span) => { |
There was a problem hiding this comment.
Should _slf also be declared as a syn::Indent like above?
There was a problem hiding this comment.
Initially I did that but got a hygiene error and backed out. Probably that needs investigating but if the test suite compiles I'm not super concerned.
|
I've no Approve button, so I will just write 👍 |
|
👍 thanks for the review!
If you're reviewing regularly we should look at getting that set up 👀 |
Follow-up to #3587
This removes the
type_is_pymodulecheck. I had to propagate the span of the first argument through the macro code to get the compiler error to hint at the right place, overall I think this is a nice cleanup.