wrap_x: change macros back to macro_rules!
#2363
Conversation
src/macros.rs
Outdated
| #[macro_export] | ||
| macro_rules! wrap_pyfunction { | ||
| ($function:path) => { | ||
| &|arg| { | ||
| use $function as def; | ||
| $crate::impl_::pyfunction::wrap_pyfunction(def::DEF, arg) | ||
| } | ||
| }; | ||
| ($function:path, $arg:expr) => {{ | ||
| use $function as def; | ||
| $crate::impl_::pyfunction::wrap_pyfunction(def::DEF, $arg) | ||
| }}; | ||
| } |
There was a problem hiding this comment.
IMO it is kinda weird that there are two ways to invoke the macro. (I know it did this before, this is somewhat off-topic). AFAIK the first branch is only for use with add_wrapped, right?
I've seen several people get confused by this macro, so if there's a chance to simplify it I'd like to take it. But at least that should be documented.
There was a problem hiding this comment.
I agree with this, but I'd like to avoid making changes to meet these points in this PR... let me try to push an experiment.
There was a problem hiding this comment.
See #2367 as the way I hope we eventually can get rid of the wrap macroa for good.
src/macros.rs
Outdated
| ($function:path, $arg:expr) => {{ | ||
| use $function as def; | ||
| $crate::impl_::pyfunction::wrap_pyfunction(def::DEF, $arg) |
There was a problem hiding this comment.
| ($function:path, $arg:expr) => {{ | |
| use $function as def; | |
| $crate::impl_::pyfunction::wrap_pyfunction(def::DEF, $arg) | |
| ($function:path, $module:expr) => {{ | |
| use $function as def; | |
| $crate::impl_::pyfunction::wrap_pyfunction(def::DEF, $module) |
There was a problem hiding this comment.
Will go for module_or_py (because those are the things it can be, IIRC).
davidhewitt
force-pushed
the
macro_rules_wrap
branch
2 times, most recently
from
2199315
to
cca0e0b
Compare
davidhewitt
force-pushed
the
macro_rules_wrap
branch
2 times, most recently
from
3463942
to
321cbff
Compare
|
Hmm this runs into trouble with glob imports because the macro rules are slightly wonky with paths... see rust-lang/rust#48067 |
|
Actually a slight bit of breakage related to glob imports of submodules with the same name as the containing Rust module seems inevitable but perhaps better behaviour overall - see #2366 (comment) I think this case should be pretty rare so think it's fine to proceed. |
davidhewitt
force-pushed
the
macro_rules_wrap
branch
from
321cbff
to
39ab95a
Compare
davidhewitt
force-pushed
the
macro_rules_wrap
branch
from
39ab95a
to
7a9e70e
Compare
Marginal cleanup to convert the
wrap_pyfunctionandwrap_pymodulemacros back tomacro_rules!macros.I think it's marginally easier for both readers and maintainers to follow
macro_rules!macros in this simple case. Also allows for better doc links.