From 541b442b99e1e9a1c514e2aacfe1e83561a9ab68 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 15 Jul 2015 13:36:56 +0100 Subject: Don't create nested types (or field accessors) for map types. I'm sure I've implemented this before, but somehow it's been lost in a maze of twisty little branches, all alike. --- .../protobuf/compiler/csharp/csharp_helpers.h | 6 +++++ .../protobuf/compiler/csharp/csharp_message.cc | 27 ++++++++++++++++++++-- .../protobuf/compiler/csharp/csharp_message.h | 2 ++ 3 files changed, 33 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/google/protobuf/compiler/csharp/csharp_helpers.h b/src/google/protobuf/compiler/csharp/csharp_helpers.h index bd3d6e7d..a7c2395b 100644 --- a/src/google/protobuf/compiler/csharp/csharp_helpers.h +++ b/src/google/protobuf/compiler/csharp/csharp_helpers.h @@ -105,6 +105,12 @@ uint FixedMakeTag(const FieldDescriptor* descriptor); FieldGeneratorBase* CreateFieldGenerator(const FieldDescriptor* descriptor, int fieldOrdinal); +// Determines whether the given message is a map entry message, i.e. one implicitly created +// by protoc due to a map field. +inline bool IsMapEntryMessage(const Descriptor* descriptor) { + return descriptor->options().map_entry(); +} + // Determines whether we're generating code for the proto representation of descriptors etc, // for use in the runtime. This is the only type which is allowed to use proto2 syntax, // and it generates internal classes. diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index 4acc899b..5a72b2e7 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -121,6 +121,9 @@ void MessageGenerator::GenerateStaticVariables(io::Printer* printer) { "full_class_name", full_class_name()); for (int i = 0; i < descriptor_->nested_type_count(); i++) { + // Don't generate accessor table fields for maps... + if (IsMapEntryMessage(descriptor_->nested_type(i))) continue; + MessageGenerator messageGenerator(descriptor_->nested_type(i)); messageGenerator.GenerateStaticVariables(printer); } @@ -159,8 +162,10 @@ void MessageGenerator::GenerateStaticVariableInitializers(io::Printer* printer) } printer->Print("});\n"); - // Generate static member initializers for all nested types. + // Generate static member initializers for all non-map-entry nested types. for (int i = 0; i < descriptor_->nested_type_count(); i++) { + if (IsMapEntryMessage(descriptor_->nested_type(i))) continue; + MessageGenerator messageGenerator(descriptor_->nested_type(i)); messageGenerator.GenerateStaticVariableInitializers(printer); } @@ -285,7 +290,7 @@ void MessageGenerator::Generate(io::Printer* printer) { GenerateMergingMethods(printer); // Nested messages and enums - if (descriptor_->enum_type_count() + descriptor_->nested_type_count() > 0) { + if (HasNestedGeneratedTypes()) { printer->Print("#region Nested types\n" "[global::System.Diagnostics.DebuggerNonUserCodeAttribute()]\n"); WriteGeneratedCodeAttributes(printer); @@ -296,6 +301,9 @@ void MessageGenerator::Generate(io::Printer* printer) { enumGenerator.Generate(printer); } for (int i = 0; i < descriptor_->nested_type_count(); i++) { + // Don't generate nested types for maps... + if (IsMapEntryMessage(descriptor_->nested_type(i))) continue; + MessageGenerator messageGenerator(descriptor_->nested_type(i)); messageGenerator.Generate(printer); } @@ -310,6 +318,21 @@ void MessageGenerator::Generate(io::Printer* printer) { printer->Print("\n"); } +// Helper to work out whether we need to generate a class to hold nested types/enums. +// Only tricky because we don't want to generate map entry types. +bool MessageGenerator::HasNestedGeneratedTypes() +{ + if (descriptor_->enum_type_count() > 0) { + return true; + } + for (int i = 0; i < descriptor_->nested_type_count(); i++) { + if (!IsMapEntryMessage(descriptor_->nested_type(i))) { + return true; + } + } + return false; +} + void MessageGenerator::GenerateCloningCode(io::Printer* printer) { map vars; vars["class_name"] = class_name(); diff --git a/src/google/protobuf/compiler/csharp/csharp_message.h b/src/google/protobuf/compiler/csharp/csharp_message.h index 6c7153aa..fbe8a3be 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.h +++ b/src/google/protobuf/compiler/csharp/csharp_message.h @@ -69,6 +69,8 @@ class MessageGenerator : public SourceGeneratorBase { FieldGeneratorBase* CreateFieldGeneratorInternal( const FieldDescriptor* descriptor); + bool HasNestedGeneratedTypes(); + std::string class_name(); std::string full_class_name(); -- cgit v1.2.3 From db52c9dd585da7d5c9063409117b8a5680e384db Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 15 Jul 2015 22:03:38 +0100 Subject: Address requested change from code review. --- .../protobuf/compiler/csharp/csharp_message.cc | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index 5a72b2e7..79b762d1 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -122,10 +122,10 @@ void MessageGenerator::GenerateStaticVariables(io::Printer* printer) { for (int i = 0; i < descriptor_->nested_type_count(); i++) { // Don't generate accessor table fields for maps... - if (IsMapEntryMessage(descriptor_->nested_type(i))) continue; - - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.GenerateStaticVariables(printer); + if (!IsMapEntryMessage(descriptor_->nested_type(i))) { + MessageGenerator messageGenerator(descriptor_->nested_type(i)); + messageGenerator.GenerateStaticVariables(printer); + } } } @@ -164,10 +164,10 @@ void MessageGenerator::GenerateStaticVariableInitializers(io::Printer* printer) // Generate static member initializers for all non-map-entry nested types. for (int i = 0; i < descriptor_->nested_type_count(); i++) { - if (IsMapEntryMessage(descriptor_->nested_type(i))) continue; - - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.GenerateStaticVariableInitializers(printer); + if (!IsMapEntryMessage(descriptor_->nested_type(i))) { + MessageGenerator messageGenerator(descriptor_->nested_type(i)); + messageGenerator.GenerateStaticVariableInitializers(printer); + } } } @@ -302,10 +302,10 @@ void MessageGenerator::Generate(io::Printer* printer) { } for (int i = 0; i < descriptor_->nested_type_count(); i++) { // Don't generate nested types for maps... - if (IsMapEntryMessage(descriptor_->nested_type(i))) continue; - - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.Generate(printer); + if (!IsMapEntryMessage(descriptor_->nested_type(i))) { + MessageGenerator messageGenerator(descriptor_->nested_type(i)); + messageGenerator.Generate(printer); + } } printer->Outdent(); printer->Print("}\n" -- cgit v1.2.3