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

Remove wrangling since std types are just re-exports of core and alloc types whenever possible #28

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

the10thWiz
Copy link
Collaborator

@the10thWiz the10thWiz commented Dec 10, 2024

This also makes the std feature dependent on alloc.

I'm pretty sure serde has the import wrangling section simply so they can use crate::lib::*; in both the serialize and deserialize modules. My understanding is that import wrangling should never be needed.

Every std type with a corresponding core or alloc type is (to the best of my knowledge) actually just a re-export of the core or alloc type. For crates that don't need alloc or std only types, they can just declare #![no_std] unconditionally, and still work just fine with std crates.

@JRRudy1
Copy link
Owner

JRRudy1 commented Dec 10, 2024

I knew that was the case for core vs. std, but wasn't sure if it worked the same way for alloc and didn't find any definitive answers after a quick google. I want to dig a little bit more just to make sure there aren't cases where importing from alloc prevents optimizations or anything, but your solution is much cleaner if all else is equal

@JRRudy1
Copy link
Owner

JRRudy1 commented Dec 10, 2024

I read through std/lib.rs and std/prelude.rs and I'm convinced. Originally I wasn't positive whether the alloc crate is linked in differently or something, but you are totally right that it is just reexported. Thanks a lot for the review and edits, it definitely simplifies things 👍

@JRRudy1 JRRudy1 merged commit 4f90c2a into JRRudy1:no_std Dec 10, 2024
15 checks passed
@the10thWiz the10thWiz deleted the no_std_without_wrangling branch December 11, 2024 06:42
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.

2 participants