[PATCH] D69840: [Basic] Make SourceLocation usable as key in hash maps, NFCI
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 16 12:48:21 PDT 2020
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
More information about the cfe-commits
mailing list