Skip to content

Commit 78c6554

Browse files
authored
[AST] Give CharUnits::operator% a consistent type. NFC (#160781)
Update the `operator%` overload that accepts `CharUnits` to return `CharUnits` to match the other `operator%`. This is more logical than returning an `int64` and cleans up users that want to continue to do math with the result. Many users of this were explicitly comparing against 0. I considered updating these to compare against `CharUnits::Zero` or even introducing an `explicit operator bool()`, but they all feel clearer if we update them to use the existing `isMultipleOf()` function instead.
1 parent 9e04291 commit 78c6554

File tree

9 files changed

+19
-20
lines changed

9 files changed

+19
-20
lines changed

clang/include/clang/AST/CharUnits.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ namespace clang {
141141
/// Among other things, this promises that
142142
/// self.alignTo(N) will just return self.
143143
bool isMultipleOf(CharUnits N) const {
144-
return (*this % N) == 0;
144+
return (*this % N) == CharUnits::Zero();
145145
}
146146

147147
// Arithmetic operators.
@@ -165,8 +165,8 @@ namespace clang {
165165
CharUnits operator% (QuantityType N) const {
166166
return CharUnits(Quantity % N);
167167
}
168-
QuantityType operator% (const CharUnits &Other) const {
169-
return Quantity % Other.Quantity;
168+
CharUnits operator%(const CharUnits &Other) const {
169+
return CharUnits(Quantity % Other.Quantity);
170170
}
171171
CharUnits operator+ (const CharUnits &Other) const {
172172
return CharUnits(Quantity + Other.Quantity);

clang/lib/AST/APValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ void APValue::printPretty(raw_ostream &Out, const PrintingPolicy &Policy,
784784
if (!O.isZero()) {
785785
if (IsReference)
786786
Out << "*(";
787-
if (S.isZero() || O % S) {
787+
if (S.isZero() || !O.isMultipleOf(S)) {
788788
Out << "(char*)";
789789
S = CharUnits::One();
790790
}

clang/lib/AST/RecordLayoutBuilder.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2087,9 +2087,8 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
20872087
if (InsertExtraPadding) {
20882088
CharUnits ASanAlignment = CharUnits::fromQuantity(8);
20892089
CharUnits ExtraSizeForAsan = ASanAlignment;
2090-
if (FieldSize % ASanAlignment)
2091-
ExtraSizeForAsan +=
2092-
ASanAlignment - CharUnits::fromQuantity(FieldSize % ASanAlignment);
2090+
if (!FieldSize.isMultipleOf(ASanAlignment))
2091+
ExtraSizeForAsan += ASanAlignment - (FieldSize % ASanAlignment);
20932092
EffectiveFieldSize = FieldSize = FieldSize + ExtraSizeForAsan;
20942093
}
20952094

@@ -2119,10 +2118,10 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
21192118
if (RD->hasAttr<PackedAttr>() || !MaxFieldAlignment.isZero())
21202119
if (FieldAlign < OriginalFieldAlign)
21212120
if (D->getType()->isRecordType()) {
2122-
// If the offset is a multiple of the alignment of
2121+
// If the offset is not a multiple of the alignment of
21232122
// the type, raise the warning.
21242123
// TODO: Takes no account the alignment of the outer struct
2125-
if (FieldOffset % OriginalFieldAlign != 0)
2124+
if (!FieldOffset.isMultipleOf(OriginalFieldAlign))
21262125
Diag(D->getLocation(), diag::warn_unaligned_access)
21272126
<< Context.getCanonicalTagType(RD) << D->getName()
21282127
<< D->getType();

clang/lib/CodeGen/CGAtomic.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -880,7 +880,7 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
880880
CharUnits MaxInlineWidth =
881881
getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
882882
DiagnosticsEngine &Diags = CGM.getDiags();
883-
bool Misaligned = (Ptr.getAlignment() % TInfo.Width) != 0;
883+
bool Misaligned = !Ptr.getAlignment().isMultipleOf(TInfo.Width);
884884
bool Oversized = getContext().toBits(TInfo.Width) > MaxInlineWidthInBits;
885885
if (Misaligned) {
886886
Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)

clang/lib/CodeGen/CGExprConstant.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ llvm::Constant *ConstantAggregateBuilder::buildFrom(
433433

434434
// All remaining elements must be the same type.
435435
if (Elems[I]->getType() != CommonType ||
436-
Offset(I) % ElemSize != 0) {
436+
!Offset(I).isMultipleOf(ElemSize)) {
437437
CanEmitArray = false;
438438
break;
439439
}

clang/lib/CodeGen/CGObjCMac.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5367,7 +5367,7 @@ IvarLayoutBuilder::buildBitmap(CGObjCCommonMac &CGObjC,
53675367

53685368
// Ignore scan requests that don't start at an even multiple of the
53695369
// word size. We can't encode them.
5370-
if ((beginOfScan % WordSize) != 0)
5370+
if (!beginOfScan.isMultipleOf(WordSize))
53715371
continue;
53725372

53735373
// Ignore scan requests that start before the instance start.

clang/lib/CodeGen/CGRecordLayoutBuilder.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -369,11 +369,11 @@ void CGRecordLowering::lowerUnion(bool isNonVirtualBaseType) {
369369
appendPaddingBytes(LayoutSize - getSize(StorageType));
370370
// Set packed if we need it.
371371
const auto StorageAlignment = getAlignment(StorageType);
372-
assert((Layout.getSize() % StorageAlignment == 0 ||
373-
Layout.getDataSize() % StorageAlignment) &&
372+
assert((Layout.getSize().isMultipleOf(StorageAlignment) ||
373+
!Layout.getDataSize().isMultipleOf(StorageAlignment)) &&
374374
"Union's standard layout and no_unique_address layout must agree on "
375375
"packedness");
376-
if (Layout.getDataSize() % StorageAlignment)
376+
if (!Layout.getDataSize().isMultipleOf(StorageAlignment))
377377
Packed = true;
378378
}
379379

@@ -977,20 +977,20 @@ void CGRecordLowering::determinePacked(bool NVBaseType) {
977977
continue;
978978
// If any member falls at an offset that it not a multiple of its alignment,
979979
// then the entire record must be packed.
980-
if (Member.Offset % getAlignment(Member.Data))
980+
if (!Member.Offset.isMultipleOf(getAlignment(Member.Data)))
981981
Packed = true;
982982
if (Member.Offset < NVSize)
983983
NVAlignment = std::max(NVAlignment, getAlignment(Member.Data));
984984
Alignment = std::max(Alignment, getAlignment(Member.Data));
985985
}
986986
// If the size of the record (the capstone's offset) is not a multiple of the
987987
// record's alignment, it must be packed.
988-
if (Members.back().Offset % Alignment)
988+
if (!Members.back().Offset.isMultipleOf(Alignment))
989989
Packed = true;
990990
// If the non-virtual sub-object is not a multiple of the non-virtual
991991
// sub-object's alignment, it must be packed. We cannot have a packed
992992
// non-virtual sub-object and an unpacked complete object or vise versa.
993-
if (NVSize % NVAlignment)
993+
if (!NVSize.isMultipleOf(NVAlignment))
994994
Packed = true;
995995
// Update the alignment of the sentinel.
996996
if (!Packed)

clang/lib/Sema/SemaChecking.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15949,7 +15949,7 @@ void Sema::RefersToMemberWithReducedAlignment(
1594915949
}
1595015950

1595115951
// Check if the synthesized offset fulfills the alignment.
15952-
if (Offset % ExpectedAlignment != 0 ||
15952+
if (!Offset.isMultipleOf(ExpectedAlignment) ||
1595315953
// It may fulfill the offset it but the effective alignment may still be
1595415954
// lower than the expected expression alignment.
1595515955
CompleteObjectAlignment < ExpectedAlignment) {

clang/lib/StaticAnalyzer/Core/Store.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ std::optional<const MemRegion *> StoreManager::castRegion(const MemRegion *R,
210210
// Is the offset a multiple of the size? If so, we can layer the
211211
// ElementRegion (with elementType == PointeeTy) directly on top of
212212
// the base region.
213-
if (off % pointeeTySize == 0) {
213+
if (off.isMultipleOf(pointeeTySize)) {
214214
newIndex = off / pointeeTySize;
215215
newSuperR = baseR;
216216
}

0 commit comments

Comments
 (0)