[clang][Tooling] Get rid of a hack in SymbolOccurrences, NFCI

[miyuki]

Summary

The class SymbolOccurrences can store either a single SourceRange
in-place or multiple SourceRanges on the heap. In the latter case
the number of source ranges is stored in the internal representation
of the beginning SourceLocation of the in-place SourceRange
object.

This change gets rid of such hack by placing SourceRange in a union
which holds either a valid SourceRange or an unsigned int (a number
of ranges).

The change also adds static_asserts that check that SourceRange and
SourceLocation are trivially destructible (this is required for the
current patch and for D94237 which has already been committed).

[miyuki]

I will wait until the end of this week to see if anyone has objections.

[simon_tatham]

clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h

81

This surely relies on SourceRange having no destructor (or rather, a trivial one). If that ever changes, then destruction of this class will risk either a spurious call to the destructor or a missing one.

Is there any way to somehow arrange that a build failure will occur if the definition of SourceRange changes in that way?

[simon_tatham]

clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h

81

... aha, found one. Such as a static assertion that std::is_trivially_destructible<clang::SourceLocation>::value is 1.

[miyuki]

Added static_asserts that check that SourceRange and SourceLocation are trivially destructible.

[simon_tatham]

Thanks. LGTM now.

clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h

SymbolOccurrence(const SymbolName &Name, OccurrenceKind Kind,
                    ArrayRef<SourceLocation> Locations);
 
   SymbolOccurrence(SymbolOccurrence &&) = default;
   SymbolOccurrence &operator=(SymbolOccurrence &&) = default;
 
   OccurrenceKind getKind() const { return Kind; }
 
   ArrayRef<SourceRange> getNameRanges() const {
-    if (MultipleRanges) {
-      return llvm::makeArrayRef(MultipleRanges.get(),
-                                RangeOrNumRanges.getBegin().getRawEncoding());
-    }
-    return RangeOrNumRanges;
+    if (MultipleRanges)
+      return llvm::makeArrayRef(MultipleRanges.get(), NumRanges);
+    return SingleRange;
   }
 
 private:
   OccurrenceKind Kind;
   std::unique_ptr<SourceRange[]> MultipleRanges;
-  SourceRange RangeOrNumRanges;
+  union {
+    SourceRange SingleRange;
+    unsigned NumRanges;
+  };
 };
 
 using SymbolOccurrences = std::vector<SymbolOccurrence>;
 
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_RENAME_SYMBOL_OCCURRENCES_H

clang/lib/Basic/SourceLocation.cpp

void PrettyStackTraceLoc::print(raw_ostream &OS) const {
   }
   OS << Message << '\n';
 }
 
 //===----------------------------------------------------------------------===//
 // SourceLocation
 //===----------------------------------------------------------------------===//
 
+static_assert(std::is_trivially_destructible<SourceLocation>::value,
+              "SourceLocation must be trivially destructible because it is "
+              "used in unions");
+
+static_assert(std::is_trivially_destructible<SourceRange>::value,
+              "SourceRange must be trivially destructible because it is "
+              "used in unions");
+
 unsigned SourceLocation::getHashValue() const {
   return llvm::DenseMapInfo<unsigned>::getHashValue(ID);
 }
 
 void llvm::FoldingSetTrait<SourceLocation>::Profile(
     const SourceLocation &X, llvm::FoldingSetNodeID &ID) {
   ID.AddInteger(X.ID);
 }

clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp

 SymbolOccurrence::SymbolOccurrence(const SymbolName &Name, OccurrenceKind Kind,
                                    ArrayRef<SourceLocation> Locations)
     : Kind(Kind) {
   ArrayRef<std::string> NamePieces = Name.getNamePieces();
   assert(Locations.size() == NamePieces.size() &&
          "mismatching number of locations and lengths");
   assert(!Locations.empty() && "no locations");
   if (Locations.size() == 1) {
-    RangeOrNumRanges = SourceRange(
+    new (&SingleRange) SourceRange(
         Locations[0], Locations[0].getLocWithOffset(NamePieces[0].size()));
     return;
   }
   MultipleRanges = std::make_unique<SourceRange[]>(Locations.size());
-  RangeOrNumRanges.setBegin(
-      SourceLocation::getFromRawEncoding(Locations.size()));
+  NumRanges = Locations.size();
   for (const auto &Loc : llvm::enumerate(Locations)) {
     MultipleRanges[Loc.index()] = SourceRange(
         Loc.value(),
         Loc.value().getLocWithOffset(NamePieces[Loc.index()].size()));
   }
 }