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

Support for const types #76

Closed
ApoorvaJ opened this issue Sep 4, 2020 · 19 comments
Closed

Support for const types #76

ApoorvaJ opened this issue Sep 4, 2020 · 19 comments
Labels
enhancement New feature or request

Comments

@ApoorvaJ
Copy link

ApoorvaJ commented Sep 4, 2020

Currently, constant types don't seem to be supported:

error[E0015]: calls in constants are limited to constant functions, tuple structs and tuple variants
  --> src/main.rs:
   |
17 | const INIT_POS: Vec3 = vec3(1.0, 1.0, 1.0);
   |                        ^^^^^^^^^^^^^^^^^^^

I can't construct the tuple manually either, since there is only crate-level visibility. Would it be possible to implement this?

@bitshifter
Copy link
Owner

I can't do this across the board because the intrinsics that glam uses are not const fn. I'm a bit reluctant to do it to some types and not others because it would be inconsistent which could be confusing. As soon as intrinsics get const fn I would be keen to use it.

In the interim you could use an array or tuple are truly in the into/from implementations.

@bitshifter
Copy link
Owner

I guess I could do some union hacks to avoid calling the intrinsic functions. Last time I played around with doing that it was a bit of a perf hit so it might not pan out, should be possible though.

@ApoorvaJ
Copy link
Author

ApoorvaJ commented Sep 4, 2020

It's definitely not a pressing issue. I can easily work around it. I really like glam for its simplicity and uniformity. Just wanted to check whether this was an oversight or a limitation. :) Feel free to close this issue as you see fit.

@bitshifter
Copy link
Owner

No worries, it's something I would like to address. Will leave this open and try out the union cast thing some time.

@bitshifter
Copy link
Owner

I tried this out but unfortunately performing union casts and other unsafe casts is not allowed in const fn, tracking issue is here rust-lang/rust#51909.

It might be a while until Rust gets that feature but I'll leave this open for now anyway.

@FSMaxB
Copy link

FSMaxB commented Sep 12, 2020

Rather than making operations const fn, would it be possible to make just the constructors const fn?

@bitshifter
Copy link
Owner

That is what I was attempting to do. The simd types like __m128 are opaque and you need to call functions like _mm_set_ps to initialise them. Those functions are not const. I don't know if there's any reason those std lib intrinsic functions couldn't be made const though.

@FSMaxB
Copy link

FSMaxB commented Sep 12, 2020

That is what I was attempting to do.

Oh, ok.

@bitshifter
Copy link
Owner

I raised an issue with stdarch about this. Let's see where it goes. rust-lang/stdarch#906

@bitshifter
Copy link
Owner

It looks like it should be possible to make the constructors in std::arch const fn. That might take a while do though.

In the interim I've added a bunch of macros that can be used to create const values of all glam types:
const_mat2, const_mat3, const_mat4, const_quat, const_vec2, const_vec3, const_vec3a and const_vec4.

Internally these perform a union cast which is allowed for const values but is not allowed in a const fn in stable Rust.

This is just in git for now, at some point I'll make a new release to crates.io. I still need to write some docs and make some other changes. Let me know if you try them out.

@alice-i-cecile
Copy link

I would also be grateful to see some more utility functions for vectors made const where possible, such as .normalize <3

@bitshifter
Copy link
Owner

bitshifter commented May 4, 2021

@alice-i-cecile I don't believe that's possible right now either. Floating point arithmetic is not allowed in a const fn context in stable Rust. See rust-lang/rust#57241. I guess if that issue is resolved, glam would also need most SSE2 functions in std::arch to also be made const fn. It could happen eventually but it might be a while.

@alice-i-cecile
Copy link

Darn, that's fair enough. I'll keep my eyes on that issue too then.

@SUPERCILEX
Copy link

Is it possible to make just the ::new functions const so the macros aren't needed?

@bitshifter
Copy link
Owner

Not at the moment, see #76 (comment)

@SUPERCILEX
Copy link

::new shouldn't be doing arithmetic though, right?

@bitshifter
Copy link
Owner

bitshifter commented Apr 26, 2022

It does not, as the comment above says, the SSE init method is not const fun. This needs to be addressed in the std library

@SUPERCILEX
Copy link

Ohhhhh, I see, thanks.

@bitshifter
Copy link
Owner

@SkiFire13 pointed out unions can be used in a const eval context now so this issue is finally resolved as part of #294.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants