On Thu, Aug 3, 2023 at 10:08 AM Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx> wrote: > I think the first question to answer would be whether we want to > expose `bindings`, i.e. what are the advantages/disadvantages? > > If `kernel` were a "normal library", then I would say we shouldn't, > because users should not need to care; and, in addition, the goal is > that leaf modules do not need to access them directly. > > But, as sometimes happen, it may still be quite useful for some > developers nevertheless (the same way documenting the internal/private > details could be). > > So, it would be nice to have an overview from your point of view on > why it should be done (or not). I do understand that dilemma, but am not sure what the happy medium is between rendering them and hiding them. Where I see value is: 1. For anyone reading/writing abstractions, it's useful to quickly see how exactly bindgen did the C -> Rust mapping 2. Abstractions may want to link to the C side somehow, linking the bindings is an easier first step than linking to sphinx (in the future we may be able to do a bindings -> sphinx link) Maybe a stronger "prefer abstractions over bindings" message would suffice to discourage use outside of reference? In any case, I will put this behind a flag so it is not enabled by default. While I'm at it - is there value in adding a config option to pass `--document-private-items` to the kernel crate, or supporting `RUSTDOCFLAGS` like Cargo does? > > 1. Do we want to make this the default, or a separate target/ > > configuration? I don't think there is much downside to always > > generating. > > One downside of doing it by default would be going against the "avoid > `bindings`" guideline (ideally rule). > > Another one is render time (the C side is trying to reduce it), I > guess, especially if we keep adding headers over time. That makes sense, I will add the flag option. > > 2. The entire '.config' file could be included in the doc output, to > > make it easy to tell what settings the documentation was generated > > with. Would this be desired? Maybe with a '--cfg > > include-dotcfg=".config"' flag so published docs would have the > > option (unsure whether it may ever have sensitive information). > > This may be useful orthogonally to rendering `bindings` or not. > I will add this in a separate patch. > > Bindgen is currently invoked with '--no-doc-comments', I think this may > > be because some blocks were mistakenly interpreted as doctests. Once we > > upgrade our bindgen version we might be able to remove this. > > Yes, that is https://github.com/Rust-for-Linux/linux/issues/323 and > https://github.com/rust-lang/rust-bindgen/issues/2057, which led to > the addition of `process_comments` to `bindgen` in v0.63.0. How would switching to the library work? Since that seems like a more involved discussion, would postprocesing `generated_bindings.rs` be acceptable instead? I have been playing around with a python script that extracts the `#[doc ...]` blocks and (1) fixes the escaping and (2) formats parameters and fixes their spacing, I could extract this to a separate patch if it may be a while before we can use the library. > > Side note, 'rust/Makefile' seems to have a mix of tabs and spaces - is > > this intentional? > > Yes, it is intentional. For instance, the command definitions use > spaces like the vast majority of the kernel `Makefile`s. Ah thanks, it just looks a bit weird in the diff. > Cheers, > Miguel Thanks! Trevor
