Re: [PATCH] gpu: nova-core: bitfield: use &mut self setters instead of builder pattern
From: John Hubbard
Date: Wed Dec 31 2025 - 17:47:45 EST
On 12/31/25 2:33 PM, Timur Tabi wrote:
> On Wed, 2025-12-31 at 13:47 -0800, John Hubbard wrote:
>> The builder-pattern setters (self -> Self) enabled method chaining like:
>>
>> reg.set_foo(x).set_sec(y).write(bar);
>>
>> This made separate operations appear as a single expression, obscuring
>> that each setter is a distinct mutation.
>
> So you're concerned about the fact that the compiler is not merging the set_foo(x) and the
> set_sec(y) into a single read-modify-write?
No, I don't care about that aspect.
>
>> These setters are infallible,
>> so the chaining provides no error-propagation benefit—it just obscures
>> what are simple, independent assignments.
>>
>> Change the bitfield!() macro to generate `&mut self` setters, so each
>> operation is a distinct statement:
>>
>> reg.set_foo(x);
>> reg.set_sec(y);
>> reg.write(bar);
>
> Are you sure about this? It just seems like you're throwing out a neat little feature of Rust and
> replacing it with something that's very C-like. This breaks compatible with all users of the regs
> macros. Seems really disruptive for what seems to me like a cosmetic change.
>
It's only a neat feature if it *does* something. In this case, it *looks*
like a neat Rust feature, but under the covers it really is just handing
around copies unnecessarily, when really, it *is* doing the C-like thing
in the end.
I object to the fake Rust-ness that's being done here. It's like putting
hubcabs on a car.
thanks,
--
John Hubbard