John Hubbard
2025-Oct-28 17:18 UTC
[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits
On 10/28/25 7:44 AM, Alexandre Courbot wrote:> 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.That's too many constraints: if you want an extremely short name that carries information, *and* avoids (as requested here) confusion with existing "as" methods, then...you can't. But you are allowed to be less succinct here, because the more specialized and rare a case is, the longer you can make the name. And here, you are definitely allowed a few more characters.> `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.Accurate names are really desirable; maybe we shouldn't completely close the door to the above approach. thanks, John Hubbard> > `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. :)
Alexandre Courbot
2025-Oct-28 22:32 UTC
[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits
On Wed Oct 29, 2025 at 2:18 AM JST, John Hubbard wrote:> On 10/28/25 7:44 AM, Alexandre Courbot wrote: >> 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. > > That's too many constraints: if you want an extremely short name > that carries information, *and* avoids (as requested here) confusion > with existing "as" methods, then...you can't. > > But you are allowed to be less succinct here, because the more > specialized and rare a case is, the longer you can make the name. > And here, you are definitely allowed a few more characters. > > >> `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. > > Accurate names are really desirable; maybe we shouldn't completely > close the door to the above approach.I think we have reached the stage where any responsible adult would shove this whole discussion into a LLM and see what it comes up with. And the candidate is... `FromSafeCast`/`IntoSafeCast`. Which I have to say sounds like a good middle ground? :) The intent is definitely to perform a safe cast here.