Readability Difficulty [LWN.net]

Readability Difficulty

Posted Jul 19, 2025 6:42 UTC (Sat) by burki99 (subscriber, #17149) [Link] (1 responses)

Lisp programmers might disagree :-)

Readability Difficulty

Posted Jul 19, 2025 8:14 UTC (Sat) by Wol (subscriber, #4433) [Link]

Likewise PL/1 :-)

Cheers,
Wol

Readability Difficulty

Posted Jul 19, 2025 10:04 UTC (Sat) by DOT (subscriber, #58786) [Link] (4 responses)

Some newlines and indentation make it bulky, but much easier to parse:

data <- new_mutex!(
    (
        KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?,
        0,
    )
),

A good guideline might be to break out into multiple lines whenever you would get nested brackets of the same type. Brackets of different types seem to be easier to parse on one line.

Readability Difficulty

Posted Jul 20, 2025 1:06 UTC (Sun) by alfredtaylor (guest, #178411) [Link] (3 responses)

That looks much easier to parse, I personally love using newlines and indentation like that.

Does rustfmt autoformat like that, or can rustfmt be configured to autoformat like that?

Readability Difficulty

Posted Jul 21, 2025 1:05 UTC (Mon) by mathstuf (subscriber, #69389) [Link] (2 responses)

`rustfmt` generally ignores the contents of declarative macro calls (builtins like `vec!` or `concat!` are exceptions).

Readability Difficulty

Posted Jul 21, 2025 8:19 UTC (Mon) by excors (subscriber, #95769) [Link] (1 responses)

I believe that's incorrect: rustfmt will (by default) try to format `foo!()` like a function call, and `foo![]` like an array literal, if their content is valid Rust syntax. If they're not valid syntax, it will leave them unchanged. And it will never format `foo!{}`.

When it doesn't reformat the macro, it will still change their indentation to match their scope, if the final line of the macro (minus leading/trailing whitespace) is empty or contains only `}`, `)`, `]` characters. And it has special rules for `lazy_static!` and `vec!`.

(This is based on https://github.com/rust-lang/rustfmt/discussions/5437 and https://github.com/rust-lang/rustfmt/blob/master/src/macr... and some testing.)

The `try_pin_init!` example is not valid syntax: `data <- foo` is valid by itself, because it's like the boolean expression `data < (-foo)`, but `Self { expression }` is not valid (fields must be `identifier` or `identifier: expression`). The last line is just `}`, so rustfmt can change its indentation but won't do any other formatting.

Readability Difficulty

Posted Jul 21, 2025 10:45 UTC (Mon) by mathstuf (subscriber, #69389) [Link]

Interesting. Maybe we're just on an old version (we pin to avoid formatting "fights" with version skews). My `log::` macros never seem to get formatted even with `()` calls. But maybe it is just the `target:` instances that are subject to that.

Readability Difficulty

Posted Jul 20, 2025 6:07 UTC (Sun) by donald.buczek (subscriber, #112892) [Link] (1 responses)

You don't really need to match the parenthesis once you get used to the function signatures.

What I don't like here is the domain-specific language defined by the macros. `<-` is not a Rust operator. So even a Rust expert with extraordinary ability to count parentheses couldn't parse that. You need to look up the macro docs or definitions.

Readability Difficulty

Posted Jul 21, 2025 18:34 UTC (Mon) by NYKevin (subscriber, #129325) [Link]

> What I don't like here is the domain-specific language defined by the macros. `<-` is not a Rust operator. So even a Rust expert with extraordinary ability to count parentheses couldn't parse that. You need to look up the macro docs or definitions.

Yeah, I'm not thrilled with that either. It's strange, because I would think you could just write something like this:

field: in_place!(expr)

Macros, to the best of my understanding, can parse macro-like syntax even if the inner macro does not exist (so in_place!() does not need to be a "real" macro in its own right, it can just be magic syntax that is recognized by pin_init!()). Of course, you would still need to document this magic syntax in pin_init!(), but at least it would look more like Rust.

It is entirely legal for a macro to emit macro_rules! definitions, so you can have"real" macros that only exist inside of other macros, if you so desire (but that's probably unnecessary if you're writing a proc macro anyway). Conveniently, you don't need to do tt-munching if the macro call originates in the matched fragment (and you're just passing it through unchanged).

Readability Difficulty

Posted Jul 20, 2025 10:34 UTC (Sun) by excors (subscriber, #95769) [Link]

I think a large part of the problem is that it's storing a tuple inside the mutex, and tuples often get hard to read because they're all punctuation and no names. There isn't even a comment explaining what the second field in the (KBox<[u8; PAGE_SIZE]>, usize) is meant to represent, so I don't think this is good coding style.

It might be better if the tuple was replaced with a struct like:

struct ConfigData {
    data: KBox<[u8; PAGE_SIZE]>,
    len: usize,
}

impl ConfigData {
    fn new() -> Result<Self> {
        Ok(Self {
            data: KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?,
            len: 0,
        })
    }
}

so the fields are properly labeled, and now it can be initialised with the much more readable:

try_pin_init!(Self {
    data <- new_mutex!(ConfigData::new()?),
})

and the fields can be accessed with the self-explanatory guard.data.as_slice() instead of guard.0.as_slice().

Tuples are fine for trivial types like rgb: (u8, u8, u8), or for short-lived anonymous types like multiple return values from a function, but in most other cases I think it's worth the extra few lines of declaring a struct so you can name the fields and split out the new() implementation. (And this particular case looks like essentially a KVec<u8> with a fixed capacity of PAGE_SIZE, so the declaration could go in some common utility module and not in each driver. Or just use KVec.)