Skip to content

Commit a0e3091

Browse files
committed
[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
1 parent e16959c commit a0e3091

File tree

3 files changed

+17
-9
lines changed

3 files changed

+17
-9
lines changed

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,18 @@ class SymbolOccurrence {
6969
OccurrenceKind getKind() const { return Kind; }
7070

7171
ArrayRef<SourceRange> getNameRanges() const {
72-
if (MultipleRanges) {
73-
return llvm::makeArrayRef(MultipleRanges.get(),
74-
RangeOrNumRanges.getBegin().getRawEncoding());
75-
}
76-
return RangeOrNumRanges;
72+
if (MultipleRanges)
73+
return llvm::makeArrayRef(MultipleRanges.get(), NumRanges);
74+
return SingleRange;
7775
}
7876

7977
private:
8078
OccurrenceKind Kind;
8179
std::unique_ptr<SourceRange[]> MultipleRanges;
82-
SourceRange RangeOrNumRanges;
80+
union {
81+
SourceRange SingleRange;
82+
unsigned NumRanges;
83+
};
8384
};
8485

8586
using SymbolOccurrences = std::vector<SymbolOccurrence>;

clang/lib/Basic/SourceLocation.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ void PrettyStackTraceLoc::print(raw_ostream &OS) const {
4242
// SourceLocation
4343
//===----------------------------------------------------------------------===//
4444

45+
static_assert(std::is_trivially_destructible<SourceLocation>::value,
46+
"SourceLocation must be trivially destructible because it is "
47+
"used in unions");
48+
49+
static_assert(std::is_trivially_destructible<SourceRange>::value,
50+
"SourceRange must be trivially destructible because it is "
51+
"used in unions");
52+
4553
unsigned SourceLocation::getHashValue() const {
4654
return llvm::DenseMapInfo<unsigned>::getHashValue(ID);
4755
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,12 @@ SymbolOccurrence::SymbolOccurrence(const SymbolName &Name, OccurrenceKind Kind,
2121
"mismatching number of locations and lengths");
2222
assert(!Locations.empty() && "no locations");
2323
if (Locations.size() == 1) {
24-
RangeOrNumRanges = SourceRange(
24+
new (&SingleRange) SourceRange(
2525
Locations[0], Locations[0].getLocWithOffset(NamePieces[0].size()));
2626
return;
2727
}
2828
MultipleRanges = std::make_unique<SourceRange[]>(Locations.size());
29-
RangeOrNumRanges.setBegin(
30-
SourceLocation::getFromRawEncoding(Locations.size()));
29+
NumRanges = Locations.size();
3130
for (const auto &Loc : llvm::enumerate(Locations)) {
3231
MultipleRanges[Loc.index()] = SourceRange(
3332
Loc.value(),

0 commit comments

Comments
 (0)