Rework raw_assign_to_drop-text, allow UnsafeCell::get

Author: lukaslueg

clippy_lints/src/operators/mod.rs

@@ -851,28 +851,38 @@ declare_clippy_lint! {
     /// uninitialized or otherwise invalid, the execution of `std::ops::Drop::drop(&mut self)`
     /// is always Undefined Behavior.
     ///
-    /// Use `std::ptr::write()` to overwrite a value without executing the destructor.
+    /// If the code can guarantee that the value being replaced during assignment is safe to
+    /// drop, it is recommended to
+    /// * silence this lint by adding a narrowly-scoped `#[expect(raw_assign_to_drop)]`-attribute
+    /// * make liberal use of SAFETY-comments e.g. around declaration of this unsafe pointer,
+    ///   reminding readers that this unsafe pointer *must* contain an initialized value.
     ///
-    /// Use `std::ptr::drop_in_place()` to conditionally execute the destructor if you are
-    /// sure that the place contains an initialized value.
+    /// If the code can *not* guarantee that the previous value can be safely dropped,
+    /// use `std::ptr::write()` to overwrite the previous (possibly nonexisting) value
+    /// without executing the destructor.
+    ///
+    /// Use `std::ptr::drop_in_place()` to conditionally execute the destructor.
     ///
     /// ### Example
     /// ```no_run
     /// unsafe fn foo(oldvalue: *mut String) {
-    ///     // Direct assignment always executes `String`'s destructor on `oldvalue`
-    ///     *oldvalue = "New Value".to_owned();
+    ///     // Direct assignment always executes `String`'s destructor on `oldvalue`.
+    ///     // However, we can't guarantee that executing the destructor is safe.
+    ///     unsafe { *oldvalue = "New Value".to_owned(); }
     /// }
     /// ```
     /// Use instead:
     /// ```no_run
     /// unsafe fn foo(oldvalue: *mut String, oldvalue_is_initialized: bool) {
+    ///     let newvalue = "New Value".to_owned();
     ///     if oldvalue_is_initialized {
     ///         // Having established that `oldvalue` points to a valid value, selectively
     ///         // execute the destructor to prevent a memory-leak
-    ///         oldvalue.drop_in_place();
+    ///         unsafe { *oldvalue = newvalue; }
+    ///     } else {
+    ///         // Overwrite the old value without running the destructor.
+    ///         unsafe { oldvalue.write(newvalue); }
     ///     }
-    ///     // Overwrite the old value without running the destructor unconditionally
-    ///     oldvalue.write("New Value".to_owned());
     /// }
     /// ```
     #[clippy::version = "1.85.0"]

clippy_lints/src/operators/raw_assign_to_drop.rs

@@ -11,6 +11,15 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, lhs: &'tcx Expr<'_>) {
         && let Some(deref_ty) = ty.builtin_deref(true)
         && deref_ty.needs_drop(cx.tcx, cx.typing_env())
     {
+        if let ExprKind::MethodCall(path, self_arg, [], ..) = expr.kind
+            && let rustc_middle::ty::Adt(ty_def, ..) = cx.typeck_results().expr_ty(self_arg).kind()
+            && ty_def.is_unsafe_cell()
+            && path.ident.as_str() == "get"
+        {
+            // Don't lint if the raw pointer was directly retrieved from UnsafeCell::get()
+            // We assume those to be safely managed
+            return;
+        }
         span_lint_and_then(
             cx,
             RAW_ASSIGN_TO_DROP,
@@ -22,9 +31,9 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, lhs: &'tcx Expr<'_>) {
                 ));
                 diag.span_label(
                     expr.span,
-                    "this place may be uninitialized, causing Undefined Behavior when the destructor executes",
+                    "the value may be uninitialized, causing Undefined Behavior when the destructor executes",
                 );
-                diag.help("use `std::ptr::write()` to overwrite a (possibly uninitialized) place");
+                diag.help("use `std::ptr::write()` to overwrite a possibly uninitialized place");
                 diag.help("use `std::ptr::drop_in_place()` to drop the previous value if such value exists");
             },
         );

tests/ui/raw_assign_to_drop.rs

@@ -1,7 +1,10 @@
 #![warn(clippy::raw_assign_to_drop)]
