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

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki updated this revision to Diff 317627.
miyuki edited the summary of this revision.
miyuki added a comment.

Added `static_assert`s that check that `SourceRange` and `SourceLocation` are 
trivially destructible.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94599/new/

https://reviews.llvm.org/D94599

Files:
  clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp


Index: clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
+++ clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
@@ -21,13 +21,12 @@
          "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(),
Index: clang/lib/Basic/SourceLocation.cpp
===================================================================
--- clang/lib/Basic/SourceLocation.cpp
+++ clang/lib/Basic/SourceLocation.cpp
@@ -42,6 +42,14 @@
 // 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);
 }
Index: clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
===================================================================
--- clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
+++ clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
@@ -69,17 +69,18 @@
   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>;



Index: clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
+++ clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
@@ -21,13 +21,12 @@
          "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(),
Index: clang/lib/Basic/SourceLocation.cpp
===================================================================
--- clang/lib/Basic/SourceLocation.cpp
+++ clang/lib/Basic/SourceLocation.cpp
@@ -42,6 +42,14 @@
 // 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);
 }
Index: clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
===================================================================
--- clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
+++ clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
@@ -69,17 +69,18 @@
   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>;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits