Skip to content

pymodule derive macro #1832

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
Apr 27, 2020
Merged

pymodule derive macro #1832

merged 11 commits into from
Apr 27, 2020

Conversation

youknowone
Copy link
Member

@youknowone youknowone commented Mar 30, 2020

Creating an inner module for every stdlib looks not ideal but i have no idea how to do without them

@youknowone youknowone force-pushed the pymodule branch 4 times, most recently from cb570c8 to 186a3e1 Compare March 31, 2020 02:25
Copy link
Member

@coolreader18 coolreader18 left a comment

Choose a reason for hiding this comment

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

I really like this idea and I've been thinking about something similar for a while, though I agree that inline modules aren't optimal. It'd be great if we could put it on as an inner #![pymodule] attribute, but it looks like that's still a nightly feature; tracking issue rust-lang/rust#54726.

@coolreader18
Copy link
Member

Maybe we could only do this for some modules until custom_inner_attributes is stabilized? I think rightward drift might be an issue, as would git blame results. It'd be great if we could make rustfmt format it like below, ignoring indentation for just that block, though I don't think that's possible:

#[pymodule]
mod decl {

#[pyfunction]
fn thing() {}

}

@youknowone
Copy link
Member Author

For the formatting, I couldn't find any way to change local indent hint if we don't want to disable the whole file formatting. Also derive macro seems still always require to match the brace pair.

Is it possible to add #[pymodule] to use part of mod.rs for same result? I don't know well but it doesn't look like - because item iteration looks only possible on actual module code.

@youknowone
Copy link
Member Author

The mentioned issue looks quite old and then it means it is not expected to be solved in near future. Just go by experiencing git blame barrier?

@coolreader18
Copy link
Member

Yeah, I guess so; I haven't gotten a ton of use out of git blame for this project, and I think the stdlib is generally simple/encapsulated enough that it wouldn't be used that much there.

@youknowone youknowone merged commit eaa839b into RustPython:master Apr 27, 2020
@youknowone youknowone deleted the pymodule branch April 27, 2020 01:42
@palaviv
Copy link
Contributor

palaviv commented May 9, 2020

@youknowone I think this change make developing harder. When I have errors the compiler does not show the line numbers and add suggestion:

error[E0308]: mismatched types
  |
  = note: expected reference `&std::sync::Arc<pyobject::PyObject<(dyn pyobject::PyObjectPayload + 'static)>>`
                found struct `std::sync::Arc<pyobject::PyObject<dyn pyobject::PyObjectPayload>>`

Is there a way to fix this?

@youknowone
Copy link
Member Author

I am sorry that discovering this comment so late. should we consider to revert this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants