Skip to content

Commit 7f16c26

Browse files
sbenzaquencopybara-github
authored andcommitted
Hide the synthetic MapEntry class in the pb.cc file.
This reduces the cost of including the headers. COPYBARA_INTEGRATE_REVIEW=#28033 from yashanil98:fix/issue-10486-fatal-warnings-encode-decode d0bb423 FUTURE_COPYBARA_INTEGRATE_REVIEW=#28033 from yashanil98:fix/issue-10486-fatal-warnings-encode-decode d0bb423 PiperOrigin-RevId: 964912794
1 parent dc32c23 commit 7f16c26

7 files changed

Lines changed: 53 additions & 44 deletions

File tree

src/google/protobuf/compiler/command_line_interface.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3284,7 +3284,9 @@ bool CommandLineInterface::EncodeOrDecode(const DescriptorPool* pool) {
32843284
}
32853285
}
32863286

3287+
bool found_warning = false;
32873288
if (!message->IsInitialized()) {
3289+
found_warning = true;
32883290
std::cerr << "warning: Input message is missing required fields: "
32893291
<< message->InitializationErrorString() << std::endl;
32903292
}
@@ -3305,7 +3307,9 @@ bool CommandLineInterface::EncodeOrDecode(const DescriptorPool* pool) {
33053307
}
33063308
}
33073309

3308-
return true;
3310+
// Treat warnings as fatal when --fatal_warnings is set, after the output has
3311+
// been written so the behavior matches the compile path.
3312+
return !(fatal_warnings_ && found_warning);
33093313
}
33103314

