Skip to content
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

Hygiene: offer a way to set path to pyo3 crate #2022

Merged
merged 6 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Add `Py::setattr` method. [#2009](https://github.com/PyO3/pyo3/pull/2009)
- Add `PyCapsule`, exposing the [Capsule API](https://docs.python.org/3/c-api/capsule.html#capsules). [#1980](https://github.com/PyO3/pyo3/pull/1980)
- All PyO3 proc-macros except the deprecated `#[pyproto]` now accept a supplemental attribute `#[pyo3(crate = "some::path")]` that specifies
where to find the `pyo3` crate, in case it has been renamed or is re-exported and not found at the crate root. [#2022](https://github.com/PyO3/pyo3/pull/2022)

### Changed

Expand Down
20 changes: 20 additions & 0 deletions guide/src/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,23 @@ a: <builtins.Inner object at 0x0000020044FCC670>
b: <builtins.Inner object at 0x0000020044FCC670>
```
The downside to this approach is that any Rust code working on the `Outer` struct now has to acquire the GIL to do anything with its field.

## I want to use the `pyo3` crate re-exported from from dependency but the proc-macros fail!

All PyO3 proc-macros (`#[pyclass]`, `#[pyfunction]`, `#[derive(FromPyObject)]`
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice to document this alongside each macro too. I really like serde's reference on attributes e.g. https://serde.rs/container-attrs.html

I've tried to do this a bit for #[pyfunction] in guide/function.md, but it's sorely lacking. There's also some docs in pyo3-macros/lib.rs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I'll tackle this (also for the other attributes) in a follow-up? This attribute is certainly the least-needed one :)

Copy link
Member

Choose a reason for hiding this comment

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

Yep I think that's fine!

and so on) expect the `pyo3` crate to be available under that name in your crate
root, which is the normal situation when `pyo3` is a direct dependency of your
crate.

However, when the dependency is renamed, or your crate only indirectly depends
on `pyo3`, you need to let the macro code know where to find the crate. This is
done with the `crate` attribute:

```rust
# use pyo3::prelude::*;
# pub extern crate pyo3;
# mod reexported { pub use ::pyo3; }
#[pyclass]
#[pyo3(crate = "reexported::pyo3")]
struct MyClass;
```
15 changes: 14 additions & 1 deletion pyo3-macros-backend/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use syn::{
parse::{Parse, ParseStream},
punctuated::Punctuated,
token::Comma,
Attribute, ExprPath, Ident, LitStr, Result, Token,
Attribute, ExprPath, Ident, LitStr, Path, Result, Token,
};

pub mod kw {
Expand Down Expand Up @@ -43,6 +43,19 @@ impl Parse for NameAttribute {
}
}

/// For specifying the path to the pyo3 crate.
#[derive(Clone, Debug, PartialEq)]
pub struct CrateAttribute(pub Path);

impl Parse for CrateAttribute {
fn parse(input: ParseStream) -> Result<Self> {
let _: Token![crate] = input.parse()?;
let _: Token![=] = input.parse()?;
let string_literal: LitStr = input.parse()?;
string_literal.parse().map(CrateAttribute)
}
}

#[derive(Clone, Debug, PartialEq)]
pub struct TextSignatureAttribute {
pub kw: kw::text_signature,
Expand Down
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl ToTokens for Deprecations {
let ident = deprecation.ident(*span);
quote_spanned!(
*span =>
let _ = ::pyo3::impl_::deprecations::#ident;
let _ = _pyo3::impl_::deprecations::#ident;
)
.to_tokens(tokens)
}
Expand Down
80 changes: 52 additions & 28 deletions pyo3-macros-backend/src/from_pyobject.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::attributes::{self, get_pyo3_options, FromPyWithAttribute};
use crate::{
attributes::{self, get_pyo3_options, CrateAttribute, FromPyWithAttribute},
utils::get_pyo3_crate,
};
use proc_macro2::TokenStream;
use quote::quote;
use syn::{
Expand Down Expand Up @@ -55,14 +58,14 @@ impl<'a> Enum<'a> {
for (i, var) in self.variants.iter().enumerate() {
let struct_derive = var.build();
let ext = quote!(
let maybe_ret = || -> ::pyo3::PyResult<Self> {
let maybe_ret = || -> _pyo3::PyResult<Self> {
#struct_derive
}();

match maybe_ret {
ok @ ::std::result::Result::Ok(_) => return ok,
::std::result::Result::Err(err) => {
let py = ::pyo3::PyNativeType::py(obj);
let py = _pyo3::PyNativeType::py(obj);
err_reasons.push_str(&::std::format!("{}\n", err.value(py).str()?));
}
}
Expand All @@ -82,7 +85,7 @@ impl<'a> Enum<'a> {
#ty_name,
#error_names,
&err_reasons);
::std::result::Result::Err(::pyo3::exceptions::PyTypeError::new_err(err_msg))
::std::result::Result::Err(_pyo3::exceptions::PyTypeError::new_err(err_msg))
)
}
}
Expand Down Expand Up @@ -207,8 +210,8 @@ impl<'a> Container<'a> {
);
quote!(
::std::result::Result::Ok(#self_ty{#ident: obj.extract().map_err(|inner| {
let py = ::pyo3::PyNativeType::py(obj);
let new_err = ::pyo3::exceptions::PyTypeError::new_err(#error_msg);
let py = _pyo3::PyNativeType::py(obj);
let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg);
new_err.set_cause(py, ::std::option::Option::Some(inner));
new_err
})?})
Expand All @@ -222,11 +225,11 @@ impl<'a> Container<'a> {
};
quote!(
::std::result::Result::Ok(#self_ty(obj.extract().map_err(|err| {
let py = ::pyo3::PyNativeType::py(obj);
let py = _pyo3::PyNativeType::py(obj);
let err_msg = ::std::format!("{}: {}",
#error_msg,
err.value(py).str().unwrap());
::pyo3::exceptions::PyTypeError::new_err(err_msg)
_pyo3::exceptions::PyTypeError::new_err(err_msg)
})?))
)
}
Expand All @@ -238,9 +241,9 @@ impl<'a> Container<'a> {
for i in 0..len {
let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), i);
fields.push(quote!(
s.get_item(#i).and_then(::pyo3::types::PyAny::extract).map_err(|inner| {
let py = ::pyo3::PyNativeType::py(obj);
let new_err = ::pyo3::exceptions::PyTypeError::new_err(#error_msg);
s.get_item(#i).and_then(_pyo3::types::PyAny::extract).map_err(|inner| {
let py = _pyo3::PyNativeType::py(obj);
let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg);
new_err.set_cause(py, ::std::option::Option::Some(inner));
new_err
})?));
Expand All @@ -255,9 +258,9 @@ impl<'a> Container<'a> {
quote!("")
};
quote!(
let s = <::pyo3::types::PyTuple as ::pyo3::conversion::PyTryFrom>::try_from(obj)?;
let s = <_pyo3::types::PyTuple as _pyo3::conversion::PyTryFrom>::try_from(obj)?;
if s.len() != #len {
return ::std::result::Result::Err(::pyo3::exceptions::PyValueError::new_err(#msg))
return ::std::result::Result::Err(_pyo3::exceptions::PyValueError::new_err(#msg))
}
::std::result::Result::Ok(#self_ty(#fields))
)
Expand All @@ -279,15 +282,15 @@ impl<'a> Container<'a> {
let extractor = match &attrs.from_py_with {
None => quote!(
#get_field.extract().map_err(|inner| {
let py = ::pyo3::PyNativeType::py(obj);
let new_err = ::pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg);
let py = _pyo3::PyNativeType::py(obj);
let new_err = _pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg);
new_err.set_cause(py, ::std::option::Option::Some(inner));
new_err
})?),
Some(FromPyWithAttribute(expr_path)) => quote! (
#expr_path(#get_field).map_err(|inner| {
let py = ::pyo3::PyNativeType::py(obj);
let new_err = ::pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg);
let py = _pyo3::PyNativeType::py(obj);
let new_err = _pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg);
new_err.set_cause(py, ::std::option::Option::Some(inner));
new_err
})?
Expand All @@ -300,20 +303,25 @@ impl<'a> Container<'a> {
}
}

