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

Detect and Fix Overscope unsafe Block #13515

Open
lwz23 opened this issue Oct 7, 2024 · 0 comments
Open

Detect and Fix Overscope unsafe Block #13515

lwz23 opened this issue Oct 7, 2024 · 0 comments
Labels
A-lint Area: New lints

Comments

@lwz23
Copy link

lwz23 commented Oct 7, 2024

What it does

In actual development, I often see unsafe blocks of code that are abused. For example, in one version of the Smallvec library there is a function that looks like this:

pub fn grow(&mut self, new_cap: usize) {
        unsafe {
            let (ptr, &mut len, cap) = self.triple_mut();
            let unspilled = !self.spilled();
            assert!(new_cap >= len);
            if new_cap <= self.inline_size() {
                if unspilled {
                    return;
                }
                self.data = SmallVecData::from_inline(mem::uninitialized());
                ptr::copy_nonoverlapping(ptr, self.data.inline_mut().ptr_mut(), len);
            } else if new_cap != cap {
                let mut vec = Vec::with_capacity(new_cap);
                let new_alloc = vec.as_mut_ptr();
                mem::forget(vec);
                ptr::copy_nonoverlapping(ptr, new_alloc, len);
                self.data = SmallVecData::from_heap(new_alloc, len);
                self.capacity = new_cap;
                if unspilled {
                    return;
                }
            }
            deallocate(ptr, cap);
        }
    }

It obviously uses an unsafe block more than necessary, putting the entire function in an unsafe block. This can be disruptive to subsequent developers, who are unsure what part is truly unsafe. It would be better to use an unsafe block only to the smallest extent, using several small unsafe blocks of code, rather than an entire unsafe block that contains all functions. I want to try to implement a feature that can automatically detect (or, one step further, fix) such situations. Can rust-analyzer do this now, and is it necessary to add this feature?

Advantage

The recommended code follows Rust’s best practices, which enhances code readability and maintainability. By eliminating potential performance bottlenecks or misuse of unsafe patterns, it reduces the risk of runtime errors and undefined behavior. Furthermore, the recommended code ensures better compatibility with future Rust updates, as it adheres to idiomatic Rust principles, making it more resilient against deprecations or breaking changes.

Drawbacks

A possible drawback of such a lint might be that it could trigger false positives or penalize valid, but non-standard, coding practices. In some cases, the original code may be written for performance reasons, and refactoring it might introduce inefficiencies. Developers familiar with the original code might need additional time to adapt to the new style, which could slow down initial development and testing. Additionally, enforcing strict compliance could lead to overengineering in simple cases, adding unnecessary complexity to otherwise straightforward code.

Example

unsafe{
let mut vec = vec![1, 2, 3, 4, 5];
for i in 0..vec.len() {
    println!("{}", vec[i]);
}
}

Could be written as:

let mut vec = vec![1, 2, 3, 4, 5];
for i in 0..vec.len() {
    println!("{}", vec[i]);
}

emit overscoped unsafe block

@lwz23 lwz23 added the A-lint Area: New lints label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

1 participant