Skip to content

Conversation

1911860538
Copy link
Contributor

Make the type parameter passing more explicit by changing
io.__hyper_downcast() to io.__hyper_downcast::<T>() in the
downcast method. This improves code clarity and avoids potential
type inference ambiguities in multi-generic scenarios.

@seanmonstar
Copy link
Member

Thanks for the PR! What motivated this change? I think the original code is pretty clear and impossible for inference to mess up...

@1911860538
Copy link
Contributor Author

Thanks for the PR! What motivated this change? I think the original code is pretty clear and impossible for inference to mess up...

Yes, because this method has only one generic parameter, the original code is impossible to encounter type inference issues. My modification primarily aims to enhance readability. Improved code clarity is achieved as explicit type parameters clearly indicate the target downcast type, making the code more readable and self-documenting.

@seanmonstar
Copy link
Member

Hm, ok. I lean towards leaving it alone, but if another reviewer feels differently, I could be swayed. @hyperium/triage

@1911860538
Copy link
Contributor Author

Hm, ok. I lean towards leaving it alone, but if another reviewer feels differently, I could be swayed. @hyperium/triage

😂

pub fn downcast<T: Read + Write + Unpin + 'static>(self) -> Result<Parts<T>, Self> {
let (io, buf) = self.io.into_inner();
match io.__hyper_downcast() {
match io.__hyper_downcast::<T>() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usually, the convention i am familiar with is to rely on "turbofish" syntax like this when the type parameter is ambiguous, and a specific concrete type must be provided. for example, iter.collect::<Vec<_>>().

my disposition is also to leave this call as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants