Run clippy in CI

[djc]

I tried running clippy and got this:

error: unsafe impl missing a safety comment
   --> src/internal.rs:308:1
    |
308 | unsafe impl Send for TaskThreadDataDelayedFg {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
note: the lint level is defined here
   --> lib.rs:9:9
    |
9   | #![deny(clippy::undocumented_unsafe_blocks)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: unsafe impl missing a safety comment
   --> src/internal.rs:310:1
    |
310 | unsafe impl Sync for TaskThreadDataDelayedFg {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks

error: unsafe impl missing a safety comment
   --> src/internal.rs:440:1
    |
440 | unsafe impl Send for Rav1dContext {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks

error: unsafe impl missing a safety comment
   --> src/internal.rs:442:1
    |
442 | unsafe impl Sync for Rav1dContext {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks

clippy is configured like this:

#![allow(clippy::all)]
#![deny(clippy::undocumented_unsafe_blocks)]
#![deny(clippy::missing_safety_doc)]

It would be nice if these explicitly enabled clippy lints at least blocked PR CI.

• Closes Run clippy in CI #1343

[kkysen]

#1329 was supposed to fix the warnings, but we didn't have time to fix the issues with #1327 and then it languished.

[djc]

#1329 was supposed to fix the warnings, but we didn't have time to fix the issues with #1327 and then it languished.

Okay... this seems pretty important. Do you think we can get it fixed?

[kkysen]

Okay... this seems pretty important. Do you think we can get it fixed?

Yes, we should. I need to figure out the soundness issues with #1327 and Unique, although I can also merge that largely as is so that this PR gets unblocked, and leave an issue open to fix the Unique issues. The current unsafe impls aren't really any more sound than how I'm using Unique in #1327.

[djc]

Okay... this seems pretty important. Do you think we can get it fixed?

Yes, we should. I need to figure out the soundness issues with #1327 and Unique, although I can also merge that largely as is so that this PR gets unblocked, and leave an issue open to fix the Unique issues. The current unsafe impls aren't really any more sound than how I'm using Unique in #1327.

It's not that obvious to me how/why Unique is practically an improvement over the current situation, other than that it technically avoids these unsafe impl markers.

[kkysen]

It's not that obvious to me how/why Unique is practically an improvement over the current situation, other than that it technically avoids these unsafe impl markers.

That's more right than I thought. Feel free to add temporary // SAFETY: TODO ... comments to suppress the clippy warning for now so that we can run clippy in CI.

[kkysen]

We might be able to trim some of these CI jobs (I think there's some overlap), but it seems good for now.

[djc]

Dropped running clippy in the Android workflow, which seems to fail. It looks like there's not a lot of Android-specific code anyway, so I think it should be fine to land this -- can always expand with more coverage.

[kkysen]

Dropped running clippy in the Android workflow, which seems to fail. It looks like there's not a lot of Android-specific code anyway, so I think it should be fine to land this -- can always expand with more coverage.

Oh, it might be that the android target doesn't have clippy, since it's a tier 2 target without host tools.