Re: [PATCH v8 1/1] rust: pci: add extended capability and SR-IOV support
"Alexandre Courbot" <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <[email protected]> |
On Tue Aug 25, 2026 at 1:25 AM JST, Gary Guo wrote:
> On Mon Aug 24, 2026 at 4:46 PM BST, Gary Guo wrote:
>> On Mon Aug 24, 2026 at 4:21 PM BST, Alexandre Courbot wrote:
>>> On Mon Aug 24, 2026 at 8:59 PM JST, Gary Guo wrote:
>>>>
>>>> Aesthetic and ergnomics is a big part of code, and is what make people like
>>>> Rust. In a world where less code is produced by human, I think it's especially
>>>> important that code remains easily human readable.
>>>>
>>>> You can probably already tell that I have very strong opinion about this.
>>>
>>> So do I, and the fact we both have opinions on the matter is irrelevant.
>>> "It looks horrible" is not an argument.
>>
>> Ergnomics is a perfectly valid argument and often a key factor in design
>> decisions. Otherwise we'd make all functions carry extra argument indicating the
>> context they're in and would require sleeping functions to carry such token
>> types. We rejected that approach because it'll infect all functions and make
>> Rust code look horrible.
>>
>> Everything is a trade-off. Whether to perform extra validation vs better
>> ergnomics is a genuine thing that needs deliberation. A outright dismissal of
>> the argument is itself a non-constructive argument.
>>
>>>
>>> Safety and correctness are the very reason for using Rust, not that it
>>> looks better or is more ergonomic. It often does, it sometimes doesn't
>>> (see the hoops we have to jump through to cast a pointer for instance),
>>> and when it does, that is usually to improve correctness, not to
>>> compromise it. The reason we go through these lengths is to remove
>>> issues and footguns at build-time, and this is exactly what these
>>> helpers do because `as` is one of the footguns.
>>
>> There's nothing unsafe or incorrect in using `as`. Your suggested code is no
>> more correct than Zhi's current version. You just added some extra check to some
>> constant that will never change, and we know will not produce error. My argument
>> is that that makes the syntax horrible and it's not a trade-off worth making.
>>
>> When we create new abstractions, we try to make thing has minimal overhead.
>> Otherwise we'd be using a GC language and that'd solve memory safety issues.
>> Your abstraction is not zero-cost -- it sacrifices compilation-time, binary size
>> (for debug info) and it adds a significant cognitive overhead for writing code.
>
> The code here is const eval, so debug info doesn't apply. And for the specific
> cases of functions containing only a single cast, as all instructions are going
> to be optimized out debug info should (hopefully) not be left too. However,
> metadata sizes would still be larger because rustc record all inlined functions
> within its MIR.
>
> Anyway, this is really more an auxiliary point, my main focus is on the
> cognitive complexity.
As it happens I share that concern - about reviewer cognitive load
particularly. Every time one encounters these `CAST` comments, they need
to re-compute the proof that the conversion is indeed lossless in their
head. I want to move that unneeded labor to the compiler. Whether the
constant ever changes or not, the `CAST` comment is still there, adding
burden to the reader. And many of these are generated bindings from the
C side, which *can* change without us noticing.
Now as I mentioned earlier we can turn these into a macro that emits a
const block checking that the conversion is lossless. The SRIOV constant
would then become:
pub const SRIOV: Self = Self(casts::const_as!(bindings::PCI_EXT_CAP_ID_SRIOV => u16));
No turbofish, a single macro instead of a multitude of functions, and
the name conveys what it is better: an `as` checked at const time. I've
sent a series [1]; hopefully it addresses your concern about ergonomics.
[1] https://lore.kernel.org/rust-for-linux/[email protected]/