[clang] a0e3091 - [clang][Tooling] Get rid of a hack in SymbolOccurrences, NFCI
Mikhail Maltsev via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 22 05:01:58 PST 2021
Author: Mikhail Maltsev
Date: 2021-01-22T13:01:41Z
New Revision: a0e30914f8c8bb60795a008ce2d9e3c0a4f9b7a2
URL: https://github.com/llvm/llvm-project/commit/a0e30914f8c8bb60795a008ce2d9e3c0a4f9b7a2
DIFF: https://github.com/llvm/llvm-project/commit/a0e30914f8c8bb60795a008ce2d9e3c0a4f9b7a2.diff
LOG: [clang][Tooling] Get rid of a hack in SymbolOccurrences, NFCI
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_assert`s that check that `SourceRange` and
`SourceLocation` are trivially destructible (this is required for the
current patch and for D94237 which has already been committed).
Reviewed By: MarkMurrayARM, simon_tatham
Differential Revision: https://reviews.llvm.org/D94599
Added:
Modified:
clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
clang/lib/Basic/SourceLocation.cpp
clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h b/clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
index 3b903cb822f3..c4bfaa9cc377 100644
--- a/clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
+++ b/clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
@@ -69,17 +69,18 @@ class SymbolOccurrence {
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>;
diff --git a/clang/lib/Basic/SourceLocation.cpp b/clang/lib/Basic/SourceLocation.cpp
index fde139932c40..6f6412028d77 100644
--- a/clang/lib/Basic/SourceLocation.cpp
+++ b/clang/lib/Basic/SourceLocation.cpp
@@ -42,6 +42,14 @@ void PrettyStackTraceLoc::print(raw_ostream &OS) const {
// 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);
}
diff --git a/clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp b/clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
index 9e69d37e81ad..762864c953d8 100644
--- a/clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
+++ b/clang/lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
@@ -21,13 +21,12 @@ SymbolOccurrence::SymbolOccurrence(const SymbolName &Name, OccurrenceKind Kind,
"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(),
More information about the cfe-commits
mailing list