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

allow no-std but alloc #13

Merged
merged 5 commits into from
Sep 29, 2023
Merged

allow no-std but alloc #13

merged 5 commits into from
Sep 29, 2023

Conversation

dragazo
Copy link
Contributor

@dragazo dragazo commented Aug 30, 2023

There were a few types which implement LifetimeFree only in std mode. This PR lowers the requirement by introducing a new alloc feature that enables these traits in no-std mode but when alloc is still available. These types include String, Box, Vec, and Arc, which has an additional (automatic) cfg due to only existing on platforms with ptr-sized atomics.

@dragazo
Copy link
Contributor Author

dragazo commented Sep 15, 2023

As a side question: is it not sufficient to just unsafe impl<T: 'static> LifetimeFree for T {}? If the only requirement is not having non-'static lifetimes, I think that would do what we need for all conceivable types.

@NobodyXu
Copy link
Contributor

As a side question: is it not sufficient to just unsafe impl<T: 'static> LifetimeFree for T {}? If the only requirement is not having non-'static lifetimes, I think that would do what we need for all conceivable types.

The problem is that it would allow casting reference &T to &'static T

@sagebind
Copy link
Owner

As a side question: is it not sufficient to just unsafe impl<T: 'static> LifetimeFree for T {}? If the only requirement is not having non-'static lifetimes, I think that would do what we need for all conceivable types.

The requirement is actually stronger than that. Downcasting requires knowing about the actual concrete type and not just the generic constraints. The constraint T: 'static indicates that this instance of T is 'static, whereas T: LifetimeFree indicates that the actual reified type is always 'static. For example, given some struct Foo containing a lifetime, Foo<'static'>: 'static, but because Foo<'a>: !'static, therefore Foo: !LifetimeFree.

This extra-strict guarantee is how we ensure that it is safe to downcast a given type without knowing statically whether or not it is 'static in a generic context. So no, there's no way to write a generic impl for this in even nightly Rust as far as I know, and so has to be done on a case-by-case basis where the author of the type can guarantee it meets the constraints of LifetimeFree.

@dragazo
Copy link
Contributor Author

dragazo commented Sep 19, 2023

@NobodyXu @sagebind Ah, ok, that makes sense. Well so much for that idea lol Fortunately, that was just my hopeful musings - the changes in this PR should all be fine since they're known always-'static types.

src/lifetime_free.rs Outdated Show resolved Hide resolved
@sagebind
Copy link
Owner

The macos CI failures are unrelated to your PR and were fixed in #15 if you want to pull the latest master into your branch.

@dragazo
Copy link
Contributor Author

dragazo commented Sep 19, 2023

Oh, I wasn't aware target_has_atomic was that recent. I think your combination of 1 and 3 is probably best, like you said. That will work perfectly on more recent systems, even without atomics, and will just have errors for out-of-date rust versions that specifically target the somewhat rare devices that lack them. I just committed that change.

@sagebind
Copy link
Owner

The documentation will also need updated to reflect the new feature behaviors in the following places and probably clarify std vs core and alloc support:

castaway/src/lib.rs

Lines 3 to 6 in 7512ea7

//! This crate works fully on stable Rust, and also does not require the
//! standard library. To disable references to the standard library, you must
//! opt-out of the `std` feature using `default-features = false` in your
//! `Cargo.toml` file.

castaway/src/lib.rs

Lines 87 to 89 in 7512ea7

/// trait. This is implemented automatically for all primitive types and for
/// several `core` types. If you enable the `std` crate feature, then it will
/// also be implemented for several `std` types as well.

@dragazo
Copy link
Contributor Author

dragazo commented Sep 19, 2023

Alright, just added a little info about the new feature and fixed a broken link to crate::cast. I'm not sure why that link was broken, as it seems to be fine in the published version with no changes. I guess they changed resolution rules recently.

Copy link
Owner

@sagebind sagebind left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

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