+#![allow(clippy::missing_safety_doc)]
 
-fn main() {
-    unsafe fn foo(r: *mut String, i: *mut i32) {
+use std::cell::UnsafeCell;
+
+pub unsafe fn foo(r: *mut String, i: *mut i32) {
+    unsafe {
         *r = "foo".to_owned();
 
         // no lint on {integer}
@@ -21,3 +24,13 @@ fn main() {
         *v.as_mut_ptr() = Default::default();
     }
 }
+
+pub unsafe fn unsafecell() {
+    // No lint
+    let c = UnsafeCell::new(String::new());
+    unsafe {
+        *c.get() = String::new();
+    }
+}
+
+fn main() {}

tests/ui/raw_assign_to_drop.stderr

@@ -1,63 +1,63 @@
 error: assignment via raw pointer always executes destructor
-  --> tests/ui/raw_assign_to_drop.rs:5:10
+  --> tests/ui/raw_assign_to_drop.rs:8:10
    |
 LL |         *r = "foo".to_owned();
-   |          ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |          ^ the value may be uninitialized, causing Undefined Behavior when the destructor executes
    |
    = note: the destructor defined by `std::string::String` is executed during assignment of the new value
-   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::write()` to overwrite a possibly uninitialized place
    = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
    = note: `-D clippy::raw-assign-to-drop` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::raw_assign_to_drop)]`
 
 error: assignment via raw pointer always executes destructor
-  --> tests/ui/raw_assign_to_drop.rs:10:11
+  --> tests/ui/raw_assign_to_drop.rs:13:11
    |
 LL |         (*r, *r) = ("foo".to_owned(), "bar".to_owned());
-   |           ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |           ^ the value may be uninitialized, causing Undefined Behavior when the destructor executes
    |
    = note: the destructor defined by `std::string::String` is executed during assignment of the new value
-   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::write()` to overwrite a possibly uninitialized place
    = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
 
 error: assignment via raw pointer always executes destructor
-  --> tests/ui/raw_assign_to_drop.rs:10:15
+  --> tests/ui/raw_assign_to_drop.rs:13:15
    |
 LL |         (*r, *r) = ("foo".to_owned(), "bar".to_owned());
-   |               ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |               ^ the value may be uninitialized, causing Undefined Behavior when the destructor executes
    |
    = note: the destructor defined by `std::string::String` is executed during assignment of the new value
-   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::write()` to overwrite a possibly uninitialized place
    = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
 
 error: assignment via raw pointer always executes destructor
-  --> tests/ui/raw_assign_to_drop.rs:12:11
+  --> tests/ui/raw_assign_to_drop.rs:15:11
    |
 LL |         (*r, *i) = ("foo".to_owned(), 47);
-   |           ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |           ^ the value may be uninitialized, causing Undefined Behavior when the destructor executes
    |
    = note: the destructor defined by `std::string::String` is executed during assignment of the new value
-   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::write()` to overwrite a possibly uninitialized place
    = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
 
 error: assignment via raw pointer always executes destructor
-  --> tests/ui/raw_assign_to_drop.rs:15:10
+  --> tests/ui/raw_assign_to_drop.rs:18:10
    |
 LL |         *(&mut x as *mut _) = "Foo".to_owned();
-   |          ^^^^^^^^^^^^^^^^^^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |          ^^^^^^^^^^^^^^^^^^ the value may be uninitialized, causing Undefined Behavior when the destructor executes
    |
    = note: the destructor defined by `std::string::String` is executed during assignment of the new value
-   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::write()` to overwrite a possibly uninitialized place
    = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
 
 error: assignment via raw pointer always executes destructor
-  --> tests/ui/raw_assign_to_drop.rs:21:10
+  --> tests/ui/raw_assign_to_drop.rs:24:10
    |
 LL |         *v.as_mut_ptr() = Default::default();
-   |          ^^^^^^^^^^^^^^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |          ^^^^^^^^^^^^^^ the value may be uninitialized, causing Undefined Behavior when the destructor executes
    |
    = note: the destructor defined by `std::string::String` is executed during assignment of the new value
-   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::write()` to overwrite a possibly uninitialized place
    = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
 
 error: aborting due to 6 previous errors