Skip to content

Commit 0b4ad71

Browse files
asudarsaasudarsa
committed
Address review comments
Signed-off-by: Arvind Sudarsanam <[email protected]>
1 parent f528b64 commit 0b4ad71

File tree

4 files changed

+31
-34
lines changed

4 files changed

+31
-34
lines changed

‎llvm/include/llvm/Support/PropertySetIO.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
//
77
//===----------------------------------------------------------------------===//
88
// Models a sequence of property sets and their input and output operations.
9-
// TODO use Yaml as I/O engine.
109
// PropertyValue set format:
1110
// '['<PropertyValue set name>']'
1211
// <property name>=<property type>'|'<property value>
@@ -83,7 +82,7 @@ class PropertyValue {
8382
template <typename C, typename T = typename C::value_type>
8483
PropertyValue(const C &Data)
8584
: PropertyValue(reinterpret_cast<const byte *>(Data.data()),
86-
Data.size() * sizeof(T) * /* bits in one byte */ 8) {}
85+
Data.size() * sizeof(T) * CHAR_BIT) {}
8786
PropertyValue(const llvm::StringRef &Str)
8887
: PropertyValue(reinterpret_cast<const byte *>(Str.data()),
8988
Str.size() * sizeof(char) * /* bits in one byte */ 8) {}
@@ -94,7 +93,7 @@ class PropertyValue {
9493

9594
PropertyValue &operator=(const PropertyValue &P);
9695

97-
// get property value as unsigned 32-bit integer
96+
// Get property value as unsigned 32-bit integer
9897
uint32_t asUint32() const {
9998
if (Ty != UINT32)
10099
llvm_unreachable("must be UINT32 value");
@@ -131,7 +130,7 @@ class PropertyValue {
131130
const byte *asRawByteArray() const {
132131
if (Ty != BYTE_ARRAY)
133132
llvm_unreachable("must be BYTE_ARRAY value");
134-
auto &ByteArrayVal = std::get<byte *>(Val);
133+
auto *ByteArrayVal = std::get<byte *>(Val);
135134
return ByteArrayVal;
136135
}
137136

@@ -146,7 +145,7 @@ class PropertyValue {
146145

147146
bool isValid() const { return getType() != NONE; }
148147

149-
// set property value; the 'T' type must be convertible to a property type tag
148+
// Set property value; the 'T' type must be convertible to a property type tag
150149
template <typename T> void set(T V) {
151150
if (getTypeTag<T>() != Ty)
152151
llvm_unreachable("invalid type tag for this operation");
@@ -158,7 +157,7 @@ class PropertyValue {
158157
SizeTy size() const {
159158
switch (Ty) {
160159
case UINT32:
161-
return sizeof(std::get<uint32_t>(Val));
160+
return sizeof(uint32_t);
162161
case BYTE_ARRAY:
163162
return getRawByteArraySize();
164163
default:
@@ -220,8 +219,8 @@ class PropertySetRegistry {
220219
"category already added");
221220
auto &PropSet = PropSetMap[Category];
222221

223-
for (const auto &Prop : Props)
224-
PropSet.insert_or_assign(Prop.first, PropertyValue(Prop.second));
222+
for (const auto &[PropName, PropVal] : Props)
223+
PropSet.insert_or_assign(PropName, PropertyValue(PropVal));
225224
}
226225

227226
/// Adds the given \p PropVal with the given \p PropName into the given \p

‎llvm/lib/Support/Base64.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,13 @@ class Base64Impl {
120120
static inline int decode(char Ch) {
121121
if (Ch >= 'A' && Ch <= 'Z') // 0..25
122122
return Ch - 'A';
123-
else if (Ch >= 'a' && Ch <= 'z') // 26..51
123+
if (Ch >= 'a' && Ch <= 'z') // 26..51
124124
return Ch - 'a' + 26;
125-
else if (Ch >= '0' && Ch <= '9') // 52..61
125+
if (Ch >= '0' && Ch <= '9') // 52..61
126126
return Ch - '0' + 52;
127-
else if (Ch == '+') // 62
127+
if (Ch == '+') // 62
128128
return 62;
129-
else if (Ch == '/') // 63
129+
if (Ch == '/') // 63
130130
return 63;
131131
return -1;
132132
}
@@ -160,7 +160,7 @@ class Base64Impl {
160160
static size_t encode(const byte *Src, raw_ostream &Out, size_t SrcSize) {
161161
size_t Off = 0;
162162

163-
// encode full byte triples
163+
// Encode full byte triples
164164
for (size_t TriB = 0; TriB < SrcSize / 3; ++TriB) {
165165
Off = TriB * 3;
166166
byte Byte0 = Src[Off++];
@@ -172,7 +172,7 @@ class Base64Impl {
172172
Out << EncodingTable[composeInd(Byte1, Byte2, 4)];
173173
Out << EncodingTable[(int)(Byte2 >> 2) & 0x3F];
174174
}
175-
// encode the remainder
175+
// Encode the remainder
176176
int RemBytes = SrcSize - Off;
177177

178178
if (RemBytes > 0) {
@@ -196,14 +196,14 @@ class Base64Impl {
196196
size_t SrcOff = 0;
197197
size_t DstOff = 0;
198198

199-
// decode full quads
199+
// Decode full quads.
200200
for (size_t Qch = 0; Qch < SrcSize / 4; ++Qch, SrcOff += 4, DstOff += 3) {
201201
byte Ch[4];
202202
Expected<bool> TrRes = decode4(Src + SrcOff, Ch);
203203

204204
if (!TrRes)
205205
return TrRes.takeError();
206-
// each quad of chars produces three bytes of output
206+
// Each quad of chars produces three bytes of output.
207207
Dst[DstOff + 0] = Ch[0] | (Ch[1] << 6);
208208
Dst[DstOff + 1] = (Ch[1] >> 2) | (Ch[2] << 4);
209209
Dst[DstOff + 2] = (Ch[2] >> 4) | (Ch[3] << 2);
@@ -212,7 +212,7 @@ class Base64Impl {
212212

213213
if (RemChars == 0)
214214
return DstOff;
215-
// decode the remainder; variants:
215+
// Decode the remainder; variants:
216216
// 2 chars remain - produces single byte
217217
// 3 chars remain - produces two bytes
218218

@@ -266,4 +266,4 @@ Expected<size_t> Base64::decode(const char *Src, byte *Dst, size_t SrcSize) {
266266
Expected<std::unique_ptr<byte[]>> Base64::decode(const char *Src,
267267
size_t SrcSize) {
268268
return Base64Impl::decode(Src, SrcSize);
269-
}
269+
}

‎llvm/lib/Support/PropertySetIO.cpp‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ PropertySetRegistry::read(const MemoryBuffer *Buf) {
3636
PropertySet *CurPropSet = nullptr;
3737

3838
for (line_iterator LI(*Buf); !LI.is_at_end(); LI++) {
39-
// see if this line starts a new property set
39+
// See if this line starts a new property set
4040
if (LI->starts_with("[")) {
41-
// yes - parse the category (property name)
41+
// Parse the category (property name)
4242
auto EndPos = LI->rfind(']');
4343
if (EndPos == StringRef::npos)
4444
return makeError("invalid line: " + *LI);
@@ -48,7 +48,7 @@ PropertySetRegistry::read(const MemoryBuffer *Buf) {
4848
}
4949
if (!CurPropSet)
5050
return makeError("property category missing");
51-
// parse name and type+value
51+
// Parse name and type+value
5252
auto Parts = LI->split('=');
5353

5454
if (Parts.first.empty() || Parts.second.empty())
@@ -59,7 +59,7 @@ PropertySetRegistry::read(const MemoryBuffer *Buf) {
5959
return makeError("invalid property value: " + Parts.second);
6060
APInt Tint;
6161

62-
// parse type
62+
// Parse type
6363
if (TypeVal.first.getAsInteger(10, Tint))
6464
return makeError("invalid property type: " + TypeVal.first);
6565
Expected<PropertyValue::Type> Ttag =
@@ -70,7 +70,7 @@ PropertySetRegistry::read(const MemoryBuffer *Buf) {
7070
return Ttag.takeError();
7171
PropertyValue Prop(Ttag.get());
7272

73-
// parse value depending on its type
73+
// Parse value depending on its type
7474
switch (Ttag.get()) {
7575
case PropertyValue::Type::UINT32: {
7676
APInt ValV;
@@ -101,9 +101,9 @@ PropertySetRegistry::read(const MemoryBuffer *Buf) {
101101
}
102102

103103
namespace llvm {
104-
// output a property to a stream
104+
// Output a property to a stream
105105
raw_ostream &operator<<(raw_ostream &Out, const PropertyValue &Prop) {
106-
Out << static_cast<int>(Prop.getType()) << "|";
106+
Out << static_cast<int>(Prop.getType()) << '|';
107107
switch (Prop.getType()) {
108108
case PropertyValue::Type::UINT32:
109109
Out << Prop.asUint32();
@@ -123,11 +123,10 @@ raw_ostream &operator<<(raw_ostream &Out, const PropertyValue &Prop) {
123123

124124
void PropertySetRegistry::write(raw_ostream &Out) const {
125125
for (const auto &PropSet : PropSetMap) {
126-
Out << "[" << PropSet.first << "]\n";
126+
Out << '[' << PropSet.first << "]\n";
127127

128-
for (const auto &Props : PropSet.second) {
129-
Out << Props.first << "=" << Props.second << "\n";
130-
}
128+
for (const auto &Props : PropSet.second)
129+
Out << Props.first << '=' << Props.second << '\n';
131130
}
132131
}
133132

@@ -142,9 +141,8 @@ template <> PropertyValue::Type PropertyValue::getTypeTag<byte *>() {
142141
return BYTE_ARRAY;
143142
}
144143

145-
PropertyValue::PropertyValue(const byte *Data, SizeTy DataBitSize) {
144+
PropertyValue::PropertyValue(const byte *Data, SizeTy DataBitSize) : Ty(BYTE_ARRAY) {
146145
constexpr int ByteSizeInBits = 8;
147-
Ty = BYTE_ARRAY;
148146
SizeTy DataSize = (DataBitSize + (ByteSizeInBits - 1)) / ByteSizeInBits;
149147
constexpr size_t SizeFieldSize = sizeof(SizeTy);
150148

‎llvm/unittests/Support/PropertySetIOTest.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//===- llvm/unittest/Support/PropertySetIO.cpp - Property set I/O tests ---===//
1+
//=- llvm/unittest/Support/PropertySetIOTest.cpp - Property set I/O tests ---=//
22
//
33
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
44
// See https://llvm.org/LICENSE.txt for license information.
@@ -40,7 +40,7 @@ TEST(PropertySet, IntValuesIO) {
4040
PropSetsPtr->get()->write(OS);
4141
}
4242
// Check that the original and the serialized version are equal
43-
ASSERT_EQ(Serialized, Content);
43+
EXPECT_EQ(Serialized, Content);
4444
}
4545

4646
TEST(PropertySet, ByteArrayValuesIO) {
@@ -67,6 +67,6 @@ TEST(PropertySet, ByteArrayValuesIO) {
6767
PropSetsPtr->get()->write(OS);
6868
}
6969
// Check that the original and the serialized version are equal
70-
ASSERT_EQ(Serialized, Content);
70+
EXPECT_EQ(Serialized, Content);
7171
}
7272
} // namespace

0 commit comments

Comments
 (0)