#[derive(Default)]
struct ContainerOptions {
/// Treat the Container as a Wrapper, directly extract its fields from the input object.
transparent: bool,
/// Change the name of an enum variant in the generated error message.
annotation: Option<syn::LitStr>,
/// Change the path for the pyo3 crate
krate: Option<CrateAttribute>,
}

/// Attributes for deriving FromPyObject scoped on containers.
#[derive(Clone, Debug, PartialEq)]
#[derive(Debug)]
enum ContainerPyO3Attribute {
/// Treat the Container as a Wrapper, directly extract its fields from the input object.
Transparent(attributes::kw::transparent),
/// Change the name of an enum variant in the generated error message.
ErrorAnnotation(LitStr),
/// Change the path for the pyo3 crate
Crate(CrateAttribute),
}

impl Parse for ContainerPyO3Attribute {
Expand All @@ -326,6 +334,8 @@ impl Parse for ContainerPyO3Attribute {
let _: attributes::kw::annotation = input.parse()?;
let _: Token![=] = input.parse()?;
input.parse().map(ContainerPyO3Attribute::ErrorAnnotation)
} else if lookahead.peek(Token![crate]) {
input.parse().map(ContainerPyO3Attribute::Crate)
} else {
Err(lookahead.error())
}
Expand All @@ -334,10 +344,8 @@ impl Parse for ContainerPyO3Attribute {

impl ContainerOptions {
fn from_attrs(attrs: &[Attribute]) -> Result<Self> {
let mut options = ContainerOptions {
transparent: false,
annotation: None,
};
let mut options = ContainerOptions::default();

for attr in attrs {
if let Some(pyo3_attrs) = get_pyo3_options(attr)? {
for pyo3_attr in pyo3_attrs {
Expand All @@ -356,6 +364,13 @@ impl ContainerOptions {
);
options.annotation = Some(lit_str);
}
ContainerPyO3Attribute::Crate(path) => {
ensure_spanned!(
options.krate.is_none(),
path.0.span() => "`crate` may only be provided once"
);
options.krate = Some(path);
}
}
}
}
Expand Down Expand Up @@ -499,13 +514,18 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
.predicates
.push(parse_quote!(#gen_ident: FromPyObject<#lt_param>))
}
let options = ContainerOptions::from_attrs(&tokens.attrs)?;
let krate = get_pyo3_crate(&options.krate);
let derives = match &tokens.data {
syn::Data::Enum(en) => {
if options.transparent || options.annotation.is_some() {
bail_spanned!(tokens.span() => "`transparent` or `annotation` is not supported \
at top level for enums");
}
let en = Enum::new(en, &tokens.ident)?;
en.build()
}
syn::Data::Struct(st) => {
let options = ContainerOptions::from_attrs(&tokens.attrs)?;
if let Some(lit_str) = &options.annotation {
bail_spanned!(lit_str.span() => "`annotation` is unsupported for structs");
}
Expand All @@ -520,11 +540,15 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {

let ident = &tokens.ident;
Ok(quote!(
#[automatically_derived]
impl#trait_generics ::pyo3::FromPyObject<#lt_param> for #ident#generics #where_clause {
fn extract(obj: &#lt_param ::pyo3::PyAny) -> ::pyo3::PyResult<Self> {
#derives
const _: () = {
use #krate as _pyo3;

#[automatically_derived]
impl#trait_generics _pyo3::FromPyObject<#lt_param> for #ident#generics #where_clause {
fn extract(obj: &#lt_param _pyo3::PyAny) -> _pyo3::PyResult<Self> {
#derives
}
}
}
};
))
}
Loading