John Hubbard
2025-Oct-27 18:46 UTC
[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits
On 10/26/25 9:44 AM, Miguel Ojeda wrote:> On Sun, Oct 26, 2025 at 3:40?PM Alexandre Courbot <acourbot at nvidia.com> wrote:...> Regarding the `.into_as()` name, it makes sense, but it can be a bit > surprising when reading out of context... The standalone functions are > super clear, in comparison. But I am not sure what could be better. > `into_in_this_arch()` or similar could emphasize that this will only > work in certain architectures, i.e. it is "an `into()` for this arch" > rather than the general one. > That would go well with the idea that you didn't implement it for > other obvious types, which I guess was to avoid developers using this > instead of `into()` by mistake, right? >Exactly: the into-as, from-as naming suffers from *appearing* to be familiar and readable, but actually, the naming gives no hint as to what it is really doing--nor how it is subtly different from the basic from/as/into standard conversions. Instead, we need to add something (almost anything) to the name, to make it clearly different from the from/as/into. into_for_arch() goes in that direction, for example. thanks, -- John Hubbard
Joel Fernandes
2025-Oct-27 18:58 UTC
[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits
On 10/27/2025 2:46 PM, John Hubbard wrote:> On 10/26/25 9:44 AM, Miguel Ojeda wrote: >> On Sun, Oct 26, 2025 at 3:40?PM Alexandre Courbot <acourbot at nvidia.com> wrote: > ... > >> Regarding the `.into_as()` name, it makes sense, but it can be a bit >> surprising when reading out of context... The standalone functions are >> super clear, in comparison. But I am not sure what could be better. >> `into_in_this_arch()` or similar could emphasize that this will only >> work in certain architectures, i.e. it is "an `into()` for this arch" >> rather than the general one. >> That would go well with the idea that you didn't implement it for >> other obvious types, which I guess was to avoid developers using this >> instead of `into()` by mistake, right? >> > > Exactly: the into-as, from-as naming suffers from *appearing* to be > familiar and readable, but actually, the naming gives no hint as to > what it is really doing--nor how it is subtly different from the > basic from/as/into standard conversions. > > Instead, we need to add something (almost anything) to the name, to > make it clearly different from the from/as/into. > > into_for_arch() goes in that direction, for example.100% agree with John. Thanks, - Joel
Alexandre Courbot
2025-Oct-28 14:44 UTC
[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits
On Tue Oct 28, 2025 at 3:46 AM JST, John Hubbard wrote:> On 10/26/25 9:44 AM, Miguel Ojeda wrote: >> On Sun, Oct 26, 2025 at 3:40?PM Alexandre Courbot <acourbot at nvidia.com> wrote: > ... > >> Regarding the `.into_as()` name, it makes sense, but it can be a bit >> surprising when reading out of context... The standalone functions are >> super clear, in comparison. But I am not sure what could be better. >> `into_in_this_arch()` or similar could emphasize that this will only >> work in certain architectures, i.e. it is "an `into()` for this arch" >> rather than the general one. >> That would go well with the idea that you didn't implement it for >> other obvious types, which I guess was to avoid developers using this >> instead of `into()` by mistake, right? >> > > Exactly: the into-as, from-as naming suffers from *appearing* to be > familiar and readable, but actually, the naming gives no hint as to > what it is really doing--nor how it is subtly different from the > basic from/as/into standard conversions. > > Instead, we need to add something (almost anything) to the name, to > make it clearly different from the from/as/into. > > into_for_arch() goes in that direction, for example.I'd like to get more input on that, for I am not sure how we can stay succint in the naming, while carrying the relevant information. `into_arch` does not sound much more explanatory than `into_as` - the intent with the latter was to say "I would normally have done an `as`, but instead here is a method that attests that this operations is indeed lossless and safe". The best naming scheme I could think of is to have the methods carry the source or destination types: e.g. `from_usize` or `into_usize` (like the standalone functions), but that would require defining as many traits, and increase the number of imports - if we go that way, we might just as well drop the traits completely and use the standalone functions. `into_native` also comes to mind, but like `arch`, it can mean many things depending on the context. ... I think I still believe that `into_as` is the clearest name, once one has read the documentation for the trait - which one should be expected to do anyway. :)