Skip to content

Add TryFrom bounds for all integer primitive types to SimdIntElement - #335

Merged
Shnatsel merged 2 commits into
linebender:mainfrom
danderson:push-rtsumvyuzsyn
Aug 26, 2026
Merged

Add TryFrom bounds for all integer primitive types to SimdIntElement#335
Shnatsel merged 2 commits into
linebender:mainfrom
danderson:push-rtsumvyuzsyn

Conversation

@danderson

Copy link
Copy Markdown
Contributor

I skipped these in #302 because conditional conversion didn't seem great for typically performance sensitive simd code. However, it turns out that these conversions are very useful in tests especially, and even in regular code LLVM can often rule out failure and elide the branch.

Still no From impls other than bool, because that's the lowest common denominator across signed/unsigned and all the different sizes.

No change to SimdFloatElement: unfortunately they don't implement TryFrom for integers, so without num-traits's cast trait (which provides the equivalent of as conversion) we're stuck with the existing lowest common denominator.

@danderson

Copy link
Copy Markdown
Contributor Author

Also added extra bounds so that TryFrom::Error must be Debug, so that unwrap and expect work without further bounds in the caller.

@danderson
danderson marked this pull request as draft August 16, 2026 19:19
@danderson

Copy link
Copy Markdown
Contributor Author

Hmm, the extra debug bounds cause failures that I don't get on my machine. Converting to draft while I figure it out.

@danderson

Copy link
Copy Markdown
Contributor Author

Okay, fixed. The TryFrom impls all use either a core::num::TryFromIntError or Infallible as their error type, so I made TryFrom::Error's the useful traits both of those implement (Error implies Debug and Display).

@danderson
danderson marked this pull request as ready for review August 16, 2026 19:39
Signed-off-by: David Anderson <dave@natulte.net>

@Shnatsel Shnatsel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry it took a while to properly review. I was concerned about different concrete error types being problematic somehow, like messing with downstream impl Trait or being hard to store in a struct, but I cannot find any fault with this. Thanks!

@Shnatsel
Shnatsel added this pull request to the merge queue Aug 26, 2026
Merged via the queue into linebender:main with commit 4cf57d8 Aug 26, 2026
22 checks passed
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