`// CAST: ...` comment on `as` casts

[ojeda]

What it does

Similar to // PANIC: ... and // SAFETY: ... comments, but for as casts.

In the Linux kernel, we try to avoid type cast expressions, i.e. the as operator, which are quite powerful, and instead aim to use restricted variants that are less powerful to avoid mistakes and make intent more clear. We already enable other Clippy lints to spot such cases, e.g. we enabled for 6.17 a set of those: ptr_as_ptr, ptr_cast_constness, as_ptr_cast_mut, as_underscore, cast_lossless and ref_as_ptr.

Thus it would be nice to require a developer to write an explanation for the remaining cases, just like they need to do so for unsafe blocks with // SAFETY: ..., or for certain panicking operations with // PANIC: ... (#15895).

That is, we would like code to look like:

// CAST: ...
value as usize

In other words, like undocumented_unsafe_blocks, but for as casts. And like as_conversions, but only triggering if the comment is not provided.

In addition, there would be a dual lint to detect unnecessary comments: #15964.

Note on pointer casts: currently in the kernel we sometimes use // CAST: ... for pointer casts via .cast() too, since those are dangerous too (even if already more restricted than as). We may want to extend this lint (or ideally have a separate one) to cover those too. This is particularly important for the dual lint.

Cc: @blyxyas @hcbarker

Advantage

The advantages are very similar to the ones we have seen from applying the // SAFETY: ... convention:

• Type cast expressions get documented better, i.e. the reason they are needed must be explained.

• It gives some pause to developers when introducing a type cast expression -- in many cases, there are better alternatives they should be using instead.

These advantages also mean review time (and thus maintainers' workload) gets reduced, since developers will more often realize an as cast should not have been used before submitting the code (or, if it was truly needed, then the reason will help others understand why without having to ask about it).

Drawbacks

No response

Example

value as usize

Should be written as:

// CAST: ...
value as usize

Comparison with existing lints

This is similar to as_conversions, but it would only lint if there is no // CAST: ... comment justifying the conversion.

One could require allow or expect for this instead -- please see the rationale for using a comment instead on #15895:

There is of course #[allow(..., reason = "...")] nowadays as well, but ideally we would want it as a comment since we already have other similar "tags", e.g. SAFETY, INVARIANT, CAST..., which would make it more consistent, and it would not require the "burden" of mentioning the lint name, deciding whether to allow or expect, writing the usual attribute syntax, and so on.

Or, seen from another perspective: these // PANIC: comments are not so much about allowing a false positive of a lint, but rather about writing a required comment. That also allows to keep allows for this lint reserved for the case where the lint somehow fires but it wasn't supposed to fire.

Additional Context

Please see the "Additional context" for // PANIC: ... on #15895.