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

[derive] Support deriving NoCell #667

Merged
merged 1 commit into from
Dec 6, 2023
Merged

[derive] Support deriving NoCell #667

merged 1 commit into from
Dec 6, 2023

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Dec 2, 2023

Makes progress on #251

Copy link
Member Author

@joshlf joshlf left a comment

Choose a reason for hiding this comment

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

TODO: Add tests for the types described in #656 (comment):

Can you expand on a few edge cases here? I'm not totally clear on what "contains" means. Is this NoCell?

struct R<'a> {
    field: &'a UnsafeCell<u8>,
}

...and this?

struct ZLA {
    field: [UnsafeCell<u8>; 0];
}

...and this?

struct CPC {
    field: PhantomData<UnsafeCell<u8>>;
}

...and this?

trait Trait {
    type Assoc;
}

impl<T> Trait for UnsafeCell<T> {
    type Assoc = T;
}

struct CAT<T>
where
    T: NoCell
{
    field: <UnsafeCell<T> as Trait>::Assoc,
}

...and this?

enum Enum {
    Uninhabited(!, UnsafeCell<u8>),
    Inhabited(u8),
}

Copy link
Collaborator

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

For testing (and potentially the derive expansion), you might find this macro nifty:

macro_rules! assert_unfreeze {
    ($ty:ty) => {
        const _: () = {
            static _assert_unfreeze: Option<$ty> = None;
        };
    }
}

assert_unfreeze!(u8); // accepted!

assert_unfreeze!(core::cell::UnsafeCell<u8>); // rejected! compilation error!

Base automatically changed from no-cell to main December 6, 2023 20:44
@joshlf joshlf force-pushed the no-cell-derive branch 2 times, most recently from ea82fcf to a5cc3f1 Compare December 6, 2023 22:22
@joshlf
Copy link
Member Author

joshlf commented Dec 6, 2023

For testing (and potentially the derive expansion), you might find this macro nifty:

macro_rules! assert_unfreeze {
    ($ty:ty) => {
        const _: () = {
            static _assert_unfreeze: Option<$ty> = None;
        };
    }
}

assert_unfreeze!(u8); // accepted!

assert_unfreeze!(core::cell::UnsafeCell<u8>); // rejected! compilation error!

Didn't end up needing this, but it's very nifty!

@joshlf joshlf requested a review from jswrenn December 6, 2023 22:24
@joshlf joshlf added this pull request to the merge queue Dec 6, 2023
Merged via the queue into main with commit 949c7bb Dec 6, 2023
126 checks passed
@joshlf joshlf deleted the no-cell-derive branch December 6, 2023 22:39
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