Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Specialization of decoding for Bytes #135

Closed
wants to merge 1 commit into from

Conversation

vorner
Copy link
Contributor

@vorner vorner commented Nov 13, 2018

I don't know if I understood #31 suggestion correctly, so I'd like to ask about it. I think it could work somehow like this:

  • There would be a special method for decoding from bytes, that would default to just decoding from Buf.
  • Generated Message instances would implement it similar as they implement the basic decoding.
  • While most values can use the default, the newly supported Bytes would get a specialization and just use the part of the original buf.

While this probably would work, the in here looks like a big kludge to me. So did you have something else in mind?

I guess specialization (or how the feature is called) would greatly help, but it's not coming soon into Rust, is it?

@danburkert
Copy link
Collaborator

@vorner there shouldn't be a special method on Message for decoding from Bytes; specialization makes that unnecessary. To expand on the second bullet point in #31 specifically, there should be a new trait for string and bytes fields which allows for specialized decoding when the input buffer is a Bytes. That way you shouldn't need to change the call sites doing decoding at all, it should 'just work' as long as the message type has zero-copy configured fields, and the buffer is a Bytes instance.

@vorner
Copy link
Contributor Author

vorner commented Nov 14, 2018

I'm probably having some kind of mental roadblock here 😇

Specialization: you mean this one, that's still nightly only? rust-lang/rust#31844 Or is there anything else I'm missing? A very small example would probably be enough for me and I'd take it the rest of the way to implement it on everything that needs it.

@danburkert
Copy link
Collaborator

Closing this out per the comment I left in #31, please feel free to file a new issue if the solution in master doesn't work for you. Thanks!

@danburkert danburkert closed this Nov 15, 2020
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.

None yet

2 participants