[clang] Use SourceLocation as key in hash maps, NFCI

[miyuki]

Summary

The patch adjusts the existing llvm::DenseMap<unsigned, T> and
llvm::DenseSet<unsigned> objects that store source locations, so
that they use SourceLocation directly instead of unsigned.

This patch relies on the DenseMapInfo trait added in D89719.

It also replaces the construction of SourceLocation objects from
the constants -1 and -2 with calls to the trait's methods getEmptyKey
and getTombstoneKey where appropriate.

[miyuki]

Changed getRawEncoding -> getHashValue in Sema.h

[aprantl]

What's the motivation behind this?

[miyuki]

The motivation is to be able to make source locations' underlying type configurable. Richard Smith suggested that this might be feasible: http://lists.llvm.org/pipermail/cfe-dev/2019-October/063515.html
So, the first step is to get rid of getRawEncoding where possible. I think this would make the code cleaner anyway.

[miyuki]

ping

[miyuki]

ping^2

[miyuki]

ping^3

[aprantl]

@rsmith can you take a look at this to see if this is going into the right direction?

[miyuki]

ping^4

[miyuki]

Rebased, updated several more hash map occurrences.

[dexonsmith]

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.

clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h

35 ↗

(On Diff #249443)

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.

clang/include/clang/Basic/SourceLocation.h

178 ↗

(On Diff #249443)

I suggest reusing the hash function for unsigned.

[miyuki]

Yes, I am still interested in working on this. For now, I've split out some changes that don't require any modifications in SourceLocation into https://reviews.llvm.org/D89705.

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

Will do.

[miyuki]

Split out the refactoring part.

[dexonsmith]

LGTM.

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

 /// clang-tidy/readability/IdentifierNamingCheck.cpp.
 template <>
 struct DenseMapInfo<
     clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId> {
   using ClassDefId =
       clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
 
   static inline ClassDefId getEmptyKey() {
-    return ClassDefId(
-        clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)),
+    return ClassDefId(DenseMapInfo<clang::SourceLocation>::getEmptyKey(),
                       "EMPTY");
   }
 
   static inline ClassDefId getTombstoneKey() {
-    return ClassDefId(
-        clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)),
+    return ClassDefId(DenseMapInfo<clang::SourceLocation>::getTombstoneKey(),
                       "TOMBSTONE");
   }
 
   static unsigned getHashValue(ClassDefId Val) {
     assert(Val != getEmptyKey() && "Cannot hash the empty key!");
     assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!");
 
     std::hash<ClassDefId::second_type> SecondHash;
-    return Val.first.getRawEncoding() + SecondHash(Val.second);
+    return Val.first.getHashValue() + SecondHash(Val.second);
   }
 
   static bool isEqual(const ClassDefId &LHS, const ClassDefId &RHS) {
     if (RHS == getEmptyKey())
       return LHS == getEmptyKey();
     if (RHS == getTombstoneKey())
       return LHS == getTombstoneKey();
     return LHS == RHS;
   }
 };
 
 } // namespace llvm
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SPECIAL_MEMBER_FUNCTIONS_H

clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h

struct NamingCheckFailure {
     }
 
     bool ShouldNotify() const {
       return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
     }
 
     ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
 
-    /// A set of all the identifier usages starting SourceLocation, in
-    /// their encoded form.
-    llvm::DenseSet<unsigned> RawUsageLocs;
+    /// A set of all the identifier usages starting SourceLocation.
+    llvm::DenseSet<SourceLocation> RawUsageLocs;
 
     NamingCheckFailure() = default;
   };
 
   using NamingCheckId = std::pair<SourceLocation, std::string>;
 
   using NamingCheckFailureMap =
       llvm::DenseMap<NamingCheckId, NamingCheckFailure>;

clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp

 namespace llvm {
 
 /// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps
 template <>
 struct DenseMapInfo<clang::tidy::RenamerClangTidyCheck::NamingCheckId> {
   using NamingCheckId = clang::tidy::RenamerClangTidyCheck::NamingCheckId;
 
   static inline NamingCheckId getEmptyKey() {
-    return NamingCheckId(
-        clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)),
+    return NamingCheckId(DenseMapInfo<clang::SourceLocation>::getEmptyKey(),
                          "EMPTY");
   }
 
   static inline NamingCheckId getTombstoneKey() {
-    return NamingCheckId(
-        clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)),
+    return NamingCheckId(DenseMapInfo<clang::SourceLocation>::getTombstoneKey(),
                          "TOMBSTONE");
   }
 
   static unsigned getHashValue(NamingCheckId Val) {
     assert(Val != getEmptyKey() && "Cannot hash the empty key!");
     assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!");
 
     std::hash<NamingCheckId::second_type> SecondHash;
-    return Val.first.getRawEncoding() + SecondHash(Val.second);
+    return DenseMapInfo<clang::SourceLocation>::getHashValue(Val.first) +
+           SecondHash(Val.second);
   }
 
   static bool isEqual(const NamingCheckId &LHS, const NamingCheckId &RHS) {
     if (RHS == getEmptyKey())
       return LHS == getEmptyKey();
     if (RHS == getTombstoneKey())
       return LHS == getTombstoneKey();
     return LHS == RHS;
if (SourceMgr)
     FixLocation = SourceMgr->getSpellingLoc(FixLocation);
   if (FixLocation.isInvalid())
     return;
 
   // Try to insert the identifier location in the Usages map, and bail out if it
   // is already in there
   RenamerClangTidyCheck::NamingCheckFailure &Failure =
       NamingCheckFailures[Decl];
-
-  if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
+  if (!Failure.RawUsageLocs.insert(FixLocation).second)
     return;
 
   if (!Failure.ShouldFix())
     return;
 
   if (SourceMgr && SourceMgr->isWrittenInScratchSpace(FixLocation))
     Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro;
 
if (Failure.ShouldNotify()) {
           // identifier names by combining multiple tokens.
           //
           // For destructors, we already take care of it by remembering the
           // location of the start of the identifier and not the start of the
           // tilde.
           //
           // Other multi-token identifiers, such as operators are not checked at
           // all.
-          Diag << FixItHint::CreateReplacement(
-              SourceRange(SourceLocation::getFromRawEncoding(Loc)),
+          Diag << FixItHint::CreateReplacement(SourceRange(Loc),
                                                Failure.Info.Fixup);
         }
       }
     }
   }
 }
 
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clangd/FindTarget.cpp

class ExplicitReferenceCollector
     : public RecursiveASTVisitor<ExplicitReferenceCollector> {
 public:
   ExplicitReferenceCollector(llvm::function_ref<void(ReferenceLoc)> Out)
       : Out(Out) {
     assert(Out);
   }
 
   bool VisitTypeLoc(TypeLoc TTL) {
-    if (TypeLocsToSkip.count(TTL.getBeginLoc().getRawEncoding()))
+    if (TypeLocsToSkip.count(TTL.getBeginLoc()))
       return true;
     visitNode(DynTypedNode::create(TTL));
     return true;
   }
 
   bool TraverseElaboratedTypeLoc(ElaboratedTypeLoc L) {
     // ElaboratedTypeLoc will reports information for its inner type loc.
     // Otherwise we loose information about inner types loc's qualifier.
     TypeLoc Inner = L.getNamedTypeLoc().getUnqualifiedLoc();
-    TypeLocsToSkip.insert(Inner.getBeginLoc().getRawEncoding());
+    TypeLocsToSkip.insert(Inner.getBeginLoc());
     return RecursiveASTVisitor::TraverseElaboratedTypeLoc(L);
   }
 
   bool VisitStmt(Stmt *S) {
     visitNode(DynTypedNode::create(*S));
     return true;
   }
 
public:
 
   // We have to use Traverse* because there is no corresponding Visit*.
   bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc L) {
     if (!L.getNestedNameSpecifier())
       return true;
     visitNode(DynTypedNode::create(L));
     // Inner type is missing information about its qualifier, skip it.
     if (auto TL = L.getTypeLoc())
-      TypeLocsToSkip.insert(TL.getBeginLoc().getRawEncoding());
+      TypeLocsToSkip.insert(TL.getBeginLoc());
     return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(L);
   }
 
   bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
     visitNode(DynTypedNode::create(*Init));
     return RecursiveASTVisitor::TraverseConstructorInitializer(Init);
   }
 
