Use static member functions from class instead of instances #25903
Conversation
src/crypto/muhash.cpp
Outdated
| @@ -299,7 +299,7 @@ Num3072 MuHash3072::ToNum3072(Span<const unsigned char> in) { | |||
| unsigned char tmp[Num3072::BYTE_SIZE]; | |||
|
|
|||
| uint256 hashed_in{(HashWriter{} << in).GetSHA256()}; | |||
| ChaCha20(hashed_in.data(), hashed_in.size()).Keystream(tmp, Num3072::BYTE_SIZE); | |||
| ChaCha20(hashed_in.data(), uint256::size()).Keystream(tmp, Num3072::BYTE_SIZE); | |||
There was a problem hiding this comment.
I think the pattern is generally a.data(), a.size(). If you want to change this, it might be best to use spans instead of the changes here in this pull.
There was a problem hiding this comment.
i am unfamiliar with spans, how would that work?
There was a problem hiding this comment.
See std::span and our implementation of it Span
There was a problem hiding this comment.
Thanks, attempted in b8f7201.
Not sure if I should add this commit in a separate PR.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
0ab31bc to
b8f7201
Compare
src/core_write.cpp
Outdated
| @@ -69,7 +69,7 @@ std::string FormatScript(const CScript& script) | |||
| ret += strprintf("0x%x ", HexStr(std::vector<uint8_t>(it2, script.end()))); | |||
| break; | |||
| } | |||
| return ret.substr(0, ret.empty() ? ret.npos : ret.size() - 1); | |||
| return ret.substr(0, ret.empty() ? std::string::npos : ret.size() - 1); | |||
There was a problem hiding this comment.
personally I prefer ret.npos, or at least think that either is fine and we don't need a linter for this
There was a problem hiding this comment.
I only left a comment on this line, but I meant all other lines as well.
I really don't get the recent rush to enforce meaningless style decisions with clang-tidy, even happily introducing bugs along the way (#25695 (comment)).
There was a problem hiding this comment.
Yes I meant I updated it everywhere as well.
Well, it came from a readability issue and so I thought it would be better to enforce it.
b8f7201 to
e9b1c81
Compare
This simplifies the usage of various hasher Write functions as there is no need to pass the buffer size.
e9b1c81 to
6f8c291
Compare
|
+1 on switching to Span, -1 on switching from the instance to the class for accessing static methods though. |
I don't agree. Sometimes it makes sense to access by instance instead of class. If there's particular cases that are confusing, those can be fixed, but I don't think adopting this as a hard rule is a good idea. |
|
Closing because of Concept NACK. |
I noticed while reviewing a PR that some static class member functions are accessed by instance instead of class. I believe this makes the code misleading.
This PR enables the clang-tidy check 'readability-static-accessed-through-instance'.
https://clang.llvm.org/extra/clang-tidy/checks/readability/static-accessed-through-instance.html
I used the
-fixoption of clang-tidy to discover and fix these issues.