Remove doc_cfg attributes

[mejrs]

with "Make cfg imply doc(cfg)" rust-lang/rust#89596, these attributes are no longer necessary.

One pain point is that now doc_cfg generates info that's sort of...wonky for some things in the ffi module.

For example...

#[repr(C)]
#[derive(Copy, Clone, Debug)]
#[cfg(not(PyPy))]
pub struct PyObject {
    #[cfg(py_sys_config = "Py_TRACE_REFS")]
    _ob_next: *mut PyObject,
    #[cfg(py_sys_config = "Py_TRACE_REFS")]
    _ob_prev: *mut PyObject,
    pub ob_refcnt: Py_ssize_t,
    pub ob_type: *mut PyTypeObject,
}

#[repr(C)]
#[derive(Debug, Copy, Clone)]
#[cfg(PyPy)]
pub struct PyObject {
    pub ob_refcnt: Py_ssize_t,
    pub ob_pypy_link: Py_ssize_t,
    pub ob_type: *mut PyTypeObject,
}

...looks like this...

..and this..

I think the best way to fix that is to combine them into a single struct (?). That way it's easy to document the fields as well.

Alternatively there's doc(cfg(all)) to override that.

I'm personally leaning towards the first option. What do you think?

[mejrs]

@davidhewitt Are these supposed to have a #[cfg(Py_3_10)]?

[davidhewitt]

Yes my bad, good catch!

[davidhewitt]

Thanks for updating this!

I'm personally leaning towards the first option. What do you think?

I agree that sounds better, because presumably that way the individual fields get their cfg information rendered correctly?

[davidhewitt]

Yes my bad, good catch!

[mejrs]

I agree that sounds better, because presumably that way the individual fields get their cfg information rendered correctly?

It doesn't look like it. I think this feature is designed for a system where all features are additive. PyO3 is not one of those.

With it condensed in one struct, this is how it renders:

I've experimented with this and it seems the conditional compilation happens before the rustdoc pass. So only if those fields are active at the time are the field and the "is supported on..." banner rendered. For example, it does render the ob_pypy_link field if --cfg PyPy is passed to rustdoc. However that also gets rid of all the non(PyPy) banners elsewhere.

There is a cursed workaround for it, of course:

#[repr(C)]
#[derive(Copy, Clone, Debug)]
pub struct PyObject {
    #[cfg(py_sys_config = "Py_TRACE_REFS")]
    pub _ob_next: *mut PyObject,
    #[cfg(py_sys_config = "Py_TRACE_REFS")]
    pub _ob_prev: *mut PyObject,
    #[cfg(docsrs)]
    #[doc(cfg(py_sys_config = "Py_TRACE_REFS"))]
    pub _ob_next: *mut PyObject,
    #[cfg(docsrs)]
    #[doc(cfg(py_sys_config = "Py_TRACE_REFS"))]
    pub _ob_prev: *mut PyObject,
    pub ob_refcnt: Py_ssize_t,
    #[cfg(PyPy)]
    pub ob_pypy_link: Py_ssize_t,
    #[cfg(docsrs)]
    #[doc(cfg(PyPy))]
    pub ob_pypy_link: Py_ssize_t,
    pub ob_type: *mut PyTypeObject,
}

However, given that...

• the feature is nightly
• it adds some tedious and error prone cfg magic
• it would be dangerous/confusing if somehow someone uses pyo3 with a docsrs cfg active

...I'm really not that enthusiastic about doing that. I'm not sure how many things would require covering it like this. I suppose we could do it for a couple of really important things...?

[mejrs]

Oh yeah, it also doesn't work correctly if the cfg is on the module and glob imports are used.

e.g. this does not, at this time, apply the "whatever" banner to B, if it is imported with pub use a::*;

#[cfg(whatever)]
pub mod a{
    pub struct B;
}

So a lot of our ffi module has missing not(Py_LIMITED_API) banners.

[davidhewitt]

Unngh, ok. I think let's not worry about the cfg at field-level for now then? As you say, the feature is nightly so with luck it'll improve (especially if we report these papercuts upstream).

Oh yeah, it also doesn't work correctly if the cfg is on the module and glob imports are used. [...] So a lot of our ffi module has missing not(Py_LIMITED_API) banners.

Hmm, that's frustrating. I think at least it's no worse that what we currently have? I guess that that's another papercut which hopefully will get fixed in time if we report it.

[davidhewitt]

Code looks good! Possibly silly question, what invocation are you using to build the docs locally? Trying this branch I don't see any feature flags at all with latest nightly so I imagine I'm doing something wrong 😅

[mejrs]

Code looks good! Possibly silly question, what invocation are you using to build the docs locally? Trying this branch I don't see any feature flags at all with latest nightly so I imagine I'm doing something wrong 😅

You need to use the docsrs cfg, the actual #![feature(doc_cfg)] is still necessary. The incantation I use is

cargo +nightly rustdoc --no-default-features --features="macros num-bigint num-complex hashbrown indexmap serde multiple-pymethods eyre anyhow" --open -- --cfg docsrs

[davidhewitt]

Aaah perfect - that works and looks nice locally 👍