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

feat: Add unique const random #19

Closed
wants to merge 1 commit into from
Closed

Conversation

AlseinX
Copy link

@AlseinX AlseinX commented Mar 8, 2021

The original implementation does not guarantee the generated number to be unique. This may cause problem when using it directly as a compile-time generated identifier.
So I added a unique version to make sure when a duplicate number is generated, it would simply re-generate it, until all possible values have been taken.

Copy link
Contributor

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

This is surprising to me -- in what case would you need a generated identifier that is both unique and random? I can understand needing unique identifiers, and separately needing random identifiers. Can you provide more information on your use case that requires both?

If you only require unique identifiers, a sequential count seems like it would be more appropriate for your use case. Is your reason to build it into this library just because some of the implementation is similar (like parsing the macro arguments)? If so, my preference would be not to extend this library in this way.

@AlseinX
Copy link
Author

AlseinX commented Mar 9, 2021

As for cases that require unique identifier, unless the identified items themselves are sequential, it would be better to use random identifier rather than sequential, otherwise some errors and vulnerabilities on identifier would be more likely to be soundless, because a valid id ± 1 would also be a valid id and sometimes they are relative.
There are certain to be many other reasons and examples that random identifier is choosed over sequential, and I believe that this feature is useful for many cases.

@tkaitchuck
Copy link
Owner

This might make sense for types like U64. But with u8 for example, I can easily imagine a case where due to dependencies more than 256 values get used. Then this creates the awkward situation where crates that build on their own don't build together (and in this case cause the build to go into an infinite loop).

I think the unique + random for smaller types like this it might be better to have tuple of (unique, random) which is used instead.

@tkaitchuck tkaitchuck closed this Mar 3, 2024
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.

None yet

3 participants