Mark a function to be used carefully

I'm a beginner in Rust (and programming at all), and I'm trying to make a binary tree.

#[derive(Debug,Clone)]
pub enum BinaryTree<T: Ord>{
    Node  {
        value: T,
        left: Box<Self>,
        right: Box<Self>},
    Leaf,
}

impl<T : Ord> BinaryTree<T>{
    pub fn new_empty () -> Self {//...}
    pub fn new_with_root (root: T) -> Self{//...}

    /// Inserts the element `el` in the structure
    pub fn insert(&mut self, el: T) {
        // CONSISTENCY (BinaryTree::get_mut): 
        // Since we search for el, we will insert it at the right place.
        match self.get_mut(&el) {
            Ok(_) => {},
            Err(leaf) => {
                *leaf = Self::new_with_root(el);
            }
        }
    }

   /// Gets a reference to the found element `el`
    pub fn get(&self, el: &T) -> Option<&T>{}

    /// Gets a mutable reference to the searched element, or a reference to the
    /// spot where it belongs.
    ///
    /// CONSISTENCY: Mutating `el` must not change its ordering, or it could break
    /// the ordered structure.
    fn get_mut(&mut self, el: &T) -> Result<&mut T,&mut Self>{//...}
    }
}

Writing the function BinaryTree::get_mut() I realised that someone might change the ordering of the element (e.g. by changing the value), so I removed the pub in front of it, so that those mistakes are only possible in the scope of BinaryTree. Then I reminded (having watched some of Crust of Rust) that it is good practice when dealing with unsafe to write a // SAFETY: ... comment to explain why it is ok to do what we are doing.

So in my BinaryTree::insert() function I wrote a // CONSISTENCY: ... comment to explain. I would have wanted to mark the function BinaryTree::get_mut() with something similar to unsafe so that the compiler would warn me of a spot where I didn't tell "it's ok". To make my self clear, I would have wrote this, but I know that unsafe is inappropriate:

impl<T : Ord> BinaryTree<T>{

    /// Inserts the element `el` in the structure
    pub fn insert(&mut self, el: T) {
        // CONSISTENCY (BinaryTree::get_mut): 
        // Since we search for el, we will insert it at the right place.
        match unsafe { self.get_mut(&el) } {
            Ok(_) => {},
            Err(leaf) => {
                *leaf = Self::new_with_root(el);
            }
        }
    }

    /// Gets a mutable reference to the searched element, or a reference to the
    /// spot where it belongs.
    ///
    /// CONSISTENCY: Mutating `el` must not change its ordering, or it could break
    /// the ordered structure.
    unsafe fn get_mut(&mut self, el: &T) -> Result<&mut T,&mut Self>{//...}
    }
}

I marked it unsafe so that if I forget that BinaryTree::get_mut() has to be used carefully, the compiler errors/warns me, I check if "it's ok", mark the usage as unsafe and write the comment to explain.

If I understood well what this definition means, unsafe marks memory safety, and shouldn't be used as I did (one of the reasons is that BinaryTree::get_mut() isn't doing anything unsafe and I don't want the compiler to not tell me if I do, because I shouldn't).

So my question is: Is there any way to "mark" functions ? Or is there a good practice that solves my problem ?

(to mark functions I read a few things about attribute macros, but I don't know if that is achievable, and since I know very little about macros I didn't want to try something impossible)

Not really. You might want to take a look at what HashMap does, as it has the same problem.

It is a logic error for a key to be modified in such a way that the key's hash, as determined by the Hash trait, or its equality, as determined by the Eq trait, changes while it is in the map. This is normally only possible through Cell , RefCell , global state, I/O, or unsafe code. The behavior resulting from such a logic error is not specified, but will not result in undefined behavior. This could include panics, incorrect results, aborts, memory leaks, and non-termination.

The HashMap type does not expose any methods for getting a mutable reference to a key at all.

2 Likes

If something is memory-safe, but potentially error-prone, I would suggest giving it a long and detailed name.

For example, in non-Rust languages I might name a function FrobbleAssumingAlreadyLocked to emphasize both "you probably just want the normal Frobble unless you're doing something special" and "if you're going to call this, you better be careful".

(Of course, best is to find a way to make the mistakes impossible in the type system, but some things do need the extra escape hatch.)

1 Like