Skip to content

vello_cpu: Fix clippy::cast_possible_wrap in Gaussian blur#1364

Merged
tomcur merged 2 commits intolinebender:mainfrom
tomcur:push-tqsskulszrys
Jan 19, 2026
Merged

vello_cpu: Fix clippy::cast_possible_wrap in Gaussian blur#1364
tomcur merged 2 commits intolinebender:mainfrom
tomcur:push-tqsskulszrys

Conversation

@tomcur
Copy link
Copy Markdown
Member

@tomcur tomcur commented Jan 15, 2026

Fixed by adding a reasonable limitation on MAX_KERNEL_SIZE, and storing the kernel size as a u8 instead of usize.

The lint itself checks for possible wrapping when going from an unsigned to signed integer, and is not currently in our lint set (hence not triggered on CI). It triggers when manually requested.

It probably should be included in the lint set, but that's for another time (linebender/linebender.github.io#135).

Fixed by adding a reasonable limitation on `MAX_KERNEL_SIZE` (checked at
compile time), and storing the kernel size as a `u8` instead of `usize`.

The lint itself checks for possible wrapping when going from an unsigned
to signed integer, and is not currently in our lint set (hence not
triggered on CI). It triggers when manually requested.

It probably should be included in the lint set, along with
`clippy::cast_sign_loss` (going from signed to unsigned), but that's for
another time.
@tomcur tomcur requested a review from grebmeg January 15, 2026 13:25
Comment on lines +38 to +47
#[cfg(test)]
const _: () = const {
if MAX_KERNEL_SIZE.is_multiple_of(2) {
panic!("`MAX_KERNEL_SIZE` must be odd");
}
if MAX_KERNEL_SIZE > u8::MAX as usize {
panic!("`MAX_KERNEL_SIZE` must be less than or equal to `u8::MAX`");
}
};

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure we don't accidentally break our own promise later!

tomcur added a commit to tomcur/linebender.github.io that referenced this pull request Jan 15, 2026
`clippy::cast_possible_wrap` checks for casting an unsigned integer to a
signed integer of the same size. If the value is too large to fit in the
signed integer, it will wrap around to negative numbers. Such casts will
usually be used when doing some math, so scrutiny of the cast is
probably welcome. It's similar to the already-enabled
`clippy::cast_possible_truncation`.

See linebender/vello#1364 for an example where
the lint is useful.

The similar lint `clippy::cast_sign_loss` more or less does the
opposite, triggering for casts from signed to unsigned numeric types.
Floats are included in this lint, as they do a saturating cast, making
it more likely to have false positives: it triggers 65 times on
`vello_common` currently. Perhaps this one should not be added to the
canonical lint set.
tomcur added a commit to linebender/linebender.github.io that referenced this pull request Jan 16, 2026
`clippy::cast_possible_wrap` checks for casting an unsigned integer to a
signed integer of the same size. If the value is too large to fit in the
signed integer, it will wrap around to negative numbers. Such casts will
usually be used when doing some math, so scrutiny of the cast is
probably welcome. It's similar to the already-enabled
`clippy::cast_possible_truncation`.

See linebender/vello#1364 for an example where
the lint is useful.

The similar lint `clippy::cast_sign_loss` more or less does the
opposite, triggering for casts from signed to unsigned numeric types.
Floats are included in this lint, as they do a saturating cast, making
it more likely to have false positives: it triggers 65 times on
`vello_common` currently. Perhaps this one should not be added to the
canonical lint set (and I have not proposed adding it here).

For the interested reader,
rust-lang/rust-clippy#9231 has some further
discussion of these lints.
@tomcur tomcur added this pull request to the merge queue Jan 19, 2026
Merged via the queue into linebender:main with commit 9ab8513 Jan 19, 2026
17 checks passed
@tomcur tomcur deleted the push-tqsskulszrys branch January 19, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants