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

Associative constants are lazy #993

Open
Ddystopia opened this issue Nov 9, 2024 · 1 comment
Open

Associative constants are lazy #993

Ddystopia opened this issue Nov 9, 2024 · 1 comment

Comments

@Ddystopia
Copy link

Hello, looking at rtic_sync::channel I saw this:

impl<T, const N: usize> Channel<T, N> {
    const _CHECK: () = assert!(N < 256, "This queue support a maximum of 255 entries");

    const INIT_SLOTS: UnsafeCell<MaybeUninit<T>> = UnsafeCell::new(MaybeUninit::uninit());

    /// Create a new channel.
    pub const fn new() -> Self {
        Self {
            freeq: UnsafeCell::new(Deque::new()),
            readyq: UnsafeCell::new(Deque::new()),
            receiver_waker: WakerRegistration::new(),
            slots: [Self::INIT_SLOTS; N],
            wait_queue: WaitQueue::new(),
            receiver_dropped: UnsafeCell::new(false),
            num_senders: UnsafeCell::new(0),
        }
    }

    /// Split the queue into a `Sender`/`Receiver` pair.
    pub fn split(&mut self) -> (Sender<'_, T, N>, Receiver<'_, T, N>) {
        // Fill free queue
        for idx in 0..N as u8 {
            debug_assert!(!self.freeq.get_mut().is_full());

            // SAFETY: This safe as the loop goes from 0 to the capacity of the underlying queue.
            unsafe {
                self.freeq.get_mut().push_back_unchecked(idx);
            }
        }

        debug_assert!(self.freeq.get_mut().is_full());

        // There is now 1 sender
        *self.num_senders.get_mut() = 1;

        (Sender(self), Receiver(self))
    }

    fn access<'a>(&'a self, _cs: critical_section::CriticalSection) -> UnsafeAccess<'a, N> {
        // SAFETY: This is safe as are in a critical section.
        unsafe {
            UnsafeAccess {
                freeq: &mut *self.freeq.get(),
                readyq: &mut *self.readyq.get(),
                receiver_dropped: &mut *self.receiver_dropped.get(),
                num_senders: &mut *self.num_senders.get(),
            }
        }
    }
}
   

Here _CHECK is actually ignored - it is never executed. And I indeed can create channel with capacity 300.

How to fix it

In new this line should be added:

        Self::_CHECK;

This will force _CHECK to be computed, and assert fired. Note, that this should be done in every constructor of the type, even private.

Other code

I don't know the codebase, but you probably do. Every such place should be fixed, so I will not pr this particular change.

@perlindgren
Copy link
Collaborator

Good catch!

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

No branches or pull requests

2 participants