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

made vec conversions const #464

Closed
wants to merge 1 commit into from

Conversation

sanbox-irl
Copy link
Contributor

Added const to the as_ conversions for Vec. This might also be appropriate in other places, but this must is what I had time and familiarity enough for!

@sanbox-irl
Copy link
Contributor Author

hmm the errors the CI is getting i am not getting on my local!

@bitshifter
Copy link
Owner

Are you on an arm chip? Platforms that use SIMD like x86_64 use deref coercion to access data members, this is not const fn.

@sanbox-irl
Copy link
Contributor Author

I am indeed! I targeted amd64 intel for macos and can see the errors locally now -- I suspect that the utility of this PR is not worth the amount of maintenance burden it would introduce since we'd have to branch in the tera scripts on sse2 for just the const. Is there a way to do basically do:

pub NO_SSE2_CONST fn /* everything else */

is such a way that's easy to maintain?

@bitshifter
Copy link
Owner

Hey, so as a rule I try to make the API consistent no matter what architecture you are building for. Otherwise, say in this example if you were using the const fn as_vec4 in a const context your code might compile on arm but not x86, which is not ideal. Given that, I'm going to close this PR.

@bitshifter bitshifter closed this Jan 23, 2024
@sanbox-irl
Copy link
Contributor Author

Checks out to me! Thanks for addressing this quickly :) -- here's hoping we get const derefs soon

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