Skip to content

Conversation

@dhardy
Copy link
Member

@dhardy dhardy commented Jun 7, 2021

Closes #1128. Review @vks, @abonander.

Thoughts? Not quite sure myself; it may be more natural to have rng.gen_string(Alphanumeric, 20).

@vks
Copy link
Collaborator

vks commented Jun 7, 2021

Looks good to me. We might want to generalize this to allow bulk generation of vectors. Such an API might benefit more from SIMD than our current one.

@dhardy
Copy link
Member Author

dhardy commented Jun 8, 2021

We might want to generalize this to allow bulk generation of vectors.

What did you have in mind? A String is not a Vec<u8>.

My understanding is that the unique challenge here is that asserting a bunch of bytes is a valid UTF-8 String is unsafe or requires a (redundant) test.

@vks
Copy link
Collaborator

vks commented Jun 8, 2021

I guess I wanted to unify this with the Fill API. Anyway, this is something that can be done later.

@dhardy
Copy link
Member Author

dhardy commented Jun 8, 2021

You mean like try_fill_with_length(&mut self, rng: &mut R, len: usize) ?

How would len be interpreted in the case of a String? It usually refers to the byte-length.

@vks
Copy link
Collaborator

vks commented Jun 8, 2021

I think we would still need a different trait for strings, for the reason you mentioned, I just think the interfaces could be more uniform.

@dhardy
Copy link
Member Author

dhardy commented Jun 8, 2021

It's already possible to do something like this:

let mut v: Vec<f32> = iter::repeat(f32::NAN).take(20);
use rand::Fill;
v[..].try_fill(&mut rand::thread_rng())?;

That still requires initialisation but skipping that is #1080 ... we're way off topic.


Anyway, we can wait a couple of days to give @abonander and @qoh a chance to review this if they're interested. Then it's time to review the change-log for the next patch release.

@abonander
Copy link

@dhardy This looks fine to me but I thought you preferred to keep the interface simple?

@dhardy
Copy link
Member Author

dhardy commented Jun 10, 2021

It's one trait plus two impls, so reasonably simple.

@TheIronBorn
Copy link
Contributor

Looks good to me. We might want to generalize this to allow bulk generation of vectors. Such an API might benefit more from SIMD than our current one.

I'm working on a SIMD alphanumeric append_string if that covers this.

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.

Implement Alphanumeric::gen_string

6 participants