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

Add std::ops for vector-scalars #381

Merged
merged 1 commit into from
Mar 19, 2018
Merged

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Mar 18, 2018

Today some of the traits are implemented for vectors, and some of them are implemented for scalars:

let u = i32x4::new(1, 2, 3, 4);
let v = u << 1; // OK
let w = 1 << v; // ERROR
let t = v << v; // ERROR
let u= f32x4::new(1., 2., 3., 4.);
let v = u+ u;  // OK
let w = 1_f32 + v; // ERROR
let t = v + 1_f32; // ERROR

This PR fixes these errors for all operators. The implementation is compartmentalized so that we don't need to stabilize all of these at once. For example, we can stabilize (vector, vector) -> vector binary ops first, and decide on (vector, scalar) -> vector and (scalar, vector) -> vector later.

It is important to be comprehensive with the std::ops traits because due to trait coherence users cannot implement them themselves on the portable vector types. Having these available on nightly should help with experimentation.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Mar 19, 2018

Failures are spurious.

@alexcrichton
Copy link
Member

Thanks!

I'm personally somewhat wary of this though in the sense that I think the hidden splat functions in the impls here can be surprising at times. For example I think it's ambiguous whether the intent of u8x32 + u8 is "sum everything up" or "splat the element and do a vector add"

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Mar 19, 2018

Yeah, I don't know either. For the RFC I would prefer to just provide "vector" operators, and if somebody wants to do vec << 1 they would just need to write vec << splat(1) and that's it. In any case, that requires implementing the vector ops for the shifts, which this PR does.

I've separated the interfaces of both vector and vector/scalar implementations into different files and behind different macros, so that before stabilization we either remove the ones that aren't necessary or put them behind an unstable feature.

@alexcrichton
Copy link
Member

Ok! Sounds like it's all cleanly separated and otherwise easy-ish to manage, so let's go ahead and merge

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.

None yet

2 participants