33113315
bool CommandLineInterface::WriteDescriptorSet(

src/google/protobuf/compiler/command_line_interface_unittest.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6103,6 +6103,17 @@ TEST_P(EncodeDecodeTest, Partial) {
61036103
ExpectWarning("warning: Input message is missing required fields: a, b, c");
61046104
}
61056105

6106+
TEST_P(EncodeDecodeTest, PartialWithFatalWarnings) {
6107+
RedirectStdinFromText("");
6108+
// With --fatal_warnings, the missing-required-fields warning must make protoc
6109+
// exit non-zero, while still emitting the warning and the output.
6110+
EXPECT_FALSE(
6111+
Run("google/protobuf/unittest.proto"
6112+
" --encode=proto2_unittest.TestRequired --fatal_warnings"));
6113+
ExpectStdoutMatchesText("");
6114+
ExpectWarning("warning: Input message is missing required fields: a, b, c");
6115+
}
6116+
61066117
TEST_P(EncodeDecodeTest, DecodeRaw) {
61076118
proto2_unittest::TestAllTypes message;
61086119
message.set_optional_int32(123);

src/google/protobuf/compiler/cpp/field_generators/map_field.cc

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ std::vector<Sub> Vars(const FieldDescriptor* field, const Options& opts,
5555
return {
5656
{"Map", absl::Substitute("::$2::Map<$0, $1>", key_type, val_type,
5757
ProtobufNamespace(opts))},
58-
{"Entry", ClassName(field->message_type(), false)},
5958
{"Key", PrimitiveTypeName(opts, key->cpp_type())},
6059
{"Val", val_type},
6160
{"MapField", lite ? "MapFieldLite" : "MapField"},
@@ -194,13 +193,16 @@ void Map::GeneratePrivateMembers(io::Printer* p) const {
194193
$pbi$::MapFieldLite<$Key$, $Val$> $name$_;
195194
)cc");
196195
} else {
197-
p->Emit({{"kKeyType",
198-
absl::AsciiStrToUpper(DeclaredTypeMethodName(key_->type()))},
199-
{"kValType",
200-
absl::AsciiStrToUpper(DeclaredTypeMethodName(val_->type()))}},
201-
R"cc(
202-
$pbi$::$MapField$<$Entry$, $Key$, $Val$> $name$_;
203-
)cc");
196+
p->Emit(
197+
{{"globals", MsgGlobalsInstanceName(field_->message_type(), options_)},
198+
{"kKeyType",
199+
absl::AsciiStrToUpper(DeclaredTypeMethodName(key_->type()))},
200+
{"kValType",
201+
absl::AsciiStrToUpper(DeclaredTypeMethodName(val_->type()))}},
202+
R"cc(
203+
$pbi$::$MapField$<static_cast<const void*>(&$globals$), $Key$, $Val$>
204+
$name$_;
205+
)cc");
204206
}
205207
}
206208

src/google/protobuf/compiler/cpp/message.cc

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,9 +1414,6 @@ void MessageGenerator::GenerateMapEntryClassDefinition(io::Printer* p) {
14141414
const $pbi$::ClassData* $nonnull$
14151415
class_data);
14161416
explicit $Msg$($pb$::Arena* $nullable$ arena);
1417-
static constexpr const void* $nonnull$ internal_message_globals() {
1418-
return &$globals$;
1419-
}
14201417
14211418
$decl_verify_func$;
14221419
@@ -1731,7 +1728,6 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* p) {
17311728
Formatter format(p);
17321729

17331730
if (IsMapEntryMessage(descriptor_)) {
1734-
GenerateMapEntryClassDefinition(p);
17351731
return;
17361732
}
17371733

@@ -5335,7 +5331,9 @@ void MessageGenerator::GenerateSourceDefaultInstance(io::Printer* p) {
53355331
auto v = p->WithVars(ClassVars(descriptor_, options_));
53365332
auto t = p->WithVars(MakeTrackerCalls(descriptor_, options_));
53375333

5338-
if (!IsMapEntryMessage(descriptor_)) {
5334+
if (IsMapEntryMessage(descriptor_)) {
5335+
GenerateMapEntryClassDefinition(p);
5336+
} else {
53395337
p->Emit(
53405338
{{"has_bit",
53415339
[&] {

src/google/protobuf/map.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,6 @@ template <typename Key, typename T>
7474
class MapFieldLite;
7575
class MapFieldBase;
7676

77-
template <typename Derived, typename Key, typename T>
78-
class MapField;
79-
8077
struct MapTestPeer;
8178
struct MapBenchmarkPeer;
8279

src/google/protobuf/map_field.h

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ class TypeDefinedMapFieldBase : public MapFieldBase {
607607
// This class provides access to map field using generated api. It is used for
608608
// internal generated message implementation only. Users should never use this
609609
// directly.
610-
template <typename Derived, typename Key, typename T>
610+
template <const void* kGlobals, typename Key, typename T>
611611
class PROTOBUF_FUTURE_ADD_EARLY_WARN_UNUSED MapField final
612612
: public TypeDefinedMapFieldBase<Key, T> {
613613
public:
@@ -625,14 +625,12 @@ class PROTOBUF_FUTURE_ADD_EARLY_WARN_UNUSED MapField final
625625
MapField(InternalVisibility, InternalMetadataOffset offset, Arena* arena,
626626
const MapField& from)
627627
: TypeDefinedMapFieldBase<Key, T>(
628-
MessageGlobalsBase::ToDefaultInstance<Message>(
629-
Derived::internal_message_globals()),
630-
offset, arena, from) {}
628+
MessageGlobalsBase::ToDefaultInstance<Message>(kGlobals), offset,
629+
arena, from) {}
631630

632631
private:
633632
explicit constexpr MapField(InternalMetadataOffset offset)
634-
: MapField::TypeDefinedMapFieldBase(Derived::internal_message_globals(),
635-
offset) {}
633+
: MapField::TypeDefinedMapFieldBase(kGlobals, offset) {}
636634

637635
typedef void InternalArenaConstructable_;
638636
typedef void DestructorSkippable_;
@@ -651,23 +649,23 @@ bool AllAreInitialized(const TypeDefinedMapFieldBase<Key, T>& field) {
651649
return true;
652650
}
653651

654-
template <typename Derived, typename Key, typename T>
655-
using MapFieldWithArena = FieldWithArena<MapField<Derived, Key, T>>;
652+
template <const void* kGlobals, typename Key, typename T>
653+
using MapFieldWithArena = FieldWithArena<MapField<kGlobals, Key, T>>;
656654

657-
template <typename Derived, typename Key, typename T>
658-
struct FieldArenaRep<MapField<Derived, Key, T>> {
659-
using Type = MapFieldWithArena<Derived, Key, T>;
655+
template <const void* kGlobals, typename Key, typename T>
656+
struct FieldArenaRep<MapField<kGlobals, Key, T>> {
657+
using Type = MapFieldWithArena<kGlobals, Key, T>;
660658

661-
static MapField<Derived, Key, T>* Get(Type* arena_rep) {
659+
static MapField<kGlobals, Key, T>* Get(Type* arena_rep) {
662660
return &arena_rep->field();
663661
}
664662
};
665663

666-
template <typename Derived, typename Key, typename T>
667-
struct FieldArenaRep<const MapField<Derived, Key, T>> {
668-
using Type = const MapFieldWithArena<Derived, Key, T>;
664+
template <const void* kGlobals, typename Key, typename T>
665+
struct FieldArenaRep<const MapField<kGlobals, Key, T>> {
666+
using Type = const MapFieldWithArena<kGlobals, Key, T>;
669667

670-
static const MapField<Derived, Key, T>* Get(Type* arena_rep) {
668+
static const MapField<kGlobals, Key, T>* Get(Type* arena_rep) {
671669
return &arena_rep->field();
672670
}
673671
};
@@ -748,7 +746,7 @@ class PROTOBUF_EXPORT MapValueConstRef {
748746
FieldDescriptor::CppType type_;
749747

750748
private:
751-
template <typename Derived, typename K, typename V>
749+
template <const void* kGlobals, typename K, typename V>
752750
friend class internal::MapField;
753751
template <typename K, typename V>
754752
friend class internal::TypeDefinedMapFieldBase;
@@ -852,7 +850,7 @@ class PROTOBUF_EXPORT MapIteratorBase {
852850
protected:
853851
template <typename Key, typename T>
854852
friend class internal::TypeDefinedMapFieldBase;
855-
template <typename Derived, typename Key, typename T>
853+
template <const void* kGlobals, typename Key, typename T>
856854
friend class internal::MapField;
857855
friend class internal::MapFieldBase;
858856

src/google/protobuf/map_field_test.cc

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,15 @@ struct MapFieldTestPeer {
4444
}
4545
};
4646

47-
using TestMapField = ::google::protobuf::internal::MapField<
48-
proto2_unittest::TestMap_MapInt32Int32Entry_DoNotUse, ::int32_t, ::int32_t>;
47+
constexpr const void* kEntryGlobals = static_cast<const void*>(
48+
&proto2_unittest::TestMap_MapInt32Int32Entry_DoNotUse_globals_);
49+
50+
using TestMapField =
51+
::google::protobuf::internal::MapField<kEntryGlobals, ::int32_t, ::int32_t>;
4952

5053
class MapFieldBasePrimitiveTest : public testing::TestWithParam<bool> {
5154
protected:
52-
typedef proto2_unittest::TestMap_MapInt32Int32Entry_DoNotUse EntryType;
53-
typedef MapField<EntryType, int32_t, int32_t> MapFieldType;
55+
typedef MapField<kEntryGlobals, int32_t, int32_t> MapFieldType;
5456

5557
MapFieldBasePrimitiveTest()
5658
: arena_(GetParam() ? new Arena() : nullptr),
@@ -166,8 +168,7 @@ enum State { CLEAN, MAP_DIRTY, REPEATED_DIRTY };
166168
class MapFieldStateTest
167169
: public testing::TestWithParam<std::tuple<State, bool>> {
168170
protected:
169-
typedef proto2_unittest::TestMap_MapInt32Int32Entry_DoNotUse EntryType;
170-
typedef MapField<EntryType, int32_t, int32_t> MapFieldType;
171+
typedef MapField<kEntryGlobals, int32_t, int32_t> MapFieldType;
171172
MapFieldStateTest()
172173
: arena_(std::get<1>(GetParam()) ? new Arena() : nullptr),
173174
map_field_(arena_.get()),
@@ -436,9 +437,7 @@ TEST_P(MapFieldStateTest, MutableMapField) {
436437
}
437438
}
438439

439-
using MyMapField =
440-
MapField<proto2_unittest::TestMap_MapInt32Int32Entry_DoNotUse, int32_t,
441-
int32_t>;
440+
using MyMapField = MapField<kEntryGlobals, int32_t, int32_t>;
442441

443442
TEST(MapFieldTest, ConstInit) {
444443
// This tests that `MapField` and all its base classes can be constant

0 commit comments

Comments
 (0)