dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: sstefan1.
If you're still interested in pursuing this, I'm happy to review it.
A general comment is that there are a couple of functional changes in this
patch, where hash values are changing and data structures are being changed,
but they're buried in the noise of the refactoring that's going on at the same
time. I suggest splitting this up at least into two, where most of the NFC
changes are in a different commit.
================
Comment at:
clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h:35
private:
- std::unordered_set<unsigned> MatchedTemplateLocations;
+ llvm::DenseSet<SourceLocation> MatchedTemplateLocations;
};
----------------
Changing the data structure (`DenseSet` <= `std::unordered_set`) seems
unrelated to the rest of the patch. If it's necessary / useful, then please do
it separately. It'll be important to confirm that the users of
`MatchedTemplateLocations` don't rely on the values having stable addresses.
================
Comment at: clang/include/clang/Basic/SourceLocation.h:178
+ unsigned getHashValue() const {
+ return ID * 37U;
----------------
I suggest reusing the hash function for `unsigned`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69840/new/
https://reviews.llvm.org/D69840
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
