From d846b0b059b4d867536b98aa29475a387aa09114 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 8 Jun 2015 16:24:57 -0400 Subject: Beta quality drop of Objective C Support. - Add more to the ObjC dir readme. - Merge the ExtensionField and ExtensionDescriptor to reduce overhead. - Fix an initialization race. - Clean up the Xcode schemes. - Remove the class/enum filter. - Remove some forced inline that were bloating things without proof of performance wins. - Rename some internal types to avoid conflicts with the well know types protos. - Drop the use of ApplyFunctions to the compiler/optimizer can do what it wants. - Better document some possible future improvements. - Add missing support for parsing repeated primitive fields in packed or unpacked forms. - Improve -hash. - Add *Count for repeated and map<> fields to avoid auto create when checking for them being set. --- src/Makefile.am | 2 +- .../compiler/objectivec/objectivec_enum_field.cc | 12 +- .../compiler/objectivec/objectivec_extension.cc | 39 +-- .../compiler/objectivec/objectivec_extension.h | 6 +- .../compiler/objectivec/objectivec_field.cc | 91 ++--- .../compiler/objectivec/objectivec_file.cc | 293 +++++++--------- .../protobuf/compiler/objectivec/objectivec_file.h | 4 - .../compiler/objectivec/objectivec_generator.cc | 4 - .../compiler/objectivec/objectivec_helpers.cc | 63 +--- .../compiler/objectivec/objectivec_helpers.h | 5 +- .../objectivec/objectivec_helpers_unittest.cc | 8 + .../compiler/objectivec/objectivec_message.cc | 369 ++++++++++----------- .../compiler/objectivec/objectivec_message.h | 11 +- .../objectivec/objectivec_message_field.cc | 2 +- 14 files changed, 355 insertions(+), 554 deletions(-) (limited to 'src') diff --git a/src/Makefile.am b/src/Makefile.am index 33894dc1..6affcbf4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -147,7 +147,7 @@ nobase_include_HEADERS = \ google/protobuf/compiler/java/java_names.h \ google/protobuf/compiler/javanano/javanano_generator.h \ google/protobuf/compiler/objectivec/objectivec_generator.h \ - google/protobuf/compiler/objectivec/objectivec_helpers.h \ + google/protobuf/compiler/objectivec/objectivec_helpers.h \ google/protobuf/compiler/python/python_generator.h \ google/protobuf/compiler/ruby/ruby_generator.h \ google/protobuf/compiler/csharp/csharp_generator.h diff --git a/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc index d6609692..30a13ddb 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc @@ -55,8 +55,6 @@ void SetEnumVariables(const FieldDescriptor* descriptor, (descriptor->file() != descriptor->enum_type()->file())) { (*variables)["property_type"] = "enum " + type; } - // TODO(thomasvl): Make inclusion of descriptor compile time and output - // both of these. Note: Extensions currently have to have the EnumDescription. (*variables)["enum_verifier"] = type + "_IsValidValue"; (*variables)["enum_desc_func"] = type + "_EnumDescriptor"; @@ -74,11 +72,9 @@ EnumFieldGenerator::~EnumFieldGenerator() {} void EnumFieldGenerator::GenerateFieldDescriptionTypeSpecific( io::Printer* printer) const { - // TODO(thomasvl): Output the CPP check to use descFunc or validator based - // on final compile. printer->Print( variables_, - " .typeSpecific.enumDescFunc = $enum_desc_func$,\n"); + " .dataTypeSpecific.enumDescFunc = $enum_desc_func$,\n"); } void EnumFieldGenerator::GenerateCFunctionDeclarations( @@ -103,7 +99,7 @@ void EnumFieldGenerator::GenerateCFunctionImplementations( "int32_t $owning_message_class$_$capitalized_name$_RawValue($owning_message_class$ *message) {\n" " GPBDescriptor *descriptor = [$owning_message_class$ descriptor];\n" " GPBFieldDescriptor *field = [descriptor fieldWithNumber:$field_number_name$];\n" - " return GPBGetInt32IvarWithField(message, field);\n" + " return GPBGetMessageInt32Field(message, field);\n" "}\n" "\n" "void Set$owning_message_class$_$capitalized_name$_RawValue($owning_message_class$ *message, int32_t value) {\n" @@ -137,11 +133,9 @@ RepeatedEnumFieldGenerator::~RepeatedEnumFieldGenerator() {} void RepeatedEnumFieldGenerator::GenerateFieldDescriptionTypeSpecific( io::Printer* printer) const { - // TODO(thomasvl): Output the CPP check to use descFunc or validator based - // on final compile. printer->Print( variables_, - " .typeSpecific.enumDescFunc = $enum_desc_func$,\n"); + " .dataTypeSpecific.enumDescFunc = $enum_desc_func$,\n"); } } // namespace objectivec diff --git a/src/google/protobuf/compiler/objectivec/objectivec_extension.cc b/src/google/protobuf/compiler/objectivec/objectivec_extension.cc index 76137c80..4e348393 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_extension.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_extension.cc @@ -46,24 +46,6 @@ ExtensionGenerator::ExtensionGenerator(const string& root_class_name, : method_name_(ExtensionMethodName(descriptor)), root_class_and_method_name_(root_class_name + "_" + method_name_), descriptor_(descriptor) { - // Extensions can be filtered via the method they are accessed off the - // file's Root with. - if (FilterClass(root_class_and_method_name_)) { - filter_reason_ = - string("Extension |") + root_class_and_method_name_ + "| was not whitelisted."; - } else { - // Extensions that add a Message field also require that field be allowed - // by the filter, or they aren't usable. - ObjectiveCType objc_type = GetObjectiveCType(descriptor_); - if (objc_type == OBJECTIVECTYPE_MESSAGE) { - const string message_class_name(ClassName(descriptor_->message_type())); - if (FilterClass(message_class_name)) { - filter_reason_ = string("Extension |") + root_class_and_method_name_ + - "| needs message |" + message_class_name + - "|, which was not whitelisted."; - } - } - } if (descriptor->is_map()) { // NOTE: src/google/protobuf/compiler/plugin.cc makes use of cerr for some // error cases, so it seems to be ok to use as a back door for errors. @@ -77,10 +59,6 @@ ExtensionGenerator::ExtensionGenerator(const string& root_class_name, ExtensionGenerator::~ExtensionGenerator() {} void ExtensionGenerator::GenerateMembersHeader(io::Printer* printer) { - if (IsFiltered()) { - printer->Print("// $filter_reason$\n\n", "filter_reason", filter_reason_); - return; - } map vars; vars["method_name"] = method_name_; SourceLocation location; @@ -91,15 +69,11 @@ void ExtensionGenerator::GenerateMembersHeader(io::Printer* printer) { } printer->Print(vars, "$comments$" - "+ (GPBExtensionField*)$method_name$;\n"); + "+ (GPBExtensionDescriptor *)$method_name$;\n"); } void ExtensionGenerator::GenerateStaticVariablesInitialization( - io::Printer* printer, bool* out_generated, bool root) { - if (IsFiltered()) { - return; - } - *out_generated = true; + io::Printer* printer) { map vars; vars["root_class_and_method_name"] = root_class_and_method_name_; vars["extended_type"] = ClassName(descriptor_->containing_type()); @@ -122,14 +96,14 @@ void ExtensionGenerator::GenerateStaticVariablesInitialization( vars["type"] = "NULL"; } - vars["default_name"] = GPBValueFieldName(descriptor_); + vars["default_name"] = GPBGenericValueFieldName(descriptor_); if (descriptor_->is_repeated()) { vars["default"] = "nil"; } else { vars["default"] = DefaultValue(descriptor_); } string type = GetCapitalizedType(descriptor_); - vars["extension_type"] = string("GPBType") + type; + vars["extension_type"] = string("GPBDataType") + type; if (objc_type == OBJECTIVECTYPE_ENUM) { vars["enum_desc_func_name"] = @@ -141,7 +115,7 @@ void ExtensionGenerator::GenerateStaticVariablesInitialization( printer->Print(vars, "{\n" " .singletonName = GPBStringifySymbol($root_class_and_method_name$),\n" - " .type = $extension_type$,\n" + " .dataType = $extension_type$,\n" " .extendedClass = GPBStringifySymbol($extended_type$),\n" " .fieldNumber = $number$,\n" " .defaultValue.$default_name$ = $default$,\n" @@ -152,9 +126,6 @@ void ExtensionGenerator::GenerateStaticVariablesInitialization( } void ExtensionGenerator::GenerateRegistrationSource(io::Printer* printer) { - if (IsFiltered()) { - return; - } printer->Print( "[registry addExtension:$root_class_and_method_name$];\n", "root_class_and_method_name", root_class_and_method_name_); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_extension.h b/src/google/protobuf/compiler/objectivec/objectivec_extension.h index 553f0887..e361e639 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_extension.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_extension.h @@ -52,16 +52,12 @@ class ExtensionGenerator { ~ExtensionGenerator(); void GenerateMembersHeader(io::Printer* printer); - void GenerateStaticVariablesInitialization(io::Printer* printer, - bool* out_generated, bool root); + void GenerateStaticVariablesInitialization(io::Printer* printer); void GenerateRegistrationSource(io::Printer* printer); - bool IsFiltered() const { return filter_reason_.length() > 0; } - private: string method_name_; string root_class_and_method_name_; - string filter_reason_; const FieldDescriptor* descriptor_; GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ExtensionGenerator); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_field.cc index c5f05653..0f96a4e6 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_field.cc @@ -87,18 +87,16 @@ void SetCommonFieldVariables(const FieldDescriptor* descriptor, field_flags.push_back("GPBFieldHasDefaultValue"); if (needs_custom_name) field_flags.push_back("GPBFieldTextFormatNameCustom"); if (descriptor->type() == FieldDescriptor::TYPE_ENUM) { - // TODO(thomasvl): Output the CPP check to use descFunc or validator based - // on final compile. field_flags.push_back("GPBFieldHasEnumDescriptor"); } (*variables)["fieldflags"] = BuildFlagsString(field_flags); (*variables)["default"] = DefaultValue(descriptor); - (*variables)["default_name"] = GPBValueFieldName(descriptor); + (*variables)["default_name"] = GPBGenericValueFieldName(descriptor); - (*variables)["typeSpecific_name"] = "className"; - (*variables)["typeSpecific_value"] = "NULL"; + (*variables)["dataTypeSpecific_name"] = "className"; + (*variables)["dataTypeSpecific_value"] = "NULL"; string field_options = descriptor->options().SerializeAsString(); // Must convert to a standard byte order for packing length into @@ -117,45 +115,6 @@ void SetCommonFieldVariables(const FieldDescriptor* descriptor, (*variables)["storage_attribute"] = ""; } -// A field generator that writes nothing. -class EmptyFieldGenerator : public FieldGenerator { - public: - EmptyFieldGenerator(const FieldDescriptor* descriptor, const string& reason) - : FieldGenerator(descriptor), reason_(reason) {} - virtual ~EmptyFieldGenerator() {} - - virtual void GenerateFieldStorageDeclaration(io::Printer* printer) const {} - virtual void GeneratePropertyDeclaration(io::Printer* printer) const { - string name = FieldName(descriptor_); - string type; - switch (GetObjectiveCType(descriptor_)) { - case OBJECTIVECTYPE_MESSAGE: - type = ClassName(descriptor_->message_type()) + " *"; - break; - - case OBJECTIVECTYPE_ENUM: - type = EnumName(descriptor_->enum_type()) + " "; - break; - - default: - type = string(descriptor_->type_name()) + " "; - break; - } - printer->Print("// Field |$type$$name$| $reason$\n\n", "type", type, "name", - name, "reason", reason_); - } - - virtual void GenerateFieldNumberConstant(io::Printer* printer) const {} - virtual void GeneratePropertyImplementation(io::Printer* printer) const {} - virtual void GenerateFieldDescription(io::Printer* printer) const {} - - virtual bool WantsHasProperty(void) const { return false; } - - private: - string reason_; - GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(EmptyFieldGenerator); -}; - } // namespace FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field) { @@ -163,12 +122,7 @@ FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field) { if (field->is_repeated()) { switch (GetObjectiveCType(field)) { case OBJECTIVECTYPE_MESSAGE: { - string type = ClassName(field->message_type()); - if (FilterClass(type)) { - string reason = - "Filtered by |" + type + "| not being whitelisted."; - result = new EmptyFieldGenerator(field, reason); - } else if (field->is_map()) { + if (field->is_map()) { result = new MapFieldGenerator(field); } else { result = new RepeatedMessageFieldGenerator(field); @@ -185,14 +139,7 @@ FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field) { } else { switch (GetObjectiveCType(field)) { case OBJECTIVECTYPE_MESSAGE: { - string type = ClassName(field->message_type()); - if (FilterClass(type)) { - string reason = - "Filtered by |" + type + "| not being whitelisted."; - result = new EmptyFieldGenerator(field, reason); - } else { - result = new MessageFieldGenerator(field); - } + result = new MessageFieldGenerator(field); break; } case OBJECTIVECTYPE_ENUM: @@ -249,11 +196,17 @@ void FieldGenerator::GenerateFieldDescription( " .number = $field_number_name$,\n" " .hasIndex = $has_index$,\n" " .flags = $fieldflags$,\n" - " .type = GPBType$field_type$,\n" - " .offset = offsetof($classname$_Storage, $name$),\n" + " .dataType = GPBDataType$field_type$,\n" + " .offset = offsetof($classname$__storage_, $name$),\n" " .defaultValue.$default_name$ = $default$,\n"); - // " .typeSpecific.value* = [something]," + // TODO(thomasvl): It might be useful to add a CPP wrapper to support + // compiling away the EnumDescriptors. To do that, we'd need a #if here + // to control setting the descriptor vs. the validator, and above in + // SetCommonFieldVariables() we'd want to wrap how we add + // GPBFieldHasDefaultValue to the flags. + + // " .dataTypeSpecific.value* = [something]," GenerateFieldDescriptionTypeSpecific(printer); const string& field_options(variables_.find("fieldoptions")->second); @@ -280,7 +233,7 @@ void FieldGenerator::GenerateFieldDescriptionTypeSpecific( io::Printer* printer) const { printer->Print( variables_, - " .typeSpecific.$typeSpecific_name$ = $typeSpecific_value$,\n"); + " .dataTypeSpecific.$dataTypeSpecific_name$ = $dataTypeSpecific_value$,\n"); } void FieldGenerator::SetOneofIndexBase(int index_base) { @@ -410,22 +363,24 @@ void RepeatedFieldGenerator::GenerateFieldStorageDeclaration( void RepeatedFieldGenerator::GeneratePropertyImplementation( io::Printer* printer) const { - printer->Print(variables_, "@dynamic $name$;\n"); + printer->Print(variables_, "@dynamic $name$, $name$_Count;\n"); } void RepeatedFieldGenerator::GeneratePropertyDeclaration( io::Printer* printer) const { - // Repeated fields don't need the has* properties, but this has the same - // logic as ObjCObjFieldGenerator::GeneratePropertyDeclaration() for dealing - // with needing Objective C's rules around storage name conventions (init*, - // new*, etc.) + // Repeated fields don't need the has* properties, but they do expose a + // *Count (to check without autocreation). So for the field property we need + // the same logic as ObjCObjFieldGenerator::GeneratePropertyDeclaration() for + // dealing with needing Objective C's rules around storage name conventions + // (init*, new*, etc.) printer->Print( variables_, "$comments$" "$array_comment$" - "@property(nonatomic, readwrite, strong) $array_storage_type$ *$name$$storage_attribute$;\n"); + "@property(nonatomic, readwrite, strong) $array_storage_type$ *$name$$storage_attribute$;\n" + "@property(nonatomic, readonly) NSUInteger $name$_Count;\n"); if (IsInitName(variables_.find("name")->second)) { // If property name starts with init we need to annotate it to get past ARC. // http://stackoverflow.com/questions/18723226/how-do-i-annotate-an-objective-c-property-with-an-objc-method-family/18723227#18723227 diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.cc b/src/google/protobuf/compiler/objectivec/objectivec_file.cc index d04eee85..1955c053 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.cc @@ -54,35 +54,24 @@ namespace objectivec { FileGenerator::FileGenerator(const FileDescriptor *file) : file_(file), root_class_name_(FileClassName(file)), - is_filtered_(true), - all_extensions_filtered_(true), is_public_dep_(false) { - // Validate the objc prefix, do this even if the file's contents are filtered - // to catch a bad prefix as soon as it is found. + // Validate the objc prefix. ValidateObjCClassPrefix(file_); for (int i = 0; i < file_->enum_type_count(); i++) { EnumGenerator *generator = new EnumGenerator(file_->enum_type(i)); - // The enums are exposed via C functions, so they will dead strip if - // not used. - is_filtered_ &= false; enum_generators_.push_back(generator); } for (int i = 0; i < file_->message_type_count(); i++) { MessageGenerator *generator = new MessageGenerator(root_class_name_, file_->message_type(i)); - is_filtered_ &= generator->IsFiltered(); - is_filtered_ &= generator->IsSubContentFiltered(); message_generators_.push_back(generator); } for (int i = 0; i < file_->extension_count(); i++) { ExtensionGenerator *generator = new ExtensionGenerator(root_class_name_, file_->extension(i)); - is_filtered_ &= generator->IsFiltered(); - all_extensions_filtered_ &= generator->IsFiltered(); extension_generators_.push_back(generator); } - // If there is nothing in the file we filter it. } FileGenerator::~FileGenerator() { @@ -116,8 +105,7 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { "protoc_gen_objc_version", SimpleItoa(GOOGLE_PROTOBUF_OBJC_GEN_VERSION)); - const vector &dependency_generators = - DependencyGenerators(); + const vector &dependency_generators = DependencyGenerators(); for (vector::const_iterator iter = dependency_generators.begin(); iter != dependency_generators.end(); ++iter) { @@ -133,19 +121,17 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { printer->Print("CF_EXTERN_C_BEGIN\n\n"); - if (!IsFiltered()) { - set fwd_decls; - for (vector::iterator iter = message_generators_.begin(); - iter != message_generators_.end(); ++iter) { - (*iter)->DetermineForwardDeclarations(&fwd_decls); - } - for (set::const_iterator i(fwd_decls.begin()); - i != fwd_decls.end(); ++i) { - printer->Print("$value$;\n", "value", *i); - } - if (fwd_decls.begin() != fwd_decls.end()) { - printer->Print("\n"); - } + set fwd_decls; + for (vector::iterator iter = message_generators_.begin(); + iter != message_generators_.end(); ++iter) { + (*iter)->DetermineForwardDeclarations(&fwd_decls); + } + for (set::const_iterator i(fwd_decls.begin()); + i != fwd_decls.end(); ++i) { + printer->Print("$value$;\n", "value", *i); + } + if (fwd_decls.begin() != fwd_decls.end()) { + printer->Print("\n"); } // need to write out all enums first @@ -160,36 +146,27 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { } // For extensions to chain together, the Root gets created even if there - // are no extensions. So if the entire file isn't filtered away, output it. - if (!IsFiltered()) { - printer->Print( - "\n" - "#pragma mark - $root_class_name$\n" - "\n" - "@interface $root_class_name$ : GPBRootObject\n" - "\n" - "// The base class provides:\n" - "// + (GPBExtensionRegistry *)extensionRegistry;\n" - "// which is an GPBExtensionRegistry that includes all the extensions defined by\n" - "// this file and all files that it depends on.\n" - "\n" - "@end\n" - "\n", - "root_class_name", root_class_name_); - } + // are no extensions. + printer->Print( + "\n" + "#pragma mark - $root_class_name$\n" + "\n" + "@interface $root_class_name$ : GPBRootObject\n" + "\n" + "// The base class provides:\n" + "// + (GPBExtensionRegistry *)extensionRegistry;\n" + "// which is an GPBExtensionRegistry that includes all the extensions defined by\n" + "// this file and all files that it depends on.\n" + "\n" + "@end\n" + "\n", + "root_class_name", root_class_name_); if (extension_generators_.size() > 0) { - // The dynamic methods block is only needed if there are extensions. If - // they are all filtered, output the @interface as a comment so there is - // something left in the header for anyone that looks. - const char *root_line_prefix = ""; - if (AreAllExtensionsFiltered()) { - root_line_prefix = "// "; - } + // The dynamic methods block is only needed if there are extensions. printer->Print( - "$root_line_prefix$@interface $root_class_name$ (DynamicMethods)\n", - "root_class_name", root_class_name_, - "root_line_prefix", root_line_prefix); + "@interface $root_class_name$ (DynamicMethods)\n", + "root_class_name", root_class_name_); for (vector::iterator iter = extension_generators_.begin(); @@ -197,8 +174,7 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { (*iter)->GenerateMembersHeader(printer); } - printer->Print("$root_line_prefix$@end\n\n", - "root_line_prefix", root_line_prefix); + printer->Print("@end\n\n"); } // extension_generators_.size() > 0 for (vector::iterator iter = message_generators_.begin(); @@ -239,136 +215,119 @@ void FileGenerator::GenerateSource(io::Printer *printer) { "// @@protoc_insertion_point(imports)\n" "\n"); - if (IsFiltered()) { - printer->Print( - "// File empty because all messages, extensions and enum have been filtered.\n" - "\n" - "\n" - "// Dummy symbol that will be stripped but will avoid linker warnings about\n" - "// no symbols in the .o form compiling this file.\n" - "static int $root_class_name$_dummy __attribute__((unused,used)) = 0;\n" - "\n" - "// @@protoc_insertion_point(global_scope)\n", - "root_class_name", root_class_name_); - return; - } - printer->Print( "#pragma mark - $root_class_name$\n" "\n" "@implementation $root_class_name$\n\n", "root_class_name", root_class_name_); - bool generated_extensions = false; - if (file_->extension_count() + file_->message_type_count() + - file_->dependency_count() > - 0) { - ostringstream extensions_stringstream; - - if (file_->extension_count() + file_->message_type_count() > 0) { - io::OstreamOutputStream extensions_outputstream(&extensions_stringstream); - io::Printer extensions_printer(&extensions_outputstream, '$'); - extensions_printer.Print( - "static GPBExtensionDescription descriptions[] = {\n"); - extensions_printer.Indent(); - for (vector::iterator iter = - extension_generators_.begin(); - iter != extension_generators_.end(); ++iter) { - (*iter)->GenerateStaticVariablesInitialization( - &extensions_printer, &generated_extensions, true); - } - for (vector::iterator iter = - message_generators_.begin(); - iter != message_generators_.end(); ++iter) { - (*iter)->GenerateStaticVariablesInitialization(&extensions_printer, - &generated_extensions); - } - extensions_printer.Outdent(); - extensions_printer.Print("};\n"); - if (generated_extensions) { - extensions_printer.Print( - "for (size_t i = 0; i < sizeof(descriptions) / sizeof(descriptions[0]); ++i) {\n" - " GPBExtensionField *extension = [[GPBExtensionField alloc] initWithDescription:&descriptions[i]];\n" - " [registry addExtension:extension];\n" - " [self globallyRegisterExtension:extension];\n" - " [extension release];\n" - "}\n"); - } else { - extensions_printer.Print("#pragma unused (descriptions)\n"); - } - const vector &dependency_generators = - DependencyGenerators(); - if (dependency_generators.size()) { - for (vector::const_iterator iter = - dependency_generators.begin(); - iter != dependency_generators.end(); ++iter) { - if (!(*iter)->IsFiltered()) { - extensions_printer.Print( - "[registry addExtensions:[$dependency$ extensionRegistry]];\n", - "dependency", (*iter)->RootClassName()); - generated_extensions = true; - } - } - } else if (!generated_extensions) { - extensions_printer.Print("#pragma unused (registry)\n"); - } + // Generate the extension initialization structures for the top level and + // any nested messages. + ostringstream extensions_stringstream; + if (file_->extension_count() + file_->message_type_count() > 0) { + io::OstreamOutputStream extensions_outputstream(&extensions_stringstream); + io::Printer extensions_printer(&extensions_outputstream, '$'); + for (vector::iterator iter = + extension_generators_.begin(); + iter != extension_generators_.end(); ++iter) { + (*iter)->GenerateStaticVariablesInitialization(&extensions_printer); + } + for (vector::iterator iter = + message_generators_.begin(); + iter != message_generators_.end(); ++iter) { + (*iter)->GenerateStaticVariablesInitialization(&extensions_printer); } + extensions_stringstream.flush(); + } - if (generated_extensions) { + // If there were any extensions or this file has any dependencies, output + // a registry to override to create the file specific registry. + const string& extensions_str = extensions_stringstream.str(); + if (extensions_str.length() > 0 || file_->dependency_count() > 0) { + printer->Print( + "+ (GPBExtensionRegistry*)extensionRegistry {\n" + " // This is called by +initialize so there is no need to worry\n" + " // about thread safety and initialization of registry.\n" + " static GPBExtensionRegistry* registry = nil;\n" + " if (!registry) {\n" + " GPBDebugCheckRuntimeVersion();\n" + " registry = [[GPBExtensionRegistry alloc] init];\n"); + + printer->Indent(); + printer->Indent(); + + if (extensions_str.length() > 0) { printer->Print( - "+ (GPBExtensionRegistry*)extensionRegistry {\n" - " // This is called by +initialize so there is no need to worry\n" - " // about thread safety and initialization of registry.\n" - " static GPBExtensionRegistry* registry = nil;\n" - " if (!registry) {\n" - " registry = [[GPBExtensionRegistry alloc] init];\n"); - - printer->Indent(); + "static GPBExtensionDescription descriptions[] = {\n"); printer->Indent(); - - extensions_stringstream.flush(); - printer->Print(extensions_stringstream.str().c_str()); - printer->Outdent(); + printer->Print(extensions_str.c_str()); printer->Outdent(); + printer->Print( + "};\n" + "for (size_t i = 0; i < sizeof(descriptions) / sizeof(descriptions[0]); ++i) {\n" + " GPBExtensionDescriptor *extension =\n" + " [[GPBExtensionDescriptor alloc] initWithExtensionDescription:&descriptions[i]];\n" + " [registry addExtension:extension];\n" + " [self globallyRegisterExtension:extension];\n" + " [extension release];\n" + "}\n"); + } + const vector &dependency_generators = + DependencyGenerators(); + for (vector::const_iterator iter = + dependency_generators.begin(); + iter != dependency_generators.end(); ++iter) { printer->Print( - " }\n" - " return registry;\n" - "}\n" - "\n"); + "[registry addExtensions:[$dependency$ extensionRegistry]];\n", + "dependency", (*iter)->RootClassName()); } + + printer->Outdent(); + printer->Outdent(); + + printer->Print( + " }\n" + " return registry;\n" + "}\n" + "\n"); } printer->Print("@end\n\n"); - - string syntax; - switch (file_->syntax()) { - case FileDescriptor::SYNTAX_UNKNOWN: - syntax = "GPBFileSyntaxUnknown"; - break; - case FileDescriptor::SYNTAX_PROTO2: - syntax = "GPBFileSyntaxProto2"; - break; - case FileDescriptor::SYNTAX_PROTO3: - syntax = "GPBFileSyntaxProto3"; - break; + // File descriptor only needed if there are messages to use it. + if (message_generators_.size() > 0) { + string syntax; + switch (file_->syntax()) { + case FileDescriptor::SYNTAX_UNKNOWN: + syntax = "GPBFileSyntaxUnknown"; + break; + case FileDescriptor::SYNTAX_PROTO2: + syntax = "GPBFileSyntaxProto2"; + break; + case FileDescriptor::SYNTAX_PROTO3: + syntax = "GPBFileSyntaxProto3"; + break; + } + printer->Print( + "#pragma mark - $root_class_name$_FileDescriptor\n" + "\n" + "static GPBFileDescriptor *$root_class_name$_FileDescriptor(void) {\n" + " // This is called by +initialize so there is no need to worry\n" + " // about thread safety of the singleton.\n" + " static GPBFileDescriptor *descriptor = NULL;\n" + " if (!descriptor) {\n" + " GPBDebugCheckRuntimeVersion();\n" + " descriptor = [[GPBFileDescriptor alloc] initWithPackage:@\"$package$\"\n" + " syntax:$syntax$];\n" + " }\n" + " return descriptor;\n" + "}\n" + "\n", + "root_class_name", root_class_name_, + "package", file_->package(), + "syntax", syntax); } - printer->Print( - "static GPBFileDescriptor *$root_class_name$_FileDescriptor(void) {\n" - " // This is called by +initialize so there is no need to worry\n" - " // about thread safety of the singleton.\n" - " static GPBFileDescriptor *descriptor = NULL;\n" - " if (!descriptor) {\n" - " descriptor = [[GPBFileDescriptor alloc] initWithPackage:@\"$package$\"\n" - " syntax:$syntax$];\n" - " }\n" - " return descriptor;\n" - "}\n" - "\n", - "root_class_name", root_class_name_, - "package", file_->package(), - "syntax", syntax); for (vector::iterator iter = enum_generators_.begin(); iter != enum_generators_.end(); ++iter) { diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.h b/src/google/protobuf/compiler/objectivec/objectivec_file.h index 95d17bfd..1bb4f0ea 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.h @@ -64,8 +64,6 @@ class FileGenerator { const string& RootClassName() const { return root_class_name_; } const string Path() const; - bool IsFiltered() const { return is_filtered_; } - bool AreAllExtensionsFiltered() const { return all_extensions_filtered_; } bool IsPublicDependency() const { return is_public_dep_; } protected: @@ -84,8 +82,6 @@ class FileGenerator { vector enum_generators_; vector message_generators_; vector extension_generators_; - bool is_filtered_; - bool all_extensions_filtered_; bool is_public_dep_; const vector& DependencyGenerators(); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc index 17776715..85e438f4 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc @@ -58,10 +58,6 @@ bool ObjectiveCGenerator::Generate(const FileDescriptor* file, return false; } - if (!InitializeClassWhitelist(error)) { - return false; - } - FileGenerator file_generator(file); string filepath = FilePath(file); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index 6d6e5959..9b645f09 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -50,8 +50,6 @@ namespace objectivec { namespace { -hash_set gClassWhitelist; - // islower()/isupper()/tolower()/toupper() change based on locale. // // src/google/protobuf/stubs/strutil.h:150 has the same pattern. For the @@ -580,7 +578,7 @@ string GetCapitalizedType(const FieldDescriptor* field) { case FieldDescriptor::TYPE_STRING: return "String"; case FieldDescriptor::TYPE_BYTES: - return "Data"; + return "Bytes"; case FieldDescriptor::TYPE_ENUM: return "Enum"; case FieldDescriptor::TYPE_GROUP: @@ -684,8 +682,9 @@ static string HandleExtremeFloatingPoint(string val, bool add_float_suffix) { } } -string GPBValueFieldName(const FieldDescriptor* field) { - // Returns the field within the GPBValue union to use for the given field. +string GPBGenericValueFieldName(const FieldDescriptor* field) { + // Returns the field within the GPBGenericValue union to use for the given + // field. if (field->is_repeated()) { return "valueMessage"; } @@ -831,60 +830,6 @@ string BuildCommentsString(const SourceLocation& location) { return final_comments; } -bool InitializeClassWhitelist(string* error) { - const char* env_var_value = getenv("GPB_OBJC_CLASS_WHITELIST_PATHS"); - if (env_var_value == NULL) { - return true; - } - - // The values are joined with ';' in case we ever want to make this a - // generator parameter also (instead of env var), and generator parameter - // parsing already has meaning for ',' and ':'. - vector file_paths = Split(env_var_value, ";", true); - - for (vector::const_iterator i = file_paths.begin(); - i != file_paths.end(); ++i) { - const string& file_path = *i; - - ifstream stream(file_path.c_str(), ifstream::in); - if (!stream.good()) { - if (error != NULL) { - stringstream err_stream; - err_stream << endl << file_path << ":0:0: error: Unable to open"; - *error = err_stream.str(); - return false; - } - } - - string input_line; - while (stream.good()) { - getline(stream, input_line); - string trimmed_line(TrimString(input_line)); - if (trimmed_line.length() == 0) { - // Skip empty lines - continue; - } - if (trimmed_line[0] == '/' || trimmed_line[0] == '#') { - // Skip comments and potential preprocessor symbols - continue; - } - gClassWhitelist.insert(trimmed_line); - } - } - return true; -} - -bool FilterClass(const string& name) { - if (gClassWhitelist.count(name) > 0) { - // Whitelisted, don't filter. - return false; - } - - // If there was no list, default to everything in. - // If there was a list, default to everything out. - return gClassWhitelist.size() > 0; -} - void TextFormatDecodeData::AddString(int32 key, const string& input_for_decode, const string& desired_output) { diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index 19317698..10d51a34 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -138,16 +138,13 @@ inline ObjectiveCType GetObjectiveCType(const FieldDescriptor* field) { bool IsPrimitiveType(const FieldDescriptor* field); bool IsReferenceType(const FieldDescriptor* field); -string GPBValueFieldName(const FieldDescriptor* field); +string GPBGenericValueFieldName(const FieldDescriptor* field); string DefaultValue(const FieldDescriptor* field); string BuildFlagsString(const vector& strings); string BuildCommentsString(const SourceLocation& location); -bool InitializeClassWhitelist(string* error); -bool FilterClass(const string& name); - // Generate decode data needed for ObjC's GPBDecodeTextFormatName() to transform // the input into the the expected output. class LIBPROTOC_EXPORT TextFormatDecodeData { diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc index b091b77a..dc1cef55 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc @@ -242,6 +242,14 @@ TEST(ObjCHelperDeathTest, TextFormatDecodeData_Failures) { } #endif // PROTOBUF_HAS_DEATH_TEST +// TODO(thomasvl): Should probably add some unittests for all the special cases +// of name mangling (class name, field name, enum names). Rather than doing +// this with an ObjC test in the objectivec directory, we should be able to +// use src/google/protobuf/compiler/importer* (like other tests) to support a +// virtual file system to feed in protos, once we have the Descriptor tree, the +// tests could use the helper methods for generating names and validate the +// right things are happening. + } // namespace } // namespace objectivec } // namespace compiler diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message.cc b/src/google/protobuf/compiler/objectivec/objectivec_message.cc index 52e583bf..32671d42 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_message.cc @@ -178,49 +178,25 @@ MessageGenerator::MessageGenerator(const string& root_classname, : root_classname_(root_classname), descriptor_(descriptor), field_generators_(descriptor), - class_name_(ClassName(descriptor_)), - sub_content_filtered_(true) { - if (FilterClass(class_name_)) { - filter_reason_ = - string("Message |") + class_name_ + "| was not whitelisted."; + class_name_(ClassName(descriptor_)) { + for (int i = 0; i < descriptor_->extension_count(); i++) { + extension_generators_.push_back( + new ExtensionGenerator(class_name_, descriptor_->extension(i))); } - if (!IsFiltered()) { - // No need to generate extensions if this message is filtered - for (int i = 0; i < descriptor_->extension_count(); i++) { - extension_generators_.push_back( - new ExtensionGenerator(class_name_, descriptor_->extension(i))); - } - // No need to generate oneofs if this message is filtered. - for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { - OneofGenerator* generator = new OneofGenerator(descriptor_->oneof_decl(i)); - oneof_generators_.push_back(generator); - } + + for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { + OneofGenerator* generator = new OneofGenerator(descriptor_->oneof_decl(i)); + oneof_generators_.push_back(generator); } - // We may have enums of this message that are used even if the message - // itself is filtered. for (int i = 0; i < descriptor_->enum_type_count(); i++) { EnumGenerator* generator = new EnumGenerator(descriptor_->enum_type(i)); - // The enums are exposed via C functions, so they will dead strip if - // not used. - sub_content_filtered_ &= false; enum_generators_.push_back(generator); } - // We may have nested messages that are used even if the message itself - // is filtered. for (int i = 0; i < descriptor_->nested_type_count(); i++) { - const Descriptor* nested_descriptor = descriptor_->nested_type(i); MessageGenerator* generator = - new MessageGenerator(root_classname_, nested_descriptor); - // Don't check map entries for being filtered, as they don't directly - // generate anything in Objective C. In theory, they only should include - // references to other toplevel types, but we still make the generators - // to be safe. - if (!IsMapEntryMessage(nested_descriptor)) { - sub_content_filtered_ &= generator->IsFiltered(); - } - sub_content_filtered_ &= generator->IsSubContentFiltered(); + new MessageGenerator(root_classname_, descriptor_->nested_type(i)); nested_message_generators_.push_back(generator); } } @@ -236,31 +212,26 @@ MessageGenerator::~MessageGenerator() { } void MessageGenerator::GenerateStaticVariablesInitialization( - io::Printer* printer, bool* out_generated) { - if (!IsFiltered()) { - // Skip extensions if we are filtered. - for (vector::iterator iter = - extension_generators_.begin(); - iter != extension_generators_.end(); ++iter) { - (*iter)->GenerateStaticVariablesInitialization(printer, out_generated, - false); - } + io::Printer* printer) { + for (vector::iterator iter = + extension_generators_.begin(); + iter != extension_generators_.end(); ++iter) { + (*iter)->GenerateStaticVariablesInitialization(printer); } - // Generating sub messages is perfectly fine though. for (vector::iterator iter = nested_message_generators_.begin(); iter != nested_message_generators_.end(); ++iter) { - (*iter)->GenerateStaticVariablesInitialization(printer, out_generated); + (*iter)->GenerateStaticVariablesInitialization(printer); } } void MessageGenerator::DetermineForwardDeclarations(set* fwd_decls) { - if (!IsFiltered() && !IsMapEntryMessage(descriptor_)) { + if (!IsMapEntryMessage(descriptor_)) { for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* fieldDescriptor = descriptor_->field(i); - // If it is a the field is repeated, the type will be and *Array, - // and we don't need any forward decl. + // If it is a the field is repeated, the type will be and *Array, and we + // don't need any forward decl. if (fieldDescriptor->is_repeated()) { continue; } @@ -291,12 +262,10 @@ void MessageGenerator::GenerateEnumHeader(io::Printer* printer) { void MessageGenerator::GenerateExtensionRegistrationSource( io::Printer* printer) { - if (!IsFiltered()) { - for (vector::iterator iter = - extension_generators_.begin(); - iter != extension_generators_.end(); ++iter) { - (*iter)->GenerateRegistrationSource(printer); - } + for (vector::iterator iter = + extension_generators_.begin(); + iter != extension_generators_.end(); ++iter) { + (*iter)->GenerateRegistrationSource(printer); } for (vector::iterator iter = @@ -317,101 +286,84 @@ void MessageGenerator::GenerateMessageHeader(io::Printer* printer) { return; } - if (IsFiltered()) { - printer->Print("// $filter_reason$\n\n", - "filter_reason", filter_reason_); - } else { - printer->Print( - "#pragma mark - $classname$\n" - "\n", - "classname", class_name_); + printer->Print( + "#pragma mark - $classname$\n" + "\n", + "classname", class_name_); - if (descriptor_->field_count()) { - // Even if there are fields, they could be filtered away, so always use - // a buffer to confirm we have something. - ostringstream fieldnumber_stringstream; - { - scoped_array sorted_fields( - SortFieldsByNumber(descriptor_)); - - io::OstreamOutputStream fieldnumber_outputstream( - &fieldnumber_stringstream); - io::Printer fieldnumber_printer(&fieldnumber_outputstream, '$'); - for (int i = 0; i < descriptor_->field_count(); i++) { - field_generators_.get(sorted_fields[i]) - .GenerateFieldNumberConstant(&fieldnumber_printer); - } - fieldnumber_stringstream.flush(); - } - const string& fieldnumber_str = fieldnumber_stringstream.str(); - if (fieldnumber_str.length()) { - printer->Print("typedef GPB_ENUM($classname$_FieldNumber) {\n", - "classname", class_name_); - printer->Indent(); - printer->Print(fieldnumber_str.c_str()); - printer->Outdent(); - printer->Print("};\n\n"); - } - } + if (descriptor_->field_count()) { + scoped_array sorted_fields( + SortFieldsByNumber(descriptor_)); - for (vector::iterator iter = oneof_generators_.begin(); - iter != oneof_generators_.end(); ++iter) { - (*iter)->GenerateCaseEnum(printer); - } + printer->Print("typedef GPB_ENUM($classname$_FieldNumber) {\n", + "classname", class_name_); + printer->Indent(); - string message_comments; - SourceLocation location; - if (descriptor_->GetSourceLocation(&location)) { - message_comments = BuildCommentsString(location); - } else { - message_comments = ""; + for (int i = 0; i < descriptor_->field_count(); i++) { + field_generators_.get(sorted_fields[i]) + .GenerateFieldNumberConstant(printer); } - printer->Print( - "$comments$@interface $classname$ : GPBMessage\n\n", - "classname", class_name_, - "comments", message_comments); + printer->Outdent(); + printer->Print("};\n\n"); + } - vector seen_oneofs(descriptor_->oneof_decl_count(), 0); - for (int i = 0; i < descriptor_->field_count(); i++) { - const FieldDescriptor* field = descriptor_->field(i); - if (field->containing_oneof() != NULL) { - const int oneof_index = field->containing_oneof()->index(); - if (!seen_oneofs[oneof_index]) { - seen_oneofs[oneof_index] = 1; - oneof_generators_[oneof_index]->GeneratePublicCasePropertyDeclaration( - printer); - } + for (vector::iterator iter = oneof_generators_.begin(); + iter != oneof_generators_.end(); ++iter) { + (*iter)->GenerateCaseEnum(printer); + } + + string message_comments; + SourceLocation location; + if (descriptor_->GetSourceLocation(&location)) { + message_comments = BuildCommentsString(location); + } else { + message_comments = ""; + } + + printer->Print( + "$comments$@interface $classname$ : GPBMessage\n\n", + "classname", class_name_, + "comments", message_comments); + + vector seen_oneofs(descriptor_->oneof_decl_count(), 0); + for (int i = 0; i < descriptor_->field_count(); i++) { + const FieldDescriptor* field = descriptor_->field(i); + if (field->containing_oneof() != NULL) { + const int oneof_index = field->containing_oneof()->index(); + if (!seen_oneofs[oneof_index]) { + seen_oneofs[oneof_index] = 1; + oneof_generators_[oneof_index]->GeneratePublicCasePropertyDeclaration( + printer); } - field_generators_.get(field) - .GeneratePropertyDeclaration(printer); } + field_generators_.get(field).GeneratePropertyDeclaration(printer); + } - printer->Print("@end\n\n"); + printer->Print("@end\n\n"); - for (int i = 0; i < descriptor_->field_count(); i++) { - field_generators_.get(descriptor_->field(i)) - .GenerateCFunctionDeclarations(printer); - } + for (int i = 0; i < descriptor_->field_count(); i++) { + field_generators_.get(descriptor_->field(i)) + .GenerateCFunctionDeclarations(printer); + } - if (!oneof_generators_.empty()) { - for (vector::iterator iter = oneof_generators_.begin(); - iter != oneof_generators_.end(); ++iter) { - (*iter)->GenerateClearFunctionDeclaration(printer); - } - printer->Print("\n"); + if (!oneof_generators_.empty()) { + for (vector::iterator iter = oneof_generators_.begin(); + iter != oneof_generators_.end(); ++iter) { + (*iter)->GenerateClearFunctionDeclaration(printer); } + printer->Print("\n"); + } - if (descriptor_->extension_count() > 0) { - printer->Print("@interface $classname$ (DynamicMethods)\n\n", - "classname", class_name_); - for (vector::iterator iter = - extension_generators_.begin(); - iter != extension_generators_.end(); ++iter) { - (*iter)->GenerateMembersHeader(printer); - } - printer->Print("@end\n\n"); + if (descriptor_->extension_count() > 0) { + printer->Print("@interface $classname$ (DynamicMethods)\n\n", + "classname", class_name_); + for (vector::iterator iter = + extension_generators_.begin(); + iter != extension_generators_.end(); ++iter) { + (*iter)->GenerateMembersHeader(printer); } + printer->Print("@end\n\n"); } for (vector::iterator iter = @@ -422,7 +374,7 @@ void MessageGenerator::GenerateMessageHeader(io::Printer* printer) { } void MessageGenerator::GenerateSource(io::Printer* printer) { - if (!IsFiltered() && !IsMapEntryMessage(descriptor_)) { + if (!IsMapEntryMessage(descriptor_)) { printer->Print( "#pragma mark - $classname$\n" "\n", @@ -454,6 +406,23 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { sort(sorted_extensions.begin(), sorted_extensions.end(), ExtensionRangeOrdering()); + // TODO(thomasvl): Finish optimizing has bit. The current behavior is as + // follows: + // 1. objectivec_field.cc's SetCommonFieldVariables() defaults the has_index + // to the field's index in the list of fields. + // 2. RepeatedFieldGenerator::RepeatedFieldGenerator() sets has_index to + // GPBNoHasBit because repeated fields & map<> fields don't use the has + // bit. + // 3. FieldGenerator::SetOneofIndexBase() overrides has_bit with a negative + // index that groups all the elements on of the oneof. + // So in has_storage, we need enough bits for the single fields that aren't + // in any oneof, and then one int32 for each oneof (to store the field + // number). So we could save a little space by not using the field's index + // and instead make a second pass only assigning indexes for the fields + // that would need it. The only savings would come when messages have over + // a multiple of 32 fields with some number being repeated or in oneofs to + // drop the count below that 32 multiple; so it hasn't seemed worth doing + // at the moment. size_t num_has_bits = descriptor_->field_count(); size_t sizeof_has_storage = (num_has_bits + 31) / 32; // Tell all the fields the oneof base. @@ -467,7 +436,7 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { printer->Print( "\n" - "typedef struct $classname$_Storage {\n" + "typedef struct $classname$__storage_ {\n" " uint32_t _has_storage_[$sizeof_has_storage$];\n", "classname", class_name_, "sizeof_has_storage", SimpleItoa(sizeof_has_storage)); @@ -479,14 +448,14 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { } printer->Outdent(); - printer->Print("} $classname$_Storage;\n\n", "classname", class_name_); + printer->Print("} $classname$__storage_;\n\n", "classname", class_name_); printer->Print( "// This method is threadsafe because it is initially called\n" "// in +initialize for each subclass.\n" "+ (GPBDescriptor *)descriptor {\n" - " static GPBDescriptor *descriptor = NULL;\n" + " static GPBDescriptor *descriptor = nil;\n" " if (!descriptor) {\n"); bool has_oneofs = oneof_generators_.size(); @@ -507,30 +476,45 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { " };\n"); } - printer->Print( - " static GPBMessageFieldDescription fields[] = {\n"); - printer->Indent(); - printer->Indent(); - printer->Indent(); TextFormatDecodeData text_format_decode_data; - for (int i = 0; i < descriptor_->field_count(); ++i) { - const FieldGenerator& field_generator = - field_generators_.get(sorted_fields[i]); - field_generator.GenerateFieldDescription(printer); - if (field_generator.needs_textformat_name_support()) { - text_format_decode_data.AddString(sorted_fields[i]->number(), - field_generator.generated_objc_name(), - field_generator.raw_field_name()); + bool has_fields = descriptor_->field_count() > 0; + if (has_fields) { + // TODO(thomasvl): The plugin's FieldGenerator::GenerateFieldDescription() + // wraps the fieldOptions's value of this structure in an CPP gate so + // they can be compiled away; but that still results in a const char* in + // the structure for a NULL pointer for every message field. If the + // fieldOptions are moved to a separate payload like the TextFormat extra + // data is, then it would shrink that static data shrinking the binaries + // a little more. + // TODO(thomasvl): proto3 syntax doens't need a defaultValue in the + // structure because primitive types are always zero. If we add a second + // structure and a different initializer, we can avoid the wasted static + // storage for every field in a proto3 message. + printer->Print( + " static GPBMessageFieldDescription fields[] = {\n"); + printer->Indent(); + printer->Indent(); + printer->Indent(); + for (int i = 0; i < descriptor_->field_count(); ++i) { + const FieldGenerator& field_generator = + field_generators_.get(sorted_fields[i]); + field_generator.GenerateFieldDescription(printer); + if (field_generator.needs_textformat_name_support()) { + text_format_decode_data.AddString(sorted_fields[i]->number(), + field_generator.generated_objc_name(), + field_generator.raw_field_name()); + } } + printer->Outdent(); + printer->Outdent(); + printer->Outdent(); + printer->Print( + " };\n"); } - printer->Outdent(); - printer->Outdent(); - printer->Outdent(); bool has_enums = enum_generators_.size(); if (has_enums) { printer->Print( - " };\n" " static GPBMessageEnumDescription enums[] = {\n"); printer->Indent(); printer->Indent(); @@ -543,12 +527,13 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { printer->Outdent(); printer->Outdent(); printer->Outdent(); + printer->Print( + " };\n"); } bool has_extensions = sorted_extensions.size(); if (has_extensions) { printer->Print( - " };\n" " static GPBExtensionRange ranges[] = {\n"); printer->Indent(); printer->Indent(); @@ -561,11 +546,16 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { printer->Outdent(); printer->Outdent(); printer->Outdent(); + printer->Print( + " };\n"); } map vars; vars["classname"] = class_name_; vars["rootclassname"] = root_classname_; + vars["fields"] = has_fields ? "fields" : "NULL"; + vars["fields_count"] = + has_fields ? "sizeof(fields) / sizeof(GPBMessageFieldDescription)" : "0"; vars["oneofs"] = has_oneofs ? "oneofs" : "NULL"; vars["oneof_count"] = has_oneofs ? "sizeof(oneofs) / sizeof(GPBMessageOneofDescription)" : "0"; @@ -578,23 +568,23 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { vars["wireformat"] = descriptor_->options().message_set_wire_format() ? "YES" : "NO"; - printer->Print(" };\n"); if (text_format_decode_data.num_entries() == 0) { printer->Print( vars, - " descriptor = [GPBDescriptor allocDescriptorForClass:[$classname$ class]\n" - " rootClass:[$rootclassname$ class]\n" - " file:$rootclassname$_FileDescriptor()\n" - " fields:fields\n" - " fieldCount:sizeof(fields) / sizeof(GPBMessageFieldDescription)\n" - " oneofs:$oneofs$\n" - " oneofCount:$oneof_count$\n" - " enums:$enums$\n" - " enumCount:$enum_count$\n" - " ranges:$ranges$\n" - " rangeCount:$range_count$\n" - " storageSize:sizeof($classname$_Storage)\n" - " wireFormat:$wireformat$];\n"); + " GPBDescriptor *localDescriptor =\n" + " [GPBDescriptor allocDescriptorForClass:[$classname$ class]\n" + " rootClass:[$rootclassname$ class]\n" + " file:$rootclassname$_FileDescriptor()\n" + " fields:$fields$\n" + " fieldCount:$fields_count$\n" + " oneofs:$oneofs$\n" + " oneofCount:$oneof_count$\n" + " enums:$enums$\n" + " enumCount:$enum_count$\n" + " ranges:$ranges$\n" + " rangeCount:$range_count$\n" + " storageSize:sizeof($classname$__storage_)\n" + " wireFormat:$wireformat$];\n"); } else { vars["extraTextFormatInfo"] = CEscape(text_format_decode_data.Data()); printer->Print( @@ -604,26 +594,29 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { "#else\n" " static const char *extraTextFormatInfo = \"$extraTextFormatInfo$\";\n" "#endif // GPBOBJC_SKIP_MESSAGE_TEXTFORMAT_EXTRAS\n" - " descriptor = [GPBDescriptor allocDescriptorForClass:[$classname$ class]\n" - " rootClass:[$rootclassname$ class]\n" - " file:$rootclassname$_FileDescriptor()\n" - " fields:fields\n" - " fieldCount:sizeof(fields) / sizeof(GPBMessageFieldDescription)\n" - " oneofs:$oneofs$\n" - " oneofCount:$oneof_count$\n" - " enums:$enums$\n" - " enumCount:$enum_count$\n" - " ranges:$ranges$\n" - " rangeCount:$range_count$\n" - " storageSize:sizeof($classname$_Storage)\n" - " wireFormat:$wireformat$\n" - " extraTextFormatInfo:extraTextFormatInfo];\n"); + " GPBDescriptor *localDescriptor =\n" + " [GPBDescriptor allocDescriptorForClass:[$classname$ class]\n" + " rootClass:[$rootclassname$ class]\n" + " file:$rootclassname$_FileDescriptor()\n" + " fields:$fields$\n" + " fieldCount:$fields_count$\n" + " oneofs:$oneofs$\n" + " oneofCount:$oneof_count$\n" + " enums:$enums$\n" + " enumCount:$enum_count$\n" + " ranges:$ranges$\n" + " rangeCount:$range_count$\n" + " storageSize:sizeof($classname$__storage_)\n" + " wireFormat:$wireformat$\n" + " extraTextFormatInfo:extraTextFormatInfo];\n"); } printer->Print( - " }\n" - " return descriptor;\n" - "}\n\n" - "@end\n\n"); + " NSAssert(descriptor == nil, @\"Startup recursed!\");\n" + " descriptor = localDescriptor;\n" + " }\n" + " return descriptor;\n" + "}\n\n" + "@end\n\n"); for (int i = 0; i < descriptor_->field_count(); i++) { field_generators_.get(descriptor_->field(i)) diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message.h b/src/google/protobuf/compiler/objectivec/objectivec_message.h index 8d03c0b8..06b536ff 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_message.h @@ -57,20 +57,13 @@ class MessageGenerator { MessageGenerator(const string& root_classname, const Descriptor* descriptor); ~MessageGenerator(); - void GenerateStaticVariablesInitialization(io::Printer* printer, - bool* out_generated); + void GenerateStaticVariablesInitialization(io::Printer* printer); void GenerateEnumHeader(io::Printer* printer); void GenerateMessageHeader(io::Printer* printer); void GenerateSource(io::Printer* printer); void GenerateExtensionRegistrationSource(io::Printer* printer); void DetermineForwardDeclarations(set* fwd_decls); - // This only speaks for this message, not sub message/enums. - bool IsFiltered() const { return filter_reason_.length() > 0; } - // This message being filtered doesn't effect this, instead it covers if - // there are any nested messages or enums. - bool IsSubContentFiltered() const { return sub_content_filtered_; } - private: void GenerateParseFromMethodsHeader(io::Printer* printer); @@ -87,8 +80,6 @@ class MessageGenerator { const Descriptor* descriptor_; FieldGeneratorMap field_generators_; const string class_name_; - string filter_reason_; - bool sub_content_filtered_; vector extension_generators_; vector enum_generators_; vector nested_message_generators_; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc index 2e3bdfdb..f2ce4e5b 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc @@ -53,7 +53,7 @@ void SetMessageVariables(const FieldDescriptor* descriptor, (*variables)["group_or_message"] = (descriptor->type() == FieldDescriptor::TYPE_GROUP) ? "Group" : "Message"; - (*variables)["typeSpecific_value"] = "GPBStringifySymbol(" + message_type + ")"; + (*variables)["dataTypeSpecific_value"] = "GPBStringifySymbol(" + message_type + ")"; } } // namespace -- cgit v1.2.3