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

Use const generics. #1

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Use const generics. #1

merged 2 commits into from
Nov 6, 2023

Conversation

ahmedcharles
Copy link
Contributor

This doesn't remove the fields, it uses them to verify that all of the values are right through debug_asserts. I have more PRs that take this approach further.

The only issue with this is that it doesn't work with the trait, because Self::Target isn't the same time for each of the various functions.

Anyways, is this a direction that you're open to? Thanks.

@sunsided
Copy link
Owner

Absolutely, yes. The reason I didn't go for it yet is because of the limited compile-time arithmetic support for const generics.

@sunsided sunsided added the enhancement New feature or request label Oct 16, 2023
@ahmedcharles
Copy link
Contributor Author

Are you ok with the PR as is, or would you prefer some changes? Thanks.

@ahmedcharles
Copy link
Contributor Author

ping?

@sunsided
Copy link
Owner

sunsided commented Nov 6, 2023

Heya, apologies for the delay. Didn't have the time to get into it in depth yet.

@sunsided
Copy link
Owner

sunsided commented Nov 6, 2023

@ahmedcharles Thanks for the implementation! I adjusted it a bit to get rid of the redundant row/column counts. Will merge in a bit.

@sunsided sunsided merged commit ad6829e into sunsided:main Nov 6, 2023
1 check passed
@ahmedcharles
Copy link
Contributor Author

I should've mentioned that I had a commit to remove those parameters, I didn't submit it because I wanted it to be obvious that the asserts were correct. :)

@ahmedcharles
Copy link
Contributor Author

Thanks.

@ahmedcharles ahmedcharles deleted the const branch November 7, 2023 06:14
@sunsided sunsided self-assigned this Nov 7, 2023
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

Successfully merging this pull request may close these issues.

2 participants