Make std.digest.hmac @safe and runnable#5587
Conversation
|
Thanks for your pull request, @wilzbach! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
wilzbach
force-pushed
the
digest-runnable
branch
2 times, most recently
from
ae46fa3 to
9f461ff
Compare
| import std.digest.hmac, std.digest.sha, std.stdio; | ||
| import std.ascii : LetterCase; | ||
| import std.digest.digest : toHexString; | ||
| import std.digest.sha : SHA1; |
There was a problem hiding this comment.
It's time we move on with #5013 and add a package.d to std.digest ...
std/digest/hmac.d
Outdated
| { | ||
| import std.digest.hmac, std.digest.sha, std.stdio; | ||
| import std.ascii : LetterCase; | ||
| import std.digest.digest : toHexString; |
There was a problem hiding this comment.
#5013 was merged and as far as I understand, this can be changed to: import std.digest : toHexString
| auto hash = HMAC!(H, blockSize)(secret); | ||
| foreach (datum; data) | ||
| copy(datum, &hash); | ||
| put(hash, datum); |
There was a problem hiding this comment.
Any difference in performance?
There was a problem hiding this comment.
None, we even save a function call on bad optimizers as copy does exactly the same:
TargetRange copy(SourceRange, TargetRange)(SourceRange source, TargetRange target)
if (!areCopyCompatibleArrays!(SourceRange, TargetRange) &&
isInputRange!SourceRange &&
isOutputRange!(TargetRange, ElementType!SourceRange))
{
// Specialize for 2 random access ranges.
// Typically 2 random access ranges are faster iterated by common
// index than by x.popFront(), y.popFront() pair
static if (isRandomAccessRange!SourceRange &&
hasLength!SourceRange &&
hasSlicing!TargetRange &&
isRandomAccessRange!TargetRange &&
hasLength!TargetRange)
{
auto len = source.length;
foreach (idx; 0 .. len)
target[idx] = source[idx];
return target[len .. target.length];
}
else
{
put(target, source);
return target;
}
}The output rangeHMAC doesn't have a length, nor slicing or random access.
wilzbach
force-pushed
the
digest-runnable
branch
from
9f461ff to
d02199a
Compare
Yes - 🎉 |
std/digest/hmac.d
Outdated
| if (allSatisfy!(isDigestibleRange, typeof(data))) | ||
| { | ||
| import std.algorithm.mutation : copy; | ||
| import std.range : put; |
There was a problem hiding this comment.
this can be specified to import std.range.primitives : put;
wilzbach
force-pushed
the
digest-runnable
branch
from
d02199a to
de8de6f
Compare
6 participants
When I first encountered the
version(StdDdoc)unittest instd.digest.hmac, I tried to go "the easy way" and ignore versioned unittest in the tests_extractor (-> dlang/tools#249).However @CyberShadow correctly reminded me that it makes more sense to fix the outlier and I even discovered another problem about the
version(StdDdoc)approach - it's not part of the testsuite and apparently at some point in the timestd.digest.hmacwas@safe(it's not anymore). I fixed that as well and as a consequencestd.digest.hmacis now completely@safe🎉So I am really happy about @CyberShadow's remark to avoid working around the outlier with the tool!
CC @jpf91