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

Add support for wrapping attributes in #[cfg_attr(feature = "pyo3", …)] #2786

Closed
wants to merge 10 commits into from
6 changes: 5 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ widestring = "0.5.1"
pyo3-build-config = { path = "pyo3-build-config", version = "0.18.1", features = ["resolve-config"] }

[features]
default = ["macros"]
default = ["macros", "pyo3"]

# Enables pyo3::inspect module and additional type information on FromPyObject
# and IntoPy traits
Expand Down Expand Up @@ -110,8 +110,12 @@ full = [
"eyre",
"anyhow",
"experimental-inspect",
"pyo3",
]

# This makes `#[cfg_attr(feature = "pyo3", pyclass)]` in our own tests work, it has no other function
pyo3 = []
Comment on lines +116 to +117
Copy link
Member

Choose a reason for hiding this comment

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

I wonder... should we move this to the pytests crate or something similar, so that we don't need to expose a no-op feature to the users?

To avoid other code changes, you might also just be able to hack it by adding RUSTFLAGS=--cfg "feature=pyo3", which is all cargo really does under the hood I think?

Copy link
Member Author

Choose a reason for hiding this comment

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

so like moving the ui test to pytests/src? The RUSTFLAGS would also do, but imho it'd be nice if cargo test would continue to work.

I've also been thinking about using a different feature name for the tests, but i couldn't come up with a good design for that either, especially for cases such as the guide doc test


[[bench]]
name = "bench_call"
harness = false
Expand Down
1 change: 1 addition & 0 deletions guide/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- [Basic object customization](class/object.md)
- [Emulating numeric types](class/numeric.md)
- [Emulating callable objects](class/call.md)
- [Optional bindings](class/optional_bindings.md)
- [Type conversions](conversions.md)
- [Mapping of Rust types to Python types](conversions/tables.md)]
- [Conversion traits](conversions/traits.md)]
Expand Down
66 changes: 66 additions & 0 deletions guide/src/class/optional_bindings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Optional bindings

You might want to write a library the is usable both in pure Rust and as a Python library. For that, PyO3 supports wrapping attributes in `#[cfg_attr(feature = "pyo3", ...)]` (the feature name unfortunately has to be hardcoded, so the feature must be named `pyo3`). This does not only apply to classes and their methods but also to e.g. `#[pyfunction]`.

Make pyo3 optional in Cargo.toml:

```toml
[lib]
# cdylib for the python module, rlib for the rust crate
crate-type = ["cdylib", "rlib"]

[dependencies]
pyo3 = { version = "0.14", features = ["extension-module", "abi3"], optional = true }
```
konstin marked this conversation as resolved.
Show resolved Hide resolved

If you're using maturin, also set `pyo3` as a default feature in pyproject.toml, so `maturin build` will work as well as `cargo build`:

```toml
[tool.maturin]
features = ["pyo3"]
```

Implementing a `Number` again, but this time making all attributes and the module function optional:

```rust
use pyo3::prelude::*;

#[cfg_attr(feature = "pyo3", pyclass)]
struct Number(i32);

#[cfg_attr(feature = "pyo3", pymethods)]
impl Number {
#[cfg_attr(feature = "pyo3", classattr)]
const SMALLEST_PRIME: i32 = 2;

#[cfg_attr(feature = "pyo3", new)]
fn new(value: i32) -> Self {
Self(value)
}

/// Computes the [Greatest common divisor](https://en.wikipedia.org/wiki/Greatest_common_divisor) of two numbers
#[cfg_attr(feature = "pyo3", pyo3(name = "gcd"))]
fn greatest_common_divisor(&self, other: &Self) -> Self {
let mut a = self.0;
let mut b = other.0;
while a != b {
if a > b {
a -= b
} else {
b -= a
}
}

Self::new(a)
}
}

#[cfg(feature = "pyo3")] // We don't want that function at all in a rust library
#[pymodule]
fn my_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_class::<Number>()?;
Ok(())
}
```

