Re: kerneldoc and rust (was [PATCH] kernel-doc: better handle '::' sequences)

[Jonathan Corbet]

Matthew Wilcox <willy@xxxxxxxxxxxxx> writes:

If we're going to talk about incorporating Rust into the doc system, we
should probably include some Rust folks - thus, I'm adding Miguel.

> On Thu, Mar 25, 2021 at 04:30:32PM -0600, Jonathan Corbet wrote:
>> Matthew Wilcox <willy@xxxxxxxxxxxxx> writes:
>> 
>> We did come to the mutual agreement that teaching kernel-doc to parse
>> Rust code as well was not an ideal solution.  Probably there will be
>> some sort of tool to translate between rustdoc and our sphinx setup.
>> Beyond that, we'll see how it goes.
>
> In the spirit of groping around for the best solution, I did some looking
> around at various options, including using rustdoc for .c files (that
> uses Markdown, which appears to be strictly worse than rST for our
> purposes).
>
> So here's my "modest proposal":
>
>  - Similar to our ".. kernel-doc::" invocation in .rst files, handle
>    ".. rustdoc::" (insert weeks of hacking here)
>  - Now add ".. rst-doc::" which parses .c files like [1] kernel-doc
>    does, but interprets a different style of comment and actually does
>    most of the repetitive boring bits for you.
>
> For example, xa_load:
>
> /**
>  * xa_load() - Load an entry from an XArray.
>  * @xa: XArray.
>  * @index: index into array.
>  *
>  * Context: Any context.  Takes and releases the RCU lock.
>  * Return: The entry at @index in @xa.
>  */
> void *xa_load(struct xarray *xa, unsigned long index)
>
> //rST
> // Load an entry from an XArray.
> //
> // :Context: Any context.  Takes and releases the RCU lock.
> // :Return: The entry in `xa` at `index`.
> void *xa_load(struct xarray *xa, unsigned long index)
>
> (more complex example below [2])
>
> Things I considered:
>
>  - Explicitly document that this is rST markup instead of Markdown or
>    whatever.
>  - Don't repeat the name of the function.  The tool can figure it out.

That worries me a wee bit just because a common source of problems is
kerneldoc comments becoming separated from the functions they describe
over time.  We finally have tooling to notice that; this seems like a
step in the other direction.

>  - Don't force documenting each parameter.  Often they are obvious
>    and there's really nothing interesting to say about the parameter.
>    Witness the number of '@foo: The foo' (of type struct foo) that we
>    have scattered throughout the tree.  It's not that the documenter is
>    lazy, it's that there's genuinely nothing to say here.

...another failure mode is developers adding parameters and not
documenting them; this would mask that problem too.

>  - Use `interpreted text` to refer to parameters instead of *emphasis* or
>    **strong emphasis**.  The tool can turn that into whatever markup
>    is appropriate.
>  - Use field lists for Context and Return instead of sections.  The markup
>    is simpler to use, and I think the rendered output is better.
>
> [1] by which i mean "in a completely different way from, but similar in
>     concept"
>
> [2] More complex example:
>
> /**
>  * xa_store() - Store this entry in the XArray.
>  * @xa: XArray.
>  * @index: Index into array.
>  * @entry: New entry.
>  * @gfp: Memory allocation flags.
>  *
>  * After this function returns, loads from this index will return @entry.
>  * Storing into an existing multi-index entry updates the entry of every index.
>  * The marks associated with @index are unaffected unless @entry is %NULL.
>  *
>  * Context: Any context.  Takes and releases the xa_lock.
>  * May sleep if the @gfp flags permit.
>  * Return: The old entry at this index on success, xa_err(-EINVAL) if @entry
>  * cannot be stored in an XArray, or xa_err(-ENOMEM) if memory allocation
>  * failed.
>  */
> void *xa_store(struct xarray *xa, unsigned long index, void *entry, gfp_t gfp)
>
> //rST
> // Store an entry in the XArray.
> //
> // After this function returns, loads from `index` will return `entry`.
> // Storing into an existing multi-index entry updates the entry of every index.
> // The marks associated with `index` are unaffected unless `entry` is ``NULL``.
> //
> // :Context: Any context.  Takes and releases the xa_lock.
> //    May sleep if the `gfp` flags permit.
> // :Return: The old entry at this index on success, xa_err(-EINVAL) if `entry`
> //    cannot be stored in an XArray, or xa_err(-ENOMEM) if memory allocation
> //    failed.
> void *xa_store(struct xarray *xa, unsigned long index, void *entry, gfp_t gfp)

Thanks,

jon