-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add Random Span-based APIs #22804
Comments
@stephentoub Should the existing |
We shouldn't change the existing |
I'm curious, why is it written as |
It could be 😄 |
😄 I was curious what NextBytes(byte[]) was doing... now I wish I hadn't looked... public virtual void NextBytes(byte[] buffer)
{
if (buffer == null) throw new ArgumentNullException(nameof(buffer));
Contract.EndContractBlock();
for (int i = 0; i < buffer.Length; i++)
{
buffer[i] = (byte)(InternalSample() % (Byte.MaxValue + 1));
}
} |
I basically just copied NextBytes for my example (changing the But, yeah, that's unnecessarily complex. And because the JIT doesn't know that InternalSample() returns a non-negative value, the resulting asm is probably going to be worse than it needs to be. I think it's unlikely to actually matter though. |
Fixed by dotnet/coreclr#13708 and dotnet/corefx#23692 |
Separated out of https://github.com/dotnet/corefx/issues/21281 for tracking purposes.
Implementation should be in terms of
Next()
. TheNextBytes(byte[])
base method is implemented in terms ofInternalSample
, butNextBytes(Span<byte>)
shouldn't be implemented in terms of that, as it's not public and thus an existing derived type couldn't have overridden it, and thus the new virtual wouldn't pick up existing behaviors. It also shouldn't be in terms ofNextBytes(byte[])
, as that would require allocating/copying an array, largely defeating the purpose of the method (at least for the base class). But sinceNext()
on the base class just delegates toInternalSample()
,Next()
is a reasonable thing to base the implementation on, e.g.The text was updated successfully, but these errors were encountered: