Re: [PATCH] gpu: nova-core: bitfield: use &mut self setters instead of builder pattern

[Joel Fernandes <joelagnelf@nvidia.com>]

Hi John,

On 12/31/2025 9:52 PM, John Hubbard wrote:
[..]
>>> But this really is fake chaining, because there are no Results involved.
>>> It's not buying us anything except a bit of indirection and cognitive
>>> load for the reader.
>>
>> Chaining is not really only about error propagation. Builder pattern can be used
>> for other cases too, like passing a setter chained expression to a function
>> argument for better clarity, I was planning to do that for the sequencer for
>> instance since there's a lot of parameters passed to it.
> 
> Let's see if that has any use for this.
> 
> So far, though, in the code base that we have today, there is absolutely
> zero benefit. The diffs here prove it.
> 

From your patch diff, I see the lines of code increased. But that's not even the
main issue I have with it (though IMO the chaining is more readable..).

>> But in this case, I am actually absolutely opposed against this, it makes the
>> API hard to use because now how do you differentiate between an IO function call
>> versus something that just mutates memory? Is set() an IO or write()?
> 
> That's a completely separate, pre-existing issue with the API.

Nope. With chaining we clearly know that the final operation is a write().

For instance, you cannot do:
  reg.set_foo()
     .write()
     .set_bar()

That wont compile. You cannot intermingle write() with set_XX() because write()
doesn't return anything that can be chained with. The builder pattern is typically:
  obj.set_something()
     .set_something()
     .do_some_action()

The 'set' can also be 'with' from what I've seen, whatever. The point is the
last thing is the action. IMO very readable and simple. I know that the write()
will be what ends up doing the I/O. It is one entity that culminates in the write().

> 
>>
>>       reg.set_foo(x);   // no IO
>>       reg.set_sec(y);
>>       reg.write(bar);   // IO.
>>
>> So no thank you, I quite dislike it. :)
>>
>> Instead with chaining, we can just rely on the last part of the chain concluding
>> in a write() with the intermediaries just mutating memory.
> 
> Same as above, just a more happy-happy chaining interface, but the same
> function calls must be made in the same order.

No, you cannot place write() anywhere except at the end of the chain - the type
system enforces this since write() returns ().

thanks,

 - Joel