Skip to content
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

Introduce trait-driven arithmetic #191

Closed
wants to merge 3 commits into from
Closed

Conversation

workingjubilee
Copy link
Member

This allows collapsing 5~10 instances of a function on the Simd type
into 1-3 copies, at least from the perspective of the docs.
The result is far more legible to a user.

Left deliberately unfinished to offer a taste of the difference
and allow the comparative angles to be discussed.

@workingjubilee workingjubilee force-pushed the trait-arith branch 2 times, most recently from 1d07dac to f8c373a Compare November 14, 2021 20:17
@jyn514
Copy link
Member

jyn514 commented Nov 15, 2021

@workingjubilee FYI the rustdoc team has considered dividing up the sidebar by impl blocks, with some summary of the impl bounds between each, which might be a nicer way to solve this problem without making the API more complicated in order to improve the docs.

@calebzulawski
Copy link
Member

@jyn514 would that perhaps cover the many duplicate entries in search, too? Fixing it in rustdoc, rather than an API change, might be my preference.

@jyn514
Copy link
Member

jyn514 commented Nov 15, 2021

@calebzulawski I haven't thought about search. Can you open an issue on rust-lang/rust?

@calebzulawski
Copy link
Member

Opened this issue: rust-lang/rust#90929

@workingjubilee
Copy link
Member Author

IMO this shouldn't complicate the API much (this rough pass does a little, but I think a better version would be somewhat simpler, this is basically just a proof of concept), but I am definitely open to comparing alternatives here.

@jyn514
Copy link
Member

jyn514 commented Nov 15, 2021

@workingjubilee gotcha - this may make sense to merge in the meantime then, I don't know how long it will take to improve this on rustdoc's side.

@workingjubilee
Copy link
Member Author

Then given that the docs right now are actively crashing user's browsers, this gets to move to the head of the line.

@calebzulawski
Copy link
Member

Regular rustdocs for std can crash the Android browser, which is what I'm guessing what's going on. I've had it happen for various crates, too...

@workingjubilee
Copy link
Member Author

Yes, and there has also been work on minimizing that from the rustdoc team. My phone can usually open Iterator now.

This allows collapsing 5~10 instances of a function on the Simd type
into 1-3 copies, at least from the perspective of the docs.
The result is far more legible to a user.
@workingjubilee
Copy link
Member Author

This does not eliminate the problems (there is also the matter of the explosive number of trait implementations!) but it makes things palpably lighter in terms of method documentations.

from_ne_bytes and to_ne_bytes deserve their own PR for various reasons.

@workingjubilee workingjubilee marked this pull request as ready for review November 17, 2021 03:27
@workingjubilee
Copy link
Member Author

This PR knocks off about 500kb for the primary Simd type's page.

@calebzulawski
Copy link
Member

I'm still confident that, if we go this route, the traits should be broken up entirely by function or function category, and not Int etc. I also think the traits should be implemented on the scalar, not the vector.

@workingjubilee
Copy link
Member Author

workingjubilee commented Nov 17, 2021

I also think the traits should be implemented on the scalar, not the vector.

That unfortunately doesn't really work when the function needs to be differently dispatched based on the trait implementation. None of the examples you cited in comparisons.rs require dispatching like that, which is why I didn't always do that.

@calebzulawski
Copy link
Member

I'm confused--you did some of them that way, so why not all?

@workingjubilee
Copy link
Member Author

The ones I could do that way, I did,
and the ones that I could not do that way, I did not.

@workingjubilee
Copy link
Member Author

Closing this in favor of carving this up into other discussions like in #198.

@calebzulawski calebzulawski deleted the trait-arith branch October 17, 2022 00:20
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