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

Make portable implementation const #439

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nazar-pc
Copy link

@nazar-pc nazar-pc commented Jan 5, 2025

I was going to use BLAKE3 in const environment, but it doesn't support such use case yet.

This relatively small PR refactors portable implementation to make it work in const environment.

There is a bit of trivially verifiable unsafe due to nicer APIs not yet available in const environment, but these all work on stable.

arrayref were hard to read and used slicing that is not compatible with const, so I replaced it with a couple simple macros where it was necessary and left other usages as is.

Copy link

@devinrsmith devinrsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I'm not a maintainer of this library, but I do like to keep up to date with some of the going-ons.

src/platform.rs Outdated Show resolved Hide resolved
@@ -120,7 +119,7 @@ pub fn compress_xof(
crate::platform::le_bytes_from_words_64(&state)
}

pub fn hash1<const N: usize>(
const fn hash1<const N: usize>(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removal of pub intended?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, but since there are a few other backends with the same function and in all cases it is internal implementation without pub (here too), I decided to remove it for consistency too. Happy to restore if maintainers have strong opinion on this one.

Copy link
Author

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for review, appreciate it!

src/platform.rs Outdated Show resolved Hide resolved
@@ -120,7 +119,7 @@ pub fn compress_xof(
crate::platform::le_bytes_from_words_64(&state)
}

pub fn hash1<const N: usize>(
const fn hash1<const N: usize>(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, but since there are a few other backends with the same function and in all cases it is internal implementation without pub (here too), I decided to remove it for consistency too. Happy to restore if maintainers have strong opinion on this one.

@nazar-pc
Copy link
Author

@oconnor663 it'd be great to get your attention here, especially since the changes are fairly simple

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