if (Ref.NameLoc.isInvalid()) {
       return;
     }
     Out(Ref);
   }
 
   llvm::function_ref<void(ReferenceLoc)> Out;
   /// TypeLocs starting at these locations must be skipped, see
   /// TraverseElaboratedTypeSpecifierLoc for details.
-  llvm::DenseSet</*SourceLocation*/ unsigned> TypeLocsToSkip;
+  llvm::DenseSet<SourceLocation> TypeLocsToSkip;
 };
 } // namespace
 
 void findExplicitReferences(const Stmt *S,
                             llvm::function_ref<void(ReferenceLoc)> Out) {
   assert(S);
   ExplicitReferenceCollector(Out).TraverseStmt(const_cast<Stmt *>(S));
 }

clang/include/clang/Edit/EditedSource.h

struct MacroArgUse {
 
     bool operator==(const MacroArgUse &Other) const {
       return std::tie(Identifier, ImmediateExpansionLoc, UseLoc) ==
              std::tie(Other.Identifier, Other.ImmediateExpansionLoc,
                       Other.UseLoc);
     }
   };
 
-  llvm::DenseMap<unsigned, SmallVector<MacroArgUse, 2>> ExpansionToArgMap;
+  llvm::DenseMap<SourceLocation, SmallVector<MacroArgUse, 2>> ExpansionToArgMap;
   SmallVector<std::pair<SourceLocation, MacroArgUse>, 2>
     CurrCommitMacroArgExps;
 
   IdentifierTable IdentTable;
   llvm::BumpPtrAllocator StrAlloc;
 
 public:
   EditedSource(const SourceManager &SM, const LangOptions &LangOpts,

clang/include/clang/Sema/Sema.h

template <> struct DenseMapInfo<clang::Sema::FunctionDeclAndLoc> {
   }
 
   static FunctionDeclAndLoc getTombstoneKey() {
     return {FDBaseInfo::getTombstoneKey(), clang::SourceLocation()};
   }
 
   static unsigned getHashValue(const FunctionDeclAndLoc &FDL) {
     return hash_combine(FDBaseInfo::getHashValue(FDL.FD),
-                        FDL.Loc.getRawEncoding());
+                        FDL.Loc.getHashValue());
   }
 
   static bool isEqual(const FunctionDeclAndLoc &LHS,
                       const FunctionDeclAndLoc &RHS) {
     return LHS.FD == RHS.FD && LHS.Loc == RHS.Loc;
   }
 };
 } // namespace llvm
 
 #endif

clang/include/clang/Tooling/Syntax/Tokens.h

private:
   ///
   /// Logically, at each point of the preprocessor execution there is a stack of
   /// macro expansions being processed and we could use it to recover the
   /// location information we need. However, the public preprocessor API only
   /// exposes the points when macro expansions start (when we push a macro onto
   /// the stack) and not when they end (when we pop a macro from the stack).
   /// To workaround this limitation, we rely on source location information
   /// stored in this map.
-  using PPExpansions = llvm::DenseMap</*SourceLocation*/ int, SourceLocation>;
+  using PPExpansions = llvm::DenseMap<SourceLocation, SourceLocation>;
   class Builder;
   class CollectPPExpansions;
 
   std::vector<syntax::Token> Expanded;
   // FIXME: we only store macro expansions, also add directives(#pragma, etc.)
   PPExpansions Expansions;
   Preprocessor &PP;
   CollectPPExpansions *Collector;
 };
 
 } // namespace syntax
 } // namespace clang
 
 #endif

clang/lib/ARCMigrate/TransGCAttrs.cpp

