[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