Skip to content

Generically implement fn horizontal_{min,max} #198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

workingjubilee
Copy link
Member

This collapses all instances of these two functions by merely depending on T being SimdElement + PartialOrd.

@workingjubilee
Copy link
Member Author

This only saves 18kb, but it makes the docs much easier to navigate, so.

@workingjubilee workingjubilee changed the title Generically implement horizontal_{min,max} Generically implement fn horizontal_{min,max} Nov 24, 2021
Copy link
Member

@programmerjake programmerjake left a comment

Choose a reason for hiding this comment

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

lgtm

@programmerjake programmerjake mentioned this pull request Nov 24, 2021
@calebzulawski
Copy link
Member

I mentioned this in zulip, I don't think PartialOrd is the correct bound, we probably want our own trait that specifically means that these functions monomorphize.

@calebzulawski
Copy link
Member

A trivial example--pointers implement PartialOrd but do we want them to implement these functions? We might, might not, and I have no idea if these functions would monomorphize.

@workingjubilee
Copy link
Member Author

That is not an actual concern in any of our exposed interfaces and was not a blocking concern for introducing the prior versions.

@workingjubilee workingjubilee force-pushed the generic-horizontal-minmax branch from 7e48c16 to 75613bc Compare December 19, 2021 23:16
@workingjubilee
Copy link
Member Author

Consensus coalesced around a different direction than here.

@workingjubilee workingjubilee deleted the generic-horizontal-minmax branch January 26, 2022 23:43
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.

3 participants