public:
   }
 
   bool handleAttr(AttributedTypeLoc TL, Decl *D = nullptr) {
     auto *OwnershipAttr = TL.getAttrAs<ObjCOwnershipAttr>();
     if (!OwnershipAttr)
       return false;
 
     SourceLocation Loc = OwnershipAttr->getLocation();
-    unsigned RawLoc = Loc.getRawEncoding();
-    if (MigrateCtx.AttrSet.count(RawLoc))
+    SourceLocation OrigLoc = Loc;
+    if (MigrateCtx.AttrSet.count(OrigLoc))
       return true;
 
     ASTContext &Ctx = MigrateCtx.Pass.Ctx;
     SourceManager &SM = Ctx.getSourceManager();
     if (Loc.isMacroID())
       Loc = SM.getImmediateExpansionRange(Loc).getBegin();
     StringRef Spell = OwnershipAttr->getKind()->getName();
     MigrationContext::GCAttrOccurrence::AttrKind Kind;
     if (Spell == "strong")
       Kind = MigrationContext::GCAttrOccurrence::Strong;
     else if (Spell == "weak")
       Kind = MigrationContext::GCAttrOccurrence::Weak;
     else
       return false;
 
-    MigrateCtx.AttrSet.insert(RawLoc);
+    MigrateCtx.AttrSet.insert(OrigLoc);
     MigrateCtx.GCAttrs.push_back(MigrationContext::GCAttrOccurrence());
     MigrationContext::GCAttrOccurrence &Attr = MigrateCtx.GCAttrs.back();
 
     Attr.Kind = Kind;
     Attr.Loc = Loc;
     Attr.ModifiedType = TL.getModifiedLoc().getType();
     Attr.Dcl = D;
     Attr.FullyMigratable = FullyMigratable;
for (unsigned i = 0, e = MigrateCtx.GCAttrs.size(); i != e; ++i) {
     MigrationContext::GCAttrOccurrence &Attr = MigrateCtx.GCAttrs[i];
     if (Attr.Kind == MigrationContext::GCAttrOccurrence::Weak) {
       if (Attr.ModifiedType.isNull() ||
           !Attr.ModifiedType->isObjCRetainableType())
         continue;
       if (!canApplyWeak(MigrateCtx.Pass.Ctx, Attr.ModifiedType,
                         /*AllowOnUnknownClass=*/true)) {
         Transaction Trans(TA);
-        if (!MigrateCtx.RemovedAttrSet.count(Attr.Loc.getRawEncoding()))
+        if (!MigrateCtx.RemovedAttrSet.count(Attr.Loc))
           TA.replaceText(Attr.Loc, "__weak", "__unsafe_unretained");
         TA.clearDiagnostic(diag::err_arc_weak_no_runtime,
                            diag::err_arc_unsupported_weak_class,
                            Attr.Loc);
       }
     }
   }
 }
