Merge pull request #566 from jskeet/csharp-reflection

Improve C# reflection support

12 files changed, 61 insertions, 41 deletions

diff --git a/src/google/protobuf/compiler/csharp/csharp_field_base.cc b/src/google/protobuf/compiler/csharp/csharp_field_base.cc
index bfb39a64..ae7336ca 100644
--- a/src/google/protobuf/compiler/csharp/csharp_field_base.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_field_base.cc
@@ -65,6 +65,7 @@ void FieldGeneratorBase::SetCommonFieldVariables(
     tag_bytes += ", " + SimpleItoa(tag_array[i]);
   }
 
+  (*variables)["access_level"] = class_access_level();
   (*variables)["tag"] = SimpleItoa(tag);
   (*variables)["tag_size"] = SimpleItoa(tag_size);
   (*variables)["tag_bytes"] = tag_bytes;
diff --git a/src/google/protobuf/compiler/csharp/csharp_generator.cc b/src/google/protobuf/compiler/csharp/csharp_generator.cc
index 2a416fac..f06d79d8 100644
--- a/src/google/protobuf/compiler/csharp/csharp_generator.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_generator.cc
@@ -68,6 +68,12 @@ bool Generator::Generate(
   vector<pair<string, string> > options;
   ParseGeneratorParameter(parameter, &options);
 
+  // We only support proto3 - but we make an exception for descriptor.proto.
+  if (file->syntax() != FileDescriptor::SYNTAX_PROTO3 && !IsDescriptorProto(file)) {
+    *error = "C# code generation only supports proto3 syntax";
+    return false;
+  }
+
   std::string file_extension = ".cs";
   for (int i = 0; i < options.size(); i++) {
     if (options[i].first == "file_extension") {
diff --git a/src/google/protobuf/compiler/csharp/csharp_helpers.h b/src/google/protobuf/compiler/csharp/csharp_helpers.h
index a8c1fa18..d9576fbd 100644
--- a/src/google/protobuf/compiler/csharp/csharp_helpers.h
+++ b/src/google/protobuf/compiler/csharp/csharp_helpers.h
@@ -101,6 +101,15 @@ uint FixedMakeTag(const FieldDescriptor* descriptor);
 
 FieldGeneratorBase* CreateFieldGenerator(const FieldDescriptor* descriptor, int fieldOrdinal);
 
+// 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.
+inline bool IsDescriptorProto(const FileDescriptor* descriptor) {
+  // TODO: Do this better! (Currently this depends on a hack in generate_protos.sh to rename
+  // the file...)
+  return descriptor->name() == "google/protobuf/descriptor_proto_file.proto";
+}
+
 }  // namespace csharp
 }  // namespace compiler
 }  // namespace protobuf
diff --git a/src/google/protobuf/compiler/csharp/csharp_map_field.cc b/src/google/protobuf/compiler/csharp/csharp_map_field.cc
index b8f1592c..cb7ce5f0 100644
--- a/src/google/protobuf/compiler/csharp/csharp_map_field.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_map_field.cc
@@ -78,7 +78,7 @@ void MapFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public pbc::MapField<$key_type_name$, $value_type_name$> $property_name$ {\n"
+    "$access_level$ pbc::MapField<$key_type_name$, $value_type_name$> $property_name$ {\n"
     "  get { return $name$_; }\n"
     "}\n");
 }
diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc
index 9e2fe9b6..13544b26 100644
--- a/src/google/protobuf/compiler/csharp/csharp_message.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_message.cc
@@ -116,8 +116,7 @@ void MessageGenerator::GenerateStaticVariables(io::Printer* printer) {
 
   // The descriptor for this type.
   printer->Print(
-      "internal static pbd::MessageDescriptor internal__$identifier$__Descriptor;\n"
-      "internal static pb::FieldAccess.FieldAccessorTable<$full_class_name$> internal__$identifier$__FieldAccessorTable;\n",
+      "internal static pb::FieldAccess.FieldAccessorTable internal__$identifier$__FieldAccessorTable;\n",
       "identifier", GetUniqueFileScopeIdentifier(descriptor_),
       "full_class_name", full_class_name());
 
@@ -130,24 +129,23 @@ void MessageGenerator::GenerateStaticVariables(io::Printer* printer) {
 void MessageGenerator::GenerateStaticVariableInitializers(io::Printer* printer) {
   map<string, string> vars;
   vars["identifier"] = GetUniqueFileScopeIdentifier(descriptor_);
-  vars["index"] = SimpleItoa(descriptor_->index());
   vars["full_class_name"] = full_class_name();
-  if (descriptor_->containing_type() != NULL) {
-    vars["parent"] = GetUniqueFileScopeIdentifier(
-	descriptor_->containing_type());
-  }
-  printer->Print(vars, "internal__$identifier$__Descriptor = ");
 
-  if (!descriptor_->containing_type()) {
-    printer->Print(vars, "Descriptor.MessageTypes[$index$];\n");
-  } else {
-    printer->Print(vars, "internal__$parent$__Descriptor.NestedTypes[$index$];\n");
+  // Work out how to get to the message descriptor (which may be multiply nested) from the file
+  // descriptor.
+  string descriptor_chain;
+  const Descriptor* current_descriptor = descriptor_;
+  while (current_descriptor->containing_type()) {
+    descriptor_chain = ".NestedTypes[" + SimpleItoa(current_descriptor->index()) + "]" + descriptor_chain;
+    current_descriptor = current_descriptor->containing_type();
   }
+  descriptor_chain = "descriptor.MessageTypes[" + SimpleItoa(current_descriptor->index()) + "]" + descriptor_chain;
+  vars["descriptor_chain"] = descriptor_chain;
 
   printer->Print(
     vars,
     "internal__$identifier$__FieldAccessorTable = \n"
-    "    new pb::FieldAccess.FieldAccessorTable<$full_class_name$>(internal__$identifier$__Descriptor,\n");
+    "    new pb::FieldAccess.FieldAccessorTable(typeof($full_class_name$), $descriptor_chain$,\n");
   printer->Print("        new string[] { ");
   for (int i = 0; i < descriptor_->field_count(); i++) {
     printer->Print("\"$property_name$\", ",
@@ -201,23 +199,35 @@ void MessageGenerator::Generate(io::Printer* printer) {
     "private static readonly uint[] _fieldTags = new uint[] { $tags$ };\n",
     "tags", JoinStrings(tags, ", "));
 
+  // Access the message descriptor via the relevant file descriptor or containing message descriptor.
+  if (!descriptor_->containing_type()) {
+    vars["descriptor_accessor"] = GetFullUmbrellaClassName(descriptor_->file())
+        + ".Descriptor.MessageTypes[" + SimpleItoa(descriptor_->index()) + "]";
+  } else {
+    vars["descriptor_accessor"] = GetClassName(descriptor_->containing_type())
+        + ".Descriptor.NestedTypes[" + SimpleItoa(descriptor_->index()) + "]";
+  }
+
   printer->Print(
     vars,
     "public static pbd::MessageDescriptor Descriptor {\n"
-    "  get { return $umbrella_class_name$.internal__$identifier$__Descriptor; }\n"
+    "  get { return $descriptor_accessor$; }\n"
     "}\n"
     "\n"
-    "public pb::FieldAccess.FieldAccessorTable<$class_name$> Fields {\n"
+    "public pb::FieldAccess.FieldAccessorTable Fields {\n"
     "  get { return $umbrella_class_name$.internal__$identifier$__FieldAccessorTable; }\n"
     "}\n"
     "\n"
     "private bool _frozen = false;\n"
     "public bool IsFrozen { get { return _frozen; } }\n\n");
 
-  // Parameterless constructor
+  // Parameterless constructor and partial OnConstruction method.
   printer->Print(
     vars,
-    "public $class_name$() { }\n\n");
+    "public $class_name$() {\n"
+    "  OnConstruction();\n"
+    "}\n\n"
+    "partial void OnConstruction();\n\n");
 
   GenerateCloningCode(printer);
   GenerateFreezingCode(printer);
@@ -304,7 +314,7 @@ void MessageGenerator::GenerateCloningCode(io::Printer* printer) {
   vars["class_name"] = class_name();
     printer->Print(
     vars,
-    "public $class_name$($class_name$ other) {\n");
+    "public $class_name$($class_name$ other) : this() {\n");
   printer->Indent();
   // Clone non-oneof fields first
   for (int i = 0; i < descriptor_->field_count(); i++) {
diff --git a/src/google/protobuf/compiler/csharp/csharp_message_field.cc b/src/google/protobuf/compiler/csharp/csharp_message_field.cc
index d8c82271..d2c3a88b 100644
--- a/src/google/protobuf/compiler/csharp/csharp_message_field.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_message_field.cc
@@ -64,7 +64,7 @@ void MessageFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public $type_name$ $property_name$ {\n"
+    "$access_level$ $type_name$ $property_name$ {\n"
     "  get { return $name$_; }\n"
     "  set {\n"
     "    pb::Freezable.CheckMutable(this);\n"
@@ -158,7 +158,7 @@ void MessageOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public $type_name$ $property_name$ {\n"
+    "$access_level$ $type_name$ $property_name$ {\n"
     "  get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : null; }\n"
     "  set {\n"
     "    pb::Freezable.CheckMutable(this);\n"
diff --git a/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc b/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc
index c40cba13..4454ef02 100644
--- a/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc
@@ -71,7 +71,7 @@ void PrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public $type_name$ $property_name$ {\n"
+    "$access_level$ $type_name$ $property_name$ {\n"
     "  get { return $name$_; }\n"
     "  set {\n"
     "    pb::Freezable.CheckMutable(this);\n");
@@ -174,7 +174,7 @@ void PrimitiveOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public $type_name$ $property_name$ {\n"
+    "$access_level$ $type_name$ $property_name$ {\n"
     "  get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : $default_value$; }\n"
     "  set {\n"
     "    pb::Freezable.CheckMutable(this);\n");
diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc
index 4445c281..d5fc6d98 100644
--- a/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc
@@ -65,7 +65,7 @@ void RepeatedEnumFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public pbc::RepeatedField<$type_name$> $property_name$ {\n"
+    "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n"
     "  get { return $name$_; }\n"
     "}\n");
 }
diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc
index c74e5d0d..6d4e6984 100644
--- a/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc
@@ -65,7 +65,7 @@ void RepeatedMessageFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public pbc::RepeatedField<$type_name$> $property_name$ {\n"
+    "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n"
     "  get { return $name$_; }\n"
     "}\n");
 }
diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc
index a3014da8..5b5d9b3d 100644
--- a/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc
@@ -65,7 +65,7 @@ void RepeatedPrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) {
   AddDeprecatedFlag(printer);
   printer->Print(
     variables_,
-    "public pbc::RepeatedField<$type_name$> $property_name$ {\n"
+    "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n"
     "  get { return $name$_; }\n"
     "}\n");
 }
diff --git a/src/google/protobuf/compiler/csharp/csharp_source_generator_base.cc b/src/google/protobuf/compiler/csharp/csharp_source_generator_base.cc
index c666c820..735d164a 100644
--- a/src/google/protobuf/compiler/csharp/csharp_source_generator_base.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_source_generator_base.cc
@@ -57,7 +57,7 @@ void SourceGeneratorBase::WriteGeneratedCodeAttributes(io::Printer* printer) {
 }
 
 std::string SourceGeneratorBase::class_access_level() {
-  return "public";  // public_classes is always on.
+  return IsDescriptorProto(descriptor_) ? "internal" : "public";  // public_classes is always on.
 }
 
 }  // namespace csharp
diff --git a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc
index 03a3b8df..8fbd3e9a 100644
--- a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc
@@ -176,22 +176,11 @@ void UmbrellaClassGenerator::WriteDescriptor(io::Printer* printer) {
   printer->Print("\"$base64$\"));\n", "base64", base64);
   printer->Outdent();
   printer->Outdent();
-  printer->Print(
-    "pbd::FileDescriptor.InternalDescriptorAssigner assigner = delegate(pbd::FileDescriptor root) {\n");
-  printer->Indent();
-  printer->Print("descriptor = root;\n");
-  for (int i = 0; i < file_->message_type_count(); i++) {
-    MessageGenerator messageGenerator(file_->message_type(i));
-    messageGenerator.GenerateStaticVariableInitializers(printer);
-  }
-
-  printer->Outdent();
-  printer->Print("};\n");
 
   // -----------------------------------------------------------------
-  // Invoke internalBuildGeneratedFileFrom() to build the file.
+  // Invoke InternalBuildGeneratedFileFrom() to build the file.
   printer->Print(
-      "pbd::FileDescriptor.InternalBuildGeneratedFileFrom(descriptorData,\n");
+      "descriptor = pbd::FileDescriptor.InternalBuildGeneratedFileFrom(descriptorData,\n");
   printer->Print("    new pbd::FileDescriptor[] {\n");
   for (int i = 0; i < file_->dependency_count(); i++) {
     printer->Print(
@@ -199,7 +188,12 @@ void UmbrellaClassGenerator::WriteDescriptor(io::Printer* printer) {
       "full_umbrella_class_name",
       GetFullUmbrellaClassName(file_->dependency(i)));
   }
-  printer->Print("    }, assigner);\n");
+  printer->Print("    });\n");
+  // Then invoke any other static variable initializers, e.g. field accessors.
+  for (int i = 0; i < file_->message_type_count(); i++) {
+      MessageGenerator messageGenerator(file_->message_type(i));
+      messageGenerator.GenerateStaticVariableInitializers(printer);
+  }
   printer->Outdent();
   printer->Print("}\n");
   printer->Print("#endregion\n\n");