Skip to content

Commit d170d28

Browse files
ckennellycopybara-github
ckennelly
authored and
copybara-github
committed
Add [[nodiscard]] to many APIs.
This covers two types of failures: * Methods that are logically const and failure to consume the result indicates a bug (an unnecessary call, etc.) * Methods that return significant errors (failure to parse, etc.) that should not be unintentionally ignored. PiperOrigin-RevId: 842042770
1 parent 0702b26 commit d170d28

39 files changed

+2644
-1519
lines changed

‎benchmarks/benchmark.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,10 @@ BENCHMARK_TEMPLATE(BM_Parse_Proto2, FileDescSV, InitBlock, Alias);
328328

329329
static void BM_SerializeDescriptor_Proto2(benchmark::State& state) {
330330
upb_benchmark::FileDescriptorProto proto;
331-
proto.ParseFromString(absl::string_view(descriptor.data, descriptor.size));
331+
(void)proto.ParseFromString(
332+
absl::string_view(descriptor.data, descriptor.size));
332333
for (auto _ : state) {
333-
proto.SerializePartialToArray(buf, sizeof(buf));
334+
(void)proto.SerializePartialToArray(buf, sizeof(buf));
334335
benchmark::DoNotOptimize(buf);
335336
}
336337
state.SetBytesProcessed(state.iterations() * descriptor.size);
@@ -420,7 +421,7 @@ BENCHMARK(BM_JsonParse_Upb);
420421
static void BM_JsonParse_Proto2(benchmark::State& state) {
421422
protobuf::FileDescriptorProto proto;
422423
absl::string_view input(descriptor.data, descriptor.size);
423-
proto.ParseFromString(input);
424+
(void)proto.ParseFromString(input);
424425
std::string json;
425426
ABSL_CHECK_OK(google::protobuf::json::MessageToJsonString(proto, &json));
426427
for (auto _ : state) {
@@ -461,7 +462,7 @@ BENCHMARK(BM_JsonSerialize_Upb);
461462
static void BM_JsonSerialize_Proto2(benchmark::State& state) {
462463
protobuf::FileDescriptorProto proto;
463464
absl::string_view input(descriptor.data, descriptor.size);
464-
proto.ParseFromString(input);
465+
(void)proto.ParseFromString(input);
465466
std::string json;
466467
for (auto _ : state) {
467468
json.clear();

‎benchmarks/gen_protobuf_binary_cc.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ def RefMessage(name):
5454
print('''
5555
{{
5656
{name} proto;
57-
proto.ParseFromArray(buf, 0);
58-
proto.SerializePartialToArray(&buf[0], 0);
57+
(void)proto.ParseFromArray(buf, 0);
58+
(void)proto.SerializePartialToArray(&buf[0], 0);
5959
}}
6060
'''.format(name=name))
6161

‎lua/upbc.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ bool LuaGenerator::Generate(const protobuf::FileDescriptor* file,
9999
protobuf::FileDescriptorProto file_proto;
100100
file->CopyTo(&file_proto);
101101
std::string file_data;
102-
file_proto.SerializeToString(&file_data);
102+
// TODO: Remove this suppression.
103+
(void)file_proto.SerializeToString(&file_data);
103104

104105
printer.Print("local descriptor = table.concat({\n");
105106
absl::string_view data(file_data);

‎python/google/protobuf/proto_api.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ PythonMessageMutator::~PythonMessageMutator() {
4646
// check.
4747
if (!PyErr_Occurred() && owned_msg_ != nullptr) {
4848
std::string wire;
49-
message_->SerializePartialToString(&wire);
49+
// TODO: Remove this suppression.
50+
(void)message_->SerializePartialToString(&wire);
5051
PyObject* py_wire = PyBytes_FromStringAndSize(
5152
wire.data(), static_cast<Py_ssize_t>(wire.size()));
5253
PyObject* parse =
@@ -113,20 +114,23 @@ bool PythonConstMessagePointer::NotChanged() {
113114
// serialize result may still diff between languages. So parse to
114115
// another c++ message for compare.
115116
std::unique_ptr<google::protobuf::Message> parsed_msg(owned_msg_->New());
116-
parsed_msg->ParsePartialFromString(
117+
// TODO: Remove this suppression.
118+
(void)parsed_msg->ParsePartialFromString(
117119
absl::string_view(data, static_cast<int>(len)));
118120
std::string wire_other;
119121
google::protobuf::io::StringOutputStream stream_other(&wire_other);
120122
google::protobuf::io::CodedOutputStream output_other(&stream_other);
121123
output_other.SetSerializationDeterministic(true);
122-
parsed_msg->SerializePartialToCodedStream(&output_other);
124+
// TODO: Remove this suppression.
125+
(void)parsed_msg->SerializePartialToCodedStream(&output_other);
123126
output_other.Trim();
124127

125128
std::string wire;
126129
google::protobuf::io::StringOutputStream stream(&wire);
127130
google::protobuf::io::CodedOutputStream output(&stream);
128131
output.SetSerializationDeterministic(true);
129-
owned_msg_->SerializePartialToCodedStream(&output);
132+
// TODO: Remove this suppression.
133+
(void)owned_msg_->SerializePartialToCodedStream(&output);
130134
output.Trim();
131135

132136
if (wire == wire_other) {

‎python/google/protobuf/pyext/descriptor.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ bool Reparse(PyMessageFactory* message_factory, const Message& from,
228228
Message* to) {
229229
// Reparse message.
230230
std::string serialized;
231-
from.SerializeToString(&serialized);
231+
// TODO: Remove this suppression.
232+
(void)from.SerializeToString(&serialized);
232233
io::CodedInputStream input(
233234
reinterpret_cast<const uint8_t*>(serialized.c_str()), serialized.size());
234235
input.SetExtensionRegistry(message_factory->pool->pool,
@@ -1490,7 +1491,8 @@ static PyObject* GetSerializedPb(PyFileDescriptor* self, void* closure) {
14901491
FileDescriptorProto file_proto;
14911492
_GetDescriptor(self)->CopyTo(&file_proto);
14921493
std::string contents;
1493-
file_proto.SerializePartialToString(&contents);
1494+
// TODO: Remove this suppression.
1495+
(void)file_proto.SerializePartialToString(&contents);
14941496
self->serialized_pb = PyBytes_FromStringAndSize(
14951497
contents.c_str(), static_cast<size_t>(contents.size()));
14961498
if (self->serialized_pb == nullptr) {

‎python/google/protobuf/pyext/message_module.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,10 @@ absl::StatusOr<google::protobuf::Message*> CreateNewMessage(PyObject* py_msg) {
285285
bool CopyToOwnedMsg(google::protobuf::Message** copy, const google::protobuf::Message& message) {
286286
*copy = message.New();
287287
std::string wire;
288-
message.SerializePartialToString(&wire);
289-
(*copy)->ParsePartialFromString(wire);
288+
// TODO: Remove this suppression.
289+
(void)message.SerializePartialToString(&wire);
290+
// TODO: Remove this suppression.
291+
(void)(*copy)->ParsePartialFromString(wire);
290292
return true;
291293
}
292294

‎src/google/protobuf/compiler/cpp/enum.cc‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,12 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
204204

205205
if (has_reflection_) {
206206
p->Emit(R"(
207-
$dllexport_decl $const $pb$::EnumDescriptor* $nonnull$ $Msg_Enum$_descriptor();
207+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD $dllexport_decl $const $pb$::EnumDescriptor* $nonnull$ $Msg_Enum$_descriptor();
208208
)");
209209
} else {
210210
p->Emit(R"cc(
211-
$return_type$ $Msg_Enum$_Name($Msg_Enum$ value);
211+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD $return_type$
212+
$Msg_Enum$_Name($Msg_Enum$ value);
212213
)cc");
213214
}
214215

@@ -230,7 +231,8 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
230231
if (should_cache_ || !has_reflection_) {
231232
p->Emit({{"static_assert", write_assert}}, R"cc(
232233
template <typename T>
233-
$return_type$ $Msg_Enum$_Name(T value) {
234+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD $return_type$
235+
$Msg_Enum$_Name(T value) {
234236
$static_assert$;
235237
return $Msg_Enum$_Name(static_cast<$Msg_Enum$>(value));
236238
}
@@ -242,7 +244,8 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
242244
// pointers, so if the enum values are sparse, it's not worth it.
243245
p->Emit(R"cc(
244246
template <>
245-
inline $return_type$ $Msg_Enum$_Name($Msg_Enum$ value) {
247+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD inline $return_type$
248+
$Msg_Enum$_Name($Msg_Enum$ value) {
246249
return $pbi$::NameOfDenseEnum<$Msg_Enum$_descriptor, $kMin$, $kMax$>(
247250
static_cast<int>(value));
248251
}
@@ -251,7 +254,8 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
251254
} else {
252255
p->Emit({{"static_assert", write_assert}}, R"cc(
253256
template <typename T>
254-
$return_type$ $Msg_Enum$_Name(T value) {
257+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD $return_type$
258+
$Msg_Enum$_Name(T value) {
255259
$static_assert$;
256260
return $pbi$::NameOfEnum($Msg_Enum$_descriptor(), value);
257261
}
@@ -260,7 +264,7 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
260264

261265
if (has_reflection_) {
262266
p->Emit(R"cc(
263-
inline bool $Msg_Enum$_Parse(
267+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD inline bool $Msg_Enum$_Parse(
264268
//~
265269
::absl::string_view name, $Msg_Enum$* $nonnull$ value) {
266270
return $pbi$::ParseNamedEnum<$Msg_Enum$>($Msg_Enum$_descriptor(), name,
@@ -269,7 +273,7 @@ void EnumGenerator::GenerateDefinition(io::Printer* p) {
269273
)cc");
270274
} else {
271275
p->Emit(R"cc(
272-
bool $Msg_Enum$_Parse(
276+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD bool $Msg_Enum$_Parse(
273277
//~
274278
::absl::string_view name, $Msg_Enum$* $nonnull$ value);
275279
)cc");
@@ -323,7 +327,8 @@ void EnumGenerator::GenerateSymbolImports(io::Printer* p) const {
323327
.AnnotatedAs(enum_),
324328
},
325329
R"cc(
326-
static inline bool $Enum$_IsValid(int value) {
330+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD static inline bool $Enum$_IsValid(
331+
int value) {
327332
return $Msg_Enum$_IsValid(value);
328333
}
329334
static constexpr $Enum_$ $Enum_MIN$ = $Msg_Enum$_$Enum$_MIN;
@@ -351,10 +356,11 @@ void EnumGenerator::GenerateSymbolImports(io::Printer* p) const {
351356

352357
p->Emit(R"cc(
353358
template <typename T>
354-
static inline $return_type$ $Enum$_Name(T value) {
359+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD static inline $return_type$ $Enum$_Name(
360+
T value) {
355361
return $Msg_Enum$_Name(value);
356362
}
357-
static inline bool $Enum$_Parse(
363+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD static inline bool $Enum$_Parse(
358364
//~
359365
::absl::string_view name, $Enum_$* $nonnull$ value) {
360366
return $Msg_Enum$_Parse(name, value);
@@ -373,31 +379,36 @@ void EnumGenerator::GenerateIsValid(io::Printer* p) const {
373379
static_cast<int64_t>(sorted_unique_values_.size()) - 1 ==
374380
sorted_unique_values_.back()) {
375381
// They are sequential. Do a simple range check.
376-
p->Emit({{"min", sorted_unique_values_.front()},
377-
{"max", sorted_unique_values_.back()}},
378-
R"cc(
379-
inline bool $Msg_Enum$_IsValid(int value) {
380-
return $min$ <= value && value <= $max$;
381-
}
382-
)cc");
382+
p->Emit(
383+
{{"min", sorted_unique_values_.front()},
384+
{"max", sorted_unique_values_.back()}},
385+
R"cc(
386+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD inline bool $Msg_Enum$_IsValid(
387+
int value) {
388+
return $min$ <= value && value <= $max$;
389+
}
390+
)cc");
383391
} else if (sorted_unique_values_.front() >= 0 &&
384392
sorted_unique_values_.back() < 64) {
385393
// Not sequential, but they fit in a 64-bit bitmap.
386394
uint64_t bitmap = 0;
387395
for (int n : sorted_unique_values_) {
388396
bitmap |= uint64_t{1} << n;
389397
}
390-
p->Emit({{"bitmap", bitmap}, {"max", sorted_unique_values_.back()}},
391-
R"cc(
392-
inline bool $Msg_Enum$_IsValid(int value) {
393-
return 0 <= value && value <= $max$ && (($bitmap$u >> value) & 1) != 0;
394-
}
395-
)cc");
398+
p->Emit(
399+
{{"bitmap", bitmap}, {"max", sorted_unique_values_.back()}},
400+
R"cc(
401+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD inline bool $Msg_Enum$_IsValid(
402+
int value) {
403+
return 0 <= value && value <= $max$ && (($bitmap$u >> value) & 1) != 0;
404+
}
405+
)cc");
396406
} else {
397407
// More complex struct. Use enum data structure for lookup.
398408
p->Emit(
399409
R"cc(
400-
inline bool $Msg_Enum$_IsValid(int value) {
410+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD inline bool $Msg_Enum$_IsValid(
411+
int value) {
401412
return $pbi$::ValidateEnum(value, $Msg_Enum$_internal_data_);
402413
}
403414
)cc");
@@ -409,7 +420,8 @@ void EnumGenerator::GenerateMethods(int idx, io::Printer* p) {
409420

410421
if (has_reflection_) {
411422
p->Emit({{"idx", idx}}, R"cc(
412-
const $pb$::EnumDescriptor* $nonnull$ $Msg_Enum$_descriptor() {
423+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD const $pb$::EnumDescriptor* $nonnull$
424+
$Msg_Enum$_descriptor() {
413425
$pbi$::AssignDescriptors(&$desc_table$);
414426
return $file_level_enum_descriptors$[$idx$];
415427
}
@@ -541,7 +553,8 @@ void EnumGenerator::GenerateMethods(int idx, io::Printer* p) {
541553
$entries_by_number$,
542554
};
543555
544-
$return_type$ $Msg_Enum$_Name($Msg_Enum$ value) {
556+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD $return_type$
557+
$Msg_Enum$_Name($Msg_Enum$ value) {
545558
static const bool kDummy = $pbi$::InitializeEnumStrings(
546559
$Msg_Enum$_entries, $Msg_Enum$_entries_by_number, $num_unique$,
547560
$Msg_Enum$_strings);
@@ -553,7 +566,8 @@ void EnumGenerator::GenerateMethods(int idx, io::Printer* p) {
553566
return idx == -1 ? $pbi$::GetEmptyString() : $Msg_Enum$_strings[idx].get();
554567
}
555568
556-
bool $Msg_Enum$_Parse(::absl::string_view name, $Msg_Enum$* $nonnull$ value) {
569+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD bool $Msg_Enum$_Parse(
570+
::absl::string_view name, $Msg_Enum$* $nonnull$ value) {
557571
int int_value;
558572
bool success = $pbi$::LookUpEnumValue(
559573
$Msg_Enum$_entries, $num_declared$, name, &int_value);

0 commit comments

Comments
 (0)