if (hasWeak && hasStrong)
     return;
 
   TransformActions &TA = MigrateCtx.Pass.TA;
   Transaction Trans(TA);
 
   if (GCAttrsCollector::hasObjCImpl(
                               cast<Decl>(IndProps.front()->getDeclContext()))) {
     if (hasWeak)
-      MigrateCtx.AtPropsWeak.insert(AtLoc.getRawEncoding());
+      MigrateCtx.AtPropsWeak.insert(AtLoc);
 
   } else {
     StringRef toAttr = "strong";
     if (hasWeak) {
       if (canApplyWeak(MigrateCtx.Pass.Ctx, IndProps.front()->getType(),
                        /*AllowOnUnknownClass=*/true))
         toAttr = "weak";
       else
for (unsigned i = 0, e = ATLs.size(); i != e; ++i) {
     if (Loc.isMacroID())
       Loc = MigrateCtx.Pass.Ctx.getSourceManager()
                 .getImmediateExpansionRange(Loc)
                 .getBegin();
     TA.remove(Loc);
     TA.clearDiagnostic(diag::err_objc_property_attr_mutually_exclusive, AtLoc);
     TA.clearDiagnostic(diag::err_arc_inconsistent_property_ownership,
                        ATLs[i].second->getLocation());
-    MigrateCtx.RemovedAttrSet.insert(Loc.getRawEncoding());
+    MigrateCtx.RemovedAttrSet.insert(Loc);
   }
 }
 
 static void checkAllProps(MigrationContext &MigrateCtx,
                           std::vector<ObjCPropertyDecl *> &AllProps) {
   typedef llvm::TinyPtrVector<ObjCPropertyDecl *> IndivPropsTy;
-  llvm::DenseMap<unsigned, IndivPropsTy> AtProps;
+  llvm::DenseMap<SourceLocation, IndivPropsTy> AtProps;
 
   for (unsigned i = 0, e = AllProps.size(); i != e; ++i) {
     ObjCPropertyDecl *PD = AllProps[i];
     if (PD->getPropertyAttributesAsWritten() &
         (ObjCPropertyAttribute::kind_assign |
          ObjCPropertyAttribute::kind_readonly)) {
       SourceLocation AtLoc = PD->getAtLoc();
       if (AtLoc.isInvalid())
         continue;
-      unsigned RawAt = AtLoc.getRawEncoding();
-      AtProps[RawAt].push_back(PD);
+      AtProps[AtLoc].push_back(PD);
     }
   }
 
-  for (llvm::DenseMap<unsigned, IndivPropsTy>::iterator
-         I = AtProps.begin(), E = AtProps.end(); I != E; ++I) {
-    SourceLocation AtLoc = SourceLocation::getFromRawEncoding(I->first);
+  for (auto I = AtProps.begin(), E = AtProps.end(); I != E; ++I) {
+    SourceLocation AtLoc = I->first;
     IndivPropsTy &IndProps = I->second;
     checkAllAtProps(MigrateCtx, AtLoc, IndProps);
   }
 }
 
 void GCAttrsTraverser::traverseTU(MigrationContext &MigrateCtx) {
   std::vector<ObjCPropertyDecl *> AllProps;
   GCAttrsCollector(MigrateCtx, AllProps).TraverseDecl(

clang/lib/ARCMigrate/TransProperties.cpp

private:
   }
 
   // Returns true if all declarations in the @property have GC __weak.
   bool hasGCWeak(PropsTy &props, SourceLocation atLoc) const {
     if (!Pass.isGCMigration())
       return false;
     if (props.empty())
       return false;
-    return MigrateCtx.AtPropsWeak.count(atLoc.getRawEncoding());
+    return MigrateCtx.AtPropsWeak.count(atLoc);
   }
 
   bool isUserDeclared(ObjCIvarDecl *ivarD) const {
     return ivarD && !ivarD->getSynthesize();
   }
 
   QualType getPropertyType(PropsTy &props) const {
     assert(!props.empty());

clang/lib/ARCMigrate/Transforms.h

struct GCAttrOccurrence {
     SourceLocation Loc;
     QualType ModifiedType;
     Decl *Dcl;
     /// true if the attribute is owned, e.g. it is in a body and not just
     /// in an interface.
     bool FullyMigratable;
   };
   std::vector<GCAttrOccurrence> GCAttrs;
-  llvm::DenseSet<unsigned> AttrSet;
-  llvm::DenseSet<unsigned> RemovedAttrSet;
+  llvm::DenseSet<SourceLocation> AttrSet;
+  llvm::DenseSet<SourceLocation> RemovedAttrSet;
 
   /// Set of raw '@' locations for 'assign' properties group that contain
   /// GC __weak.
-  llvm::DenseSet<unsigned> AtPropsWeak;
+  llvm::DenseSet<SourceLocation> AtPropsWeak;
 
   explicit MigrationContext(MigrationPass &pass) : Pass(pass) {}
   ~MigrationContext();
 
   typedef std::vector<ASTTraverser *>::iterator traverser_iterator;
   traverser_iterator traversers_begin() { return Traversers.begin(); }
   traverser_iterator traversers_end() { return Traversers.end(); }
 

clang/lib/CodeGen/CGOpenMPRuntime.h

protected:
   /// reference to the existing copy is returned.
   /// \param CriticalName Name of the critical region.
   ///
   llvm::Value *getCriticalRegionLock(StringRef CriticalName);
 
 private:
 
   /// Map for SourceLocation and OpenMP runtime library debug locations.
-  typedef llvm::DenseMap<unsigned, llvm::Value *> OpenMPDebugLocMapTy;
+  typedef llvm::DenseMap<SourceLocation, llvm::Value *> OpenMPDebugLocMapTy;
   OpenMPDebugLocMapTy OpenMPDebugLocMap;
   /// The type for a microtask which gets passed to __kmpc_fork_call().
   /// Original representation is:
   /// typedef void (kmpc_micro)(kmp_int32 global_tid, kmp_int32 bound_tid,...);
   llvm::FunctionType *Kmpc_MicroTy = nullptr;
   /// Stores debug location and ThreadID for the function.
   struct DebugLocThreadIdTy {
     llvm::Value *DebugLoc;

clang/lib/Edit/EditedSource.cpp

 
 void EditedSource::startingCommit() {}
 
 void EditedSource::finishedCommit() {
   for (auto &ExpArg : CurrCommitMacroArgExps) {
     SourceLocation ExpLoc;
     MacroArgUse ArgUse;
     std::tie(ExpLoc, ArgUse) = ExpArg;
-    auto &ArgUses = ExpansionToArgMap[ExpLoc.getRawEncoding()];
+    auto &ArgUses = ExpansionToArgMap[ExpLoc];
     if (llvm::find(ArgUses, ArgUse) == ArgUses.end())
       ArgUses.push_back(ArgUse);
   }
   CurrCommitMacroArgExps.clear();
 }
 
 StringRef EditedSource::copyString(const Twine &twine) {
   SmallString<128> Data;
   return copyString(twine.toStringRef(Data));
 }
 
 bool EditedSource::canInsertInOffset(SourceLocation OrigLoc, FileOffset Offs) {
   FileEditsTy::iterator FA = getActionForOffset(Offs);
   if (FA != FileEdits.end()) {
     if (FA->first != Offs)
       return false; // position has been removed.
   }
 
   if (SourceMgr.isMacroArgExpansion(OrigLoc)) {
     SourceLocation ExpLoc;
     MacroArgUse ArgUse;
     deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
-    auto I = ExpansionToArgMap.find(ExpLoc.getRawEncoding());
+    auto I = ExpansionToArgMap.find(ExpLoc);
     if (I != ExpansionToArgMap.end() &&
         find_if(I->second, [&](const MacroArgUse &U) {
           return ArgUse.Identifier == U.Identifier &&
                  std::tie(ArgUse.ImmediateExpansionLoc, ArgUse.UseLoc) !=
                      std::tie(U.ImmediateExpansionLoc, U.UseLoc);
         }) != I->second.end()) {
       // Trying to write in a macro argument input that has already been
       // written by a previous commit for another expansion of the same macro

clang/lib/Tooling/Syntax/BuildTree.cpp

 ///
 /// Note that all children are expected to be processed when building a node.
 ///
 /// Call finalize() to finish building the tree and consume the root node.
 class syntax::TreeBuilder {
 public:
   TreeBuilder(syntax::Arena &Arena) : Arena(Arena), Pending(Arena) {
     for (const auto &T : Arena.getTokenBuffer().expandedTokens())
-      LocationToToken.insert({T.location().getRawEncoding(), &T});
+      LocationToToken.insert({T.location(), &T});
   }
 
   llvm::BumpPtrAllocator &allocator() { return Arena.getAllocator(); }
   const SourceManager &sourceManager() const {
     return Arena.getSourceManager();
   }
 
   /// Populate children for \p New node, assuming it covers tokens from \p
private:
     std::map<const syntax::Token *, syntax::Node *> Trees;
   };
 
   /// For debugging purposes.
   std::string str() { return Pending.str(Arena); }
 
   syntax::Arena &Arena;
   /// To quickly find tokens by their start location.
-  llvm::DenseMap</*SourceLocation*/ unsigned, const syntax::Token *>
-      LocationToToken;
+  llvm::DenseMap<SourceLocation, const syntax::Token *> LocationToToken;
   Forest Pending;
   llvm::DenseSet<Decl *> DeclsWithoutSemicolons;
   ASTToSyntaxMapping Mapping;
 };
 
 namespace {
 class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
 public:
void syntax::TreeBuilder::markExprChild(Expr *Child, NodeRole Role) {
   syntax::Tree *ChildNode = Mapping.find(Child);
   assert(ChildNode != nullptr);
   setRole(ChildNode, Role);
 }
 
 const syntax::Token *syntax::TreeBuilder::findToken(SourceLocation L) const {
   if (L.isInvalid())
     return nullptr;
-  auto It = LocationToToken.find(L.getRawEncoding());
+  auto It = LocationToToken.find(L);
   assert(It != LocationToToken.end());
   return It->second;
 }
 
 syntax::TranslationUnit *
 syntax::buildSyntaxTree(Arena &A, const TranslationUnitDecl &TU) {
   TreeBuilder Builder(A);
   BuildTreeVisitor(TU.getASTContext(), Builder).TraverseAST(TU.getASTContext());
   return std::move(Builder).finalize();
 }

clang/lib/Tooling/Syntax/Tokens.cpp

if (LastExpansionEnd.isValid() &&
         !SM.isBeforeInTranslationUnit(LastExpansionEnd, Range.getEnd()))
       return;
 
     // If the macro invocation (B) starts in a macro (A) but ends in a file,
     // we'll create a merged mapping for A + B by overwriting the endpoint for
     // A's startpoint.
     if (!Range.getBegin().isFileID()) {
       Range.setBegin(SM.getExpansionLoc(Range.getBegin()));
-      assert(Collector->Expansions.count(Range.getBegin().getRawEncoding()) &&
+      assert(Collector->Expansions.count(Range.getBegin()) &&
              "Overlapping macros should have same expansion location");
     }
 
-    Collector->Expansions[Range.getBegin().getRawEncoding()] = Range.getEnd();
+    Collector->Expansions[Range.getBegin()] = Range.getEnd();
     LastExpansionEnd = Range.getEnd();
   }
   // FIXME: handle directives like #pragma, #include, etc.
 private:
   TokenCollector *Collector;
   /// Used to detect recursive macro expansions.
   SourceLocation LastExpansionEnd;
 };
auto FlushMapping = [&, this] {
       Mapping.BeginSpelled = NextSpelled;
     };
 
     while (NextSpelled < SpelledTokens.size() &&
            SpelledTokens[NextSpelled].location() < Target) {
       // If we know mapping bounds at [NextSpelled, KnownEnd] (macro expansion)
       // then we want to partition our (empty) mapping.
       //   [Start, NextSpelled) [NextSpelled, KnownEnd] (KnownEnd, Target)
-      SourceLocation KnownEnd = CollectedExpansions.lookup(
-          SpelledTokens[NextSpelled].location().getRawEncoding());
+      SourceLocation KnownEnd =
+          CollectedExpansions.lookup(SpelledTokens[NextSpelled].location());
       if (KnownEnd.isValid()) {
         FlushMapping(); // Emits [Start, NextSpelled)
         while (NextSpelled < SpelledTokens.size() &&
                SpelledTokens[NextSpelled].location() <= KnownEnd)
           ++NextSpelled;
         FlushMapping(); // Emits [NextSpelled, KnownEnd]
         // Now the loop contitues and will emit (KnownEnd, Target).
       } else {
if (Tok.location().isFileID()) {
              SpelledTokens[NextSpelled].location() ==
                  Result.ExpandedTokens[NextExpanded].location()) {
         ++NextSpelled;
         ++NextExpanded;
       }
       // We need no mapping for file tokens copied to the expanded stream.
     } else {
       // We found a new macro expansion. We should have its spelling bounds.
-      auto End = CollectedExpansions.lookup(Expansion.getRawEncoding());
+      auto End = CollectedExpansions.lookup(Expansion);
       assert(End.isValid() && "Macro expansion wasn't captured?");
 
       // Mapping starts here...
       TokenBuffer::Mapping Mapping;
       Mapping.BeginExpanded = NextExpanded;
       Mapping.BeginSpelled = NextSpelled;
       // ... consumes spelled tokens within bounds we captured ...
       while (NextSpelled < SpelledTokens.size() &&