Now you have a library that you can use both normally in rust without any python dependency and as a python library.
1 change: 1 addition & 0 deletions newsfragments/2786.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for wrapping attributes in `#[cfg_attr(feature = "pyo3", ...)]`, so you can add pyo3 as a purely optional feature to otherwise normal rust libraries. See the "Optional bindings" chapter in the guide for more info and usage examples.
166 changes: 146 additions & 20 deletions pyo3-macros-backend/src/attributes.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use proc_macro2::TokenStream;
use quote::ToTokens;
use std::iter::FromIterator;
use syn::parse::Parser;
use syn::punctuated::{IntoPairs, Pair};
use syn::{
parse::{Parse, ParseStream},
punctuated::Punctuated,
spanned::Spanned,
token::Comma,
Attribute, Expr, ExprPath, Ident, LitStr, Path, Result, Token,
Attribute, Expr, ExprPath, Ident, Lit, LitStr, Meta, MetaList, NestedMeta, Path, Result, Token,
};

pub mod kw {
Expand Down Expand Up @@ -144,22 +147,16 @@ pub type FromPyWithAttribute = KeywordAttribute<kw::from_py_with, LitStrValue<Ex
/// For specifying the path to the pyo3 crate.
pub type CrateAttribute = KeywordAttribute<Token![crate], LitStrValue<Path>>;

pub fn get_pyo3_options<T: Parse>(attr: &syn::Attribute) -> Result<Option<Punctuated<T, Comma>>> {
if is_attribute_ident(attr, "pyo3") {
/// We can either have `#[pyo3(...)]` or `#[cfg_attr(feature = "pyo3", pyo3(...))]`,
/// with a comma separated list of options parsed into `T` inside
pub fn get_pyo3_options<T: Parse>(attr: &Attribute) -> Result<Option<Punctuated<T, Comma>>> {
if attr.path.is_ident("pyo3") {
attr.parse_args_with(Punctuated::parse_terminated).map(Some)
} else {
Ok(None)
}
}

pub fn is_attribute_ident(attr: &syn::Attribute, name: &str) -> bool {
if let Some(path_segment) = attr.path.segments.last() {
attr.path.segments.len() == 1 && path_segment.ident == name
} else {
false
}
}

/// Takes attributes from an attribute vector.
///
/// For each attribute in `attrs`, `extractor` is called. If `extractor` returns `Ok(true)`, then
Expand All @@ -169,28 +166,157 @@ pub fn is_attribute_ident(attr: &syn::Attribute, name: &str) -> bool {
/// (In `retain`, returning `true` keeps the element, here it removes it.)
pub fn take_attributes(
attrs: &mut Vec<Attribute>,
mut extractor: impl FnMut(&Attribute) -> Result<bool>,
mut extractor: impl FnMut(&mut Attribute) -> Result<bool>,
) -> Result<()> {
*attrs = attrs
.drain(..)
.filter_map(|attr| {
extractor(&attr)
.filter_map(|mut attr| {
extractor(&mut attr)
.map(move |attribute_handled| if attribute_handled { None } else { Some(attr) })
.transpose()
})
.collect::<Result<_>>()?;
Ok(())
}

pub fn take_pyo3_options<T: Parse>(attrs: &mut Vec<syn::Attribute>) -> Result<Vec<T>> {
pub fn take_pyo3_options<T: Parse>(attrs: &mut Vec<Attribute>) -> Result<Vec<T>> {
let mut out = Vec::new();
take_attributes(attrs, |attr| {
if let Some(options) = get_pyo3_options(attr)? {
let mut new_attrs = Vec::new();

for mut attr in attrs.drain(..) {
Copy link
Member

Choose a reason for hiding this comment

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

You can also do

for mut attr in std::mem::take(attrs)

and then push to attrs, saving line 149 and 177.

Copy link
Member Author

Choose a reason for hiding this comment

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

I find it easier to read when we have a distinct old-attr-drain and new attr building; iterating and editing/deleting at the same time is unfortunately difficult to write straightforward

let parse_attr = |meta, _attributes: &Attribute| {
if let Meta::List(meta_list) = meta {
if meta_list.path.is_ident("pyo3") {
let parsed = Punctuated::<_, Token![,]>::parse_terminated
.parse2(meta_list.nested.to_token_stream())?;
out.extend(parsed.into_iter());
return Ok(true);
}
}
Ok(false)
};
if attr.path.is_ident("cfg_attr") {
if let Ok(mut meta) = attr.parse_meta() {
if handle_cfg_feature_pyo3(&mut attr, &mut meta, parse_attr)? {
continue;
}
}
}
if let Some(options) = get_pyo3_options(&attr)? {
out.extend(options.into_iter());
Ok(true)
continue;
} else {
Ok(false)
new_attrs.push(attr)
}
})?;
}

*attrs = new_attrs;
Ok(out)
}

/// Look for #[cfg_attr(feature = "pyo3", ...)]
/// ^^^^^^^^ ^^^^^^^ ^ ^^^^^^
fn is_cfg_feature_pyo3(
list: &MetaList,
keep: &mut Vec<Pair<NestedMeta, Comma>>,
iter: &mut IntoPairs<NestedMeta, Comma>,
) -> bool {
// #[cfg_attr(feature = "pyo3", ...)]
// ^^^^^^^^
if list.path.is_ident("cfg_attr") {
// #[cfg_attr(feature = "pyo3", ...)]
// ------- ^ ------
if let Some(pair) = iter.next() {
let pair_tuple = pair.into_tuple();
if let (NestedMeta::Meta(Meta::NameValue(name_value)), _) = &pair_tuple {
// #[cfg_attr(feature = "pyo3", ...)]
// ^^^^^^^
if name_value.path.is_ident("feature") {
if let Lit::Str(lit_str) = &name_value.lit {
// #[cfg_attr(feature = "pyo3", ...)]
// ^^^^^^
if lit_str.value() == "pyo3" {
// We want to keep the none-pyo3 pairs intact
keep.push(Pair::new(pair_tuple.0, pair_tuple.1));
return true;
}
}
}
}
keep.push(Pair::new(pair_tuple.0, pair_tuple.1));
}
}
false
}

/// Handle #[cfg_attr(feature = "pyo3", ...)]
///
/// Returns whether the attribute was completely handled and can be discarded (because there were
/// blocks in cfg_attr tail that weren't handled)
///
/// Attributes are icky: by default, we get an `Attribute` where all the real data is hidden in a
/// `TokenStream` member. Most of the attribute parsing are therefore custom `Parse` impls. We can
/// also ask syn to parse the attribute into `Meta`, which is essentially an attribute AST, which
/// also some code uses.
///
/// With `cfg_attr` we can additionally have multiple attributes rolled into one behind a gate. So
/// we have to parse and look for `cfg_attr(feature = "pyo3",`, then segment the parts behind it.
/// For each one we have to check whether it parses and also keep those where it doesn't parse for
/// subsequent proc macros (or rustc) to parse. The least bad option for this seems to parsing into
/// `Meta`, checking for `cfg_attr(feature = "pyo3",`, then splitting and letting the caller process
/// each attribute, including calling `.to_token_stream()` and then using `Parse` if necessary
/// (as e.g. [take_pyo3_options] does).
pub fn handle_cfg_feature_pyo3(
mut attr: &mut Attribute,
meta: &mut Meta,
// Return true if handled
mut parse_attr: impl FnMut(Meta, &Attribute) -> Result<bool>,
) -> Result<bool> {
if let Meta::List(list) = meta {
// These are the parts of the attr `parse_attr` told us we didn't parse and we should
// keep for subsequent proc macros
let mut keep = Vec::new();
// handrolled drain function, because `Punctuated` doesn't have one.
// We keep the comma around so what we do is lossless (keeping the spans)
let mut drain = list.nested.clone().into_pairs();
// Look for #[cfg_attr(feature = "pyo3", ...)]
if !is_cfg_feature_pyo3(list, &mut keep, &mut drain) {
// No match? Put the meta back we just swapped out, we don't actually want to drain
list.nested = Punctuated::from_iter(keep.into_iter().chain(drain));
return Ok(false);
}

// #[cfg_attr(feature = "pyo3", staticmethod, pair(name = "ferris"))]
// ^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
for nested_attr in drain {
if let NestedMeta::Meta(meta) = nested_attr.value() {
if !parse_attr(meta.clone(), &attr)? {
keep.push(nested_attr)
}
}
}

// The one that is always there is the condition in the cfg_attr (we put it in in
// is_cfg_feature_pyo3)
assert!(!keep.is_empty());
// If it's exactly 1, we handled all attributes
if keep.len() > 1 {
list.nested = Punctuated::from_iter(keep);

// Keep only the attributes we didn't parse.
// I couldn't find any method to just get the `attr.tokens` part again but with
// parentheses so here's token stream editing
let mut tokens = TokenStream::new();
list.paren_token.surround(&mut tokens, |inner| {
inner.extend(list.nested.to_token_stream())
});
attr.tokens = tokens;

return Ok(false);
}

// We handled this entire attribute, next
return Ok(true);
}
Ok(false)
}
Loading