Skip to content

perf: replace unnecessary usize::try_from with as casts#1473

Open
Nicolas0315 wants to merge 1 commit intomemorysafety:mainfrom
Nicolas0315:fix/replace-try-from-usize-with-as-casts
Open

perf: replace unnecessary usize::try_from with as casts#1473
Nicolas0315 wants to merge 1 commit intomemorysafety:mainfrom
Nicolas0315:fix/replace-try-from-usize-with-as-casts

Conversation

@Nicolas0315
Copy link

@Nicolas0315 Nicolas0315 commented Mar 1, 2026

Replace usize::try_from(n).unwrap() with n as usize in three locations where the checked conversion is unnecessary:

  • fn read_pal_indices (decode.rs:643): Block dimensions (w4, h4, bw4, bh4) are always non-negative per the AV1 spec.

  • Segmentation map update (decode.rs:3062-3063): The original comment about needing checked casts for from_raw_parts_mut is stale — the code now uses bounds-checked DisjointMut indexing, so overflow in the cast would cause a bounds-check panic rather than UB. The outdated comment is removed.

  • fn cfl_ac_rust (ipred.rs:1310): w_pad and h_pad are non-negative and immediately asserted to be less than width/height.

This removes the overhead of the checked conversion (comparison + unwrap/panic path) on every call.

Replace `usize::try_from(n).unwrap()` with `n as usize` in three
locations:

- `fn read_pal_indices` in decode.rs: block dimensions (w4, h4, bw4,
  bh4) are always non-negative per the AV1 spec.

- Segmentation map update in decode.rs: the comment about needing
  checked casts for `from_raw_parts_mut` is stale — the code now uses
  bounds-checked `DisjointMut` indexing, so overflow in the cast would
  cause a bounds-check panic rather than UB. Remove the outdated comment.

- `fn cfl_ac_rust` in ipred.rs: w_pad and h_pad are non-negative and
  immediately asserted to be less than width/height.

This removes the overhead of the checked conversion (comparison +
unwrap/panic path) on every call. Addresses memorysafety#1344.
@kkysen kkysen self-requested a review March 2, 2026 03:47
Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Could you leave inline comments saying that they're non-negative like you have in the PR description?

Also, we should probably update these variables/arguments/fields to be actually unsigned (e.g., u32), but not sure if we want to do that right here (feel free to do that if you'd like, though).

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