Skip to content

Commit a70115f

Browse files
ckennellycopybara-github
authored andcommitted
Breaking change: 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: 852313694
1 parent 3acf23c commit a70115f

File tree

75 files changed

+1720
-1182
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+1720
-1182
lines changed

benchmarks/benchmark.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ static void BM_LoadAdsDescriptor_Proto2(benchmark::State& state) {
180180
printf("Failed to find descriptor.\n");
181181
exit(1);
182182
}
183-
factory.GetPrototype(d);
183+
(void)factory.GetPrototype(d);
184184
}
185185
}
186186
state.SetBytesProcessed(state.iterations() * bytes_per_iter);

conformance/binary_json_conformance_suite.cc

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -935,7 +935,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForType(
935935
MessageType test_message;
936936
ABSL_CHECK(test_message.MergeFromString(expected_proto));
937937
std::string text;
938-
TextFormat::PrintToString(test_message, &text);
938+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
939939

940940
RunValidProtobufTest(
941941
absl::StrCat("ValidDataScalar", type_name, "[", i, "]"), REQUIRED,
@@ -958,7 +958,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForType(
958958
MessageType test_message;
959959
ABSL_CHECK(test_message.MergeFromString(expected_proto));
960960
std::string text;
961-
TextFormat::PrintToString(test_message, &text);
961+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
962962

963963
RunValidProtobufTest(absl::StrCat("RepeatedScalarSelectsLast", type_name),
964964
REQUIRED, proto, text);
@@ -1019,7 +1019,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForType(
10191019
MessageType test_message;
10201020
ABSL_CHECK(test_message.MergeFromString(default_proto_packed_expected));
10211021
std::string text;
1022-
TextFormat::PrintToString(test_message, &text);
1022+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
10231023

10241024
// Ensures both packed and unpacked data can be parsed.
10251025
RunValidProtobufTest(
@@ -1070,7 +1070,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForType(
10701070
MessageType test_message;
10711071
ABSL_CHECK(test_message.MergeFromString(expected_proto));
10721072
std::string text;
1073-
TextFormat::PrintToString(test_message, &text);
1073+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
10741074

10751075
RunValidProtobufTest(absl::StrCat("ValidDataRepeated", type_name), REQUIRED,
10761076
proto, text);
@@ -1150,7 +1150,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
11501150
MessageType test_message;
11511151
ABSL_CHECK(test_message.MergeFromString(proto));
11521152
std::string text;
1153-
TextFormat::PrintToString(test_message, &text);
1153+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
11541154
RunValidProtobufTest(absl::StrCat("ValidDataMap", key_type_name,
11551155
value_type_name, ".Default"),
11561156
REQUIRED, proto, text);
@@ -1164,7 +1164,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
11641164
MessageType test_message;
11651165
ABSL_CHECK(test_message.MergeFromString(proto));
11661166
std::string text;
1167-
TextFormat::PrintToString(test_message, &text);
1167+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
11681168
RunValidProtobufTest(absl::StrCat("ValidDataMap", key_type_name,
11691169
value_type_name, ".MissingDefault"),
11701170
REQUIRED, proto, text);
@@ -1178,7 +1178,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
11781178
MessageType test_message;
11791179
ABSL_CHECK(test_message.MergeFromString(proto));
11801180
std::string text;
1181-
TextFormat::PrintToString(test_message, &text);
1181+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
11821182
RunValidProtobufTest(absl::StrCat("ValidDataMap", key_type_name,
11831183
value_type_name, ".NonDefault"),
11841184
REQUIRED, proto, text);
@@ -1192,7 +1192,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
11921192
MessageType test_message;
11931193
ABSL_CHECK(test_message.MergeFromString(proto));
11941194
std::string text;
1195-
TextFormat::PrintToString(test_message, &text);
1195+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
11961196
RunValidProtobufTest(absl::StrCat("ValidDataMap", key_type_name,
11971197
value_type_name, ".Unordered"),
11981198
REQUIRED, proto, text);
@@ -1210,7 +1210,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
12101210
MessageType test_message;
12111211
ABSL_CHECK(test_message.MergeFromString(proto2));
12121212
std::string text;
1213-
TextFormat::PrintToString(test_message, &text);
1213+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
12141214
RunValidProtobufTest(absl::StrCat("ValidDataMap", key_type_name,
12151215
value_type_name, ".DuplicateKey"),
12161216
REQUIRED, proto, text);
@@ -1224,7 +1224,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
12241224
MessageType test_message;
12251225
ABSL_CHECK(test_message.MergeFromString(proto));
12261226
std::string text;
1227-
TextFormat::PrintToString(test_message, &text);
1227+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
12281228
RunValidProtobufTest(
12291229
absl::StrCat("ValidDataMap", key_type_name, value_type_name,
12301230
".DuplicateKeyInMapEntry"),
@@ -1239,7 +1239,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForMapType(
12391239
MessageType test_message;
12401240
ABSL_CHECK(test_message.MergeFromString(proto));
12411241
std::string text;
1242-
TextFormat::PrintToString(test_message, &text);
1242+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
12431243
RunValidProtobufTest(
12441244
absl::StrCat("ValidDataMap", key_type_name, value_type_name,
12451245
".DuplicateValueInMapEntry"),
@@ -1282,7 +1282,7 @@ void BinaryAndJsonConformanceSuiteImpl<
12821282
MessageType test_message;
12831283
ABSL_CHECK(test_message.MergeFromString(proto2));
12841284
std::string text;
1285-
TextFormat::PrintToString(test_message, &text);
1285+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
12861286
RunValidProtobufTest("ValidDataMap.STRING.MESSAGE.MergeValue", REQUIRED,
12871287
proto, text);
12881288
}
@@ -1307,7 +1307,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForOneofType(
13071307
MessageType test_message;
13081308
ABSL_CHECK(test_message.MergeFromString(proto));
13091309
std::string text;
1310-
TextFormat::PrintToString(test_message, &text);
1310+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
13111311

13121312
RunValidProtobufTest(
13131313
absl::StrCat("ValidDataOneof", type_name, ".DefaultValue"), REQUIRED,
@@ -1323,7 +1323,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForOneofType(
13231323
MessageType test_message;
13241324
ABSL_CHECK(test_message.MergeFromString(proto));
13251325
std::string text;
1326-
TextFormat::PrintToString(test_message, &text);
1326+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
13271327

13281328
RunValidProtobufTest(
13291329
absl::StrCat("ValidDataOneof", type_name, ".NonDefaultValue"), REQUIRED,
@@ -1340,7 +1340,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForOneofType(
13401340
MessageType test_message;
13411341
ABSL_CHECK(test_message.MergeFromString(expected_proto));
13421342
std::string text;
1343-
TextFormat::PrintToString(test_message, &text);
1343+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
13441344

13451345
RunValidProtobufTest(absl::StrCat("ValidDataOneof", type_name,
13461346
".MultipleValuesForSameField"),
@@ -1366,7 +1366,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestValidDataForOneofType(
13661366
MessageType test_message;
13671367
ABSL_CHECK(test_message.MergeFromString(expected_proto));
13681368
std::string text;
1369-
TextFormat::PrintToString(test_message, &text);
1369+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
13701370

13711371
RunValidProtobufTest(absl::StrCat("ValidDataOneof", type_name,
13721372
".MultipleValuesForDifferentField"),
@@ -1415,7 +1415,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::TestMergeOneofMessage() {
14151415
MessageType test_message;
14161416
ABSL_CHECK(test_message.MergeFromString(expected_proto));
14171417
std::string text;
1418-
TextFormat::PrintToString(test_message, &text);
1418+
ABSL_CHECK(TextFormat::PrintToString(test_message, &text));
14191419
RunValidProtobufTest("ValidDataOneof.MESSAGE.Merge", REQUIRED, proto, text);
14201420
RunValidBinaryProtobufTest("ValidDataOneofBinary.MESSAGE.Merge", RECOMMENDED,
14211421
proto, expected_proto);

conformance/testee_test.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ using ::testing::Return;
2626
MATCHER_P(RequestEquals, expected_textproto, "") {
2727
::conformance::ConformanceRequest request, expected;
2828
ABSL_CHECK(request.ParseFromString(arg));
29-
TextFormat::ParseFromString(expected_textproto, &expected);
29+
ABSL_CHECK(TextFormat::ParseFromString(expected_textproto, &expected));
3030
if (!util::MessageDifferencer::Equals(request, expected)) {
3131
std::string request_string;
32-
TextFormat::PrintToString(request, &request_string);
32+
ABSL_CHECK(TextFormat::PrintToString(request, &request_string));
3333
*result_listener << "with equivalent text format:\n" << request_string;
3434
return false;
3535
}
@@ -38,7 +38,7 @@ MATCHER_P(RequestEquals, expected_textproto, "") {
3838

3939
auto RespondWith(absl::string_view textproto) {
4040
::conformance::ConformanceResponse response;
41-
TextFormat::ParseFromString(textproto, &response);
41+
ABSL_CHECK(TextFormat::ParseFromString(textproto, &response));
4242
return Return(response.SerializeAsString());
4343
}
4444

editions/defaults_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ class OverridableDefaultsTest : public ::testing::Test {
252252
static void SetUpTestSuite() {
253253
google::protobuf::LinkExtensionReflection(pb::cpp);
254254
google::protobuf::LinkExtensionReflection(pb::java);
255-
DescriptorPool::generated_pool();
255+
(void)DescriptorPool::generated_pool();
256256
}
257257
};
258258

python/google/protobuf/pyext/map_container.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,6 @@ int MapReflectionFriend::MessageMapSetItem(PyObject* _self, PyObject* key,
628628
const Reflection* reflection = message->GetReflection();
629629
std::string map_key_string;
630630
MapKey map_key;
631-
MapValueRef value;
632631

633632
self->version++;
634633

src/google/protobuf/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2552,6 +2552,7 @@ cc_library(
25522552
deps = [
25532553
":protobuf",
25542554
":protobuf_lite",
2555+
"@abseil-cpp//absl/log:absl_check",
25552556
],
25562557
)
25572558

src/google/protobuf/any.h

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,12 @@ PROTOBUF_EXPORT extern const char kTypeGoogleApisComPrefix[];
3232
// "type.googleprod.com/".
3333
PROTOBUF_EXPORT extern const char kTypeGoogleProdComPrefix[];
3434

35+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
3536
std::string GetTypeUrl(absl::string_view message_name,
3637
absl::string_view type_url_prefix);
3738

3839
template <typename T>
39-
absl::string_view GetAnyMessageName() {
40+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD absl::string_view GetAnyMessageName() {
4041
return T::FullMessageName();
4142
}
4243

@@ -45,25 +46,30 @@ absl::string_view GetAnyMessageName() {
4546
#define VALUE_TYPE std::string
4647

4748
// Helper functions that only require 'lite' messages to work.
49+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
4850
PROTOBUF_EXPORT bool InternalPackFromLite(
4951
const MessageLite& message, absl::string_view type_url_prefix,
5052
absl::string_view type_name, URL_TYPE* PROTOBUF_NONNULL dst_url,
5153
VALUE_TYPE* PROTOBUF_NONNULL dst_value);
54+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
5255
PROTOBUF_EXPORT bool InternalUnpackToLite(
5356
absl::string_view type_name, absl::string_view type_url,
5457
const VALUE_TYPE& value, MessageLite* PROTOBUF_NONNULL dst_message);
58+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
5559
PROTOBUF_EXPORT bool InternalIsLite(absl::string_view type_name,
5660
absl::string_view type_url);
5761

5862
// Packs a message using the default type URL prefix: "type.googleapis.com".
5963
// The resulted type URL will be "type.googleapis.com/<message_full_name>".
6064
// Returns false if serializing the message failed.
6165
template <typename T>
62-
bool InternalPackFrom(const T& message, URL_TYPE* PROTOBUF_NONNULL dst_url,
63-
VALUE_TYPE* PROTOBUF_NONNULL dst_value) {
66+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD bool InternalPackFrom(
67+
const T& message, URL_TYPE* PROTOBUF_NONNULL dst_url,
68+
VALUE_TYPE* PROTOBUF_NONNULL dst_value) {
6469
return InternalPackFromLite(message, kTypeGoogleApisComPrefix,
6570
GetAnyMessageName<T>(), dst_url, dst_value);
6671
}
72+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
6773
PROTOBUF_EXPORT bool InternalPackFrom(const Message& message,
6874
URL_TYPE* PROTOBUF_NONNULL dst_url,
6975
VALUE_TYPE* PROTOBUF_NONNULL dst_value);
@@ -76,12 +82,14 @@ PROTOBUF_EXPORT bool InternalPackFrom(const Message& message,
7682
// URL: "type.googleapis.com/<message_full_name>".
7783
// Returns false if serializing the message failed.
7884
template <typename T>
79-
bool InternalPackFrom(const T& message, absl::string_view type_url_prefix,
80-
URL_TYPE* PROTOBUF_NONNULL dst_url,
81-
VALUE_TYPE* PROTOBUF_NONNULL dst_value) {
85+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD bool InternalPackFrom(
86+
const T& message, absl::string_view type_url_prefix,
87+
URL_TYPE* PROTOBUF_NONNULL dst_url,
88+
VALUE_TYPE* PROTOBUF_NONNULL dst_value) {
8289
return InternalPackFromLite(message, type_url_prefix, GetAnyMessageName<T>(),
8390
dst_url, dst_value);
8491
}
92+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
8593
PROTOBUF_EXPORT bool InternalPackFrom(const Message& message,
8694
absl::string_view type_url_prefix,
8795
URL_TYPE* PROTOBUF_NONNULL dst_url,
@@ -92,10 +100,12 @@ PROTOBUF_EXPORT bool InternalPackFrom(const Message& message,
92100
// name after the last "/" of the type URL doesn't match the message's actual
93101
// full name) or parsing the payload has failed.
94102
template <typename T>
95-
bool InternalUnpackTo(absl::string_view type_url, const VALUE_TYPE& value,
96-
T* PROTOBUF_NONNULL message) {
103+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD bool InternalUnpackTo(
104+
absl::string_view type_url, const VALUE_TYPE& value,
105+
T* PROTOBUF_NONNULL message) {
97106
return InternalUnpackToLite(GetAnyMessageName<T>(), type_url, value, message);
98107
}
108+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
99109
PROTOBUF_EXPORT bool InternalUnpackTo(absl::string_view type_url,
100110
const VALUE_TYPE& value,
101111
Message* PROTOBUF_NONNULL message);
@@ -104,7 +114,8 @@ PROTOBUF_EXPORT bool InternalUnpackTo(absl::string_view type_url,
104114
// A type is considered matching if its full name matches the full name after
105115
// the last "/" in the type URL.
106116
template <typename T>
107-
bool InternalIs(absl::string_view type_url) {
117+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD bool InternalIs(
118+
absl::string_view type_url) {
108119
return InternalIsLite(GetAnyMessageName<T>(), type_url);
109120
}
110121

@@ -118,6 +129,7 @@ bool InternalIs(absl::string_view type_url) {
118129
//
119130
// NOTE: this function is available publicly as a static method on the
120131
// generated message type: google::protobuf::Any::ParseAnyTypeUrl()
132+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
121133
bool ParseAnyTypeUrl(absl::string_view type_url,
122134
std::string* PROTOBUF_NONNULL full_type_name);
123135

@@ -126,12 +138,14 @@ bool ParseAnyTypeUrl(absl::string_view type_url,
126138
// "type.googleapis.com/" in *url_prefix and "rpc.QueryOrigin" in
127139
// *full_type_name. Returns false if the type_url does not have a "/" in the
128140
// type url separating the full type name.
141+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
129142
bool ParseAnyTypeUrl(absl::string_view type_url,
130143
std::string* PROTOBUF_NULLABLE url_prefix,
131144
std::string* PROTOBUF_NONNULL full_type_name);
132145

133146
// See if message is of type google.protobuf.Any, if so, return the descriptors
134147
// for "type_url" and "value" fields.
148+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD
135149
bool GetAnyFieldDescriptors(
136150
const Message& message,
137151
const FieldDescriptor* PROTOBUF_NULLABLE* PROTOBUF_NONNULL type_url_field,

src/google/protobuf/arena.h

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,9 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
258258
// Allocates an object type T if the arena passed in is not nullptr;
259259
// otherwise, returns a heap-allocated object.
260260
template <typename T, typename... Args>
261-
PROTOBUF_NDEBUG_INLINE static T* PROTOBUF_NONNULL
262-
Create(Arena* PROTOBUF_NULLABLE arena, Args&&... args) {
261+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD PROTOBUF_NDEBUG_INLINE static T*
262+
PROTOBUF_NONNULL
263+
Create(Arena* PROTOBUF_NULLABLE arena, Args&&... args) {
263264
if constexpr (is_arena_constructable<T>::value) {
264265
using Type = std::remove_const_t<T>;
265266
// DefaultConstruct/CopyConstruct are optimized for messages, which
@@ -297,7 +298,8 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
297298
}
298299

299300
// Allocates memory with the specific size and alignment.
300-
void* PROTOBUF_NONNULL AllocateAligned(size_t size, size_t align = 8) {
301+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD void* PROTOBUF_NONNULL
302+
AllocateAligned(size_t size, size_t align = 8) {
301303
if (align <= internal::ArenaAlignDefault::align) {
302304
return Allocate(internal::ArenaAlignDefault::Ceil(size));
303305
} else {
@@ -318,8 +320,9 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
318320
// (when compiled as C++11) that T is trivially default-constructible and
319321
// trivially destructible.
320322
template <typename T>
321-
PROTOBUF_NDEBUG_INLINE static T* PROTOBUF_NONNULL
322-
CreateArray(Arena* PROTOBUF_NULLABLE arena, size_t num_elements) {
323+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD PROTOBUF_NDEBUG_INLINE static T*
324+
PROTOBUF_NONNULL
325+
CreateArray(Arena* PROTOBUF_NULLABLE arena, size_t num_elements) {
323326
static_assert(std::is_trivially_default_constructible<T>::value,
324327
"CreateArray requires a trivially constructible type");
325328
static_assert(std::is_trivially_destructible<T>::value,
@@ -343,15 +346,19 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
343346
// deal. For instance allocated space depends on growth policies. Do not use
344347
// these in unit tests. Returns the total space allocated by the arena, which
345348
// is the sum of the sizes of the underlying blocks.
346-
uint64_t SpaceAllocated() const { return impl_.SpaceAllocated(); }
349+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD uint64_t SpaceAllocated() const {
350+
return impl_.SpaceAllocated();
351+
}
347352
// Returns the total space used by the arena. Similar to SpaceAllocated but
348353
// does not include free space and block overhead. This is a best-effort
349354
// estimate and may inaccurately calculate space used by other threads
350355
// executing concurrently with the call to this method. These inaccuracies
351356
// are due to race conditions, and are bounded but unpredictable. Stale data
352357
// can lead to underestimates of the space used, and race conditions can lead
353358
// to overestimates (up to the current block size).
354-
uint64_t SpaceUsed() const { return impl_.SpaceUsed(); }
359+
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD uint64_t SpaceUsed() const {
360+
return impl_.SpaceUsed();
361+
}
355362

356363
// Frees all storage allocated by this arena after calling destructors
357364
// registered with OwnDestructor() and freeing objects registered with Own().

0 commit comments

Comments
 (0)