slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
17.54k stars 600 forks source link

Clippy reports `drop_ref` on skia renderer #2725

Closed Berrysoft closed 1 year ago

Berrysoft commented 1 year ago

When clippy reports an error, it's usually some code that works unexpectedly or doesn't work. Just confused, because I don't understand what's this line for: https://github.com/slint-ui/slint/blob/b1073b2ee37f6c807388373526ee486f2d44a351/internal/renderers/skia/d3d_surface.rs#L165

The error message of clippy:

error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing
   --> internal\renderers\skia\d3d_surface.rs:165:9
    |
165 |         drop(surface);
    |         ^^^^^^^^^^^^^
    |
note: argument has type `&mut skia_safe::RCHandle<skia_bindings::SkSurface>`
   --> internal\renderers\skia\d3d_surface.rs:165:14
    |
165 |         drop(surface);
    |              ^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref
    = note: `#[deny(clippy::drop_ref)]` on by default

By the way, clippy reports tons of warnings on slint code:) I usually replaces cargo check by cargo clippy in VS Code, so that I can get suggestions frequently, and that's why I see the errors reported by clippy.

tronical commented 1 year ago

Well spotted, thanks for reporting. That's indeed not necessary - likely a leftover when it wasn't a reference. I'll push a fix quickly :)

ogoffart commented 1 year ago

By the way, clippy reports tons of warnings on slint code:) I usually replaces cargo check by cargo clippy in VS Code, so that I can get suggestions frequently, and that's why I see the errors reported by clippy.

Yes, we haven't been using clippy much so far. See also https://github.com/slint-ui/slint/issues/1874