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.

Sign up for GitHub

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

Jump to bottom

Refactor to use TryFrom #85

Open
hawkw opened this issue Mar 10, 2017 · 4 comments
Open

Refactor to use TryFrom #85

hawkw opened this issue Mar 10, 2017 · 4 comments
Labels
contrib/easy Contributing: this issue likely requires less time or experience than other issues. contrib/good first issue Contributing: this issue would make a good starting point for a first-time contributor. kind/refactor Kind: this issue describes refactoring or code quality improvement.

Comments

@hawkw
Copy link
Member

hawkw commented Mar 10, 2017

Since the TryFrom trait is in unstable Rust (rust-lang/rust#33417), we should rewrite a lot of failable conversions into TryFrom implementations. For example, a lot of the ELF parsing code that returns Result<Self, ElfError> can probably be rewritten to use TryFrom.

Where removing existing failable conversion methods would break a lot of other code, we can temporarily preserve the existing API by having it wrap a try_from call, and phase it out gradually.

We don't have to worry about TryFrom not being on stable, since we can't build SOS on stable anyway.
@hawkw hawkw added contrib/easy Contributing: this issue likely requires less time or experience than other issues. kind/refactor Kind: this issue describes refactoring or code quality improvement. labels Mar 10, 2017
@hawkw
Copy link
Member Author

hawkw commented Mar 10, 2017

I'm marking this as "easy" since it is a reasonably simple refactoring that doesn't require a great deal of OS knowledge.

hawkw added a commit that referenced this issue Mar 10, 2017
@hawkw
chore(kernel, cpu): add TODO comments for issue #85
cc21292 
[ci skip]
hawkw added a commit that referenced this issue Mar 10, 2017
@hawkw
chore(elf): add more TODO comments for issue #85
b2352d1
hawkw added a commit that referenced this issue Mar 10, 2017
@hawkw
refac(elf): start on TryFrom impls for #85
514d792
@hawkw hawkw mentioned this issue Mar 11, 2017
Merged
@hawkw hawkw added the contrib/good first issue Contributing: this issue would make a good starting point for a first-time contributor. label Jan 20, 2018
@tirkarthi
Copy link

As an update to the issue the above RFC was implemented and merged with PR : rust-lang/rust#49305 . It caused notable breakages in the ecosystem and got reverted : rust-lang/rust#49518

@hawkw
Copy link
Member Author

hawkw commented Apr 2, 2018

@tirkarthi my understanding is that all that was reverted is the addition of TryFrom to the prelude, the traits are still stabilized, correct?

@tirkarthi
Copy link

Yes, the team is looking for a way to introduce this with less breakages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
contrib/easy Contributing: this issue likely requires less time or experience than other issues. contrib/good first issue Contributing: this issue would make a good starting point for a first-time contributor. kind/refactor Kind: this issue describes refactoring or code quality improvement.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hawkw @tirkarthi