From 1dcc329427fd103a0abd96ab787270f5d0a31861 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 21 May 2015 17:14:52 -0400 Subject: Objective C Second Alpha Drop - Style fixups in the code. - map<> serialization fixes and more tests. - Autocreation of map<> fields (to match repeated fields). - @@protoc_insertion_point(global_scope|imports). - Fixup proto2 syntax extension support. - Move all startup code to +initialize so it happen on class usage and not app startup. - Have generated headers use forward declarations and move imports into generated code, reduces what is need at compile time to speed up compiled and avoid pointless rippling of rebuilds. --- .../protobuf/compiler/objectivec/objectivec_enum.h | 2 +- .../compiler/objectivec/objectivec_enum_field.cc | 23 +++- .../compiler/objectivec/objectivec_enum_field.h | 3 +- .../compiler/objectivec/objectivec_extension.cc | 4 +- .../compiler/objectivec/objectivec_extension.h | 4 +- .../compiler/objectivec/objectivec_field.cc | 21 +++- .../compiler/objectivec/objectivec_field.h | 12 +- .../compiler/objectivec/objectivec_file.cc | 136 ++++++++++++--------- .../protobuf/compiler/objectivec/objectivec_file.h | 11 +- .../compiler/objectivec/objectivec_helpers.cc | 51 ++++++-- .../compiler/objectivec/objectivec_helpers.h | 3 + .../compiler/objectivec/objectivec_map_field.cc | 2 + .../compiler/objectivec/objectivec_map_field.h | 2 +- .../compiler/objectivec/objectivec_message.cc | 23 +++- .../compiler/objectivec/objectivec_message.h | 2 +- .../objectivec/objectivec_message_field.cc | 6 + .../compiler/objectivec/objectivec_message_field.h | 7 +- .../compiler/objectivec/objectivec_oneof.h | 2 +- .../objectivec/objectivec_primitive_field.cc | 4 + .../objectivec/objectivec_primitive_field.h | 6 +- 20 files changed, 223 insertions(+), 101 deletions(-) (limited to 'src') diff --git a/src/google/protobuf/compiler/objectivec/objectivec_enum.h b/src/google/protobuf/compiler/objectivec/objectivec_enum.h index 2dc5547b..0b41cf73 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_enum.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_enum.h @@ -49,7 +49,7 @@ namespace objectivec { class EnumGenerator { public: - EnumGenerator(const EnumDescriptor* descriptor); + explicit EnumGenerator(const EnumDescriptor* descriptor); ~EnumGenerator(); void GenerateHeader(io::Printer* printer); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc index 739282b2..d6609692 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc @@ -48,6 +48,13 @@ void SetEnumVariables(const FieldDescriptor* descriptor, map* variables) { string type = EnumName(descriptor->enum_type()); (*variables)["storage_type"] = type; + // For non repeated fields, if it was defined in a different file, the + // property decls need to use "enum NAME" rather than just "NAME" to support + // the forward declaration of the enums. + if (!descriptor->is_repeated() && + (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"; @@ -76,7 +83,9 @@ void EnumFieldGenerator::GenerateFieldDescriptionTypeSpecific( void EnumFieldGenerator::GenerateCFunctionDeclarations( io::Printer* printer) const { - if (!HasPreservingUnknownEnumSemantics(descriptor_->file())) return; + if (!HasPreservingUnknownEnumSemantics(descriptor_->file())) { + return; + } printer->Print( variables_, @@ -105,6 +114,18 @@ void EnumFieldGenerator::GenerateCFunctionImplementations( "\n"); } +void EnumFieldGenerator::DetermineForwardDeclarations( + set* fwd_decls) const { + // If it is an enum defined in a different file, then we'll need a forward + // declaration for it. When it is in our file, all the enums are output + // before the message, so it will be declared before it is needed. + if (descriptor_->file() != descriptor_->enum_type()->file()) { + // Enum name is already in "storage_type". + const string& name = variable("storage_type"); + fwd_decls->insert("GPB_ENUM_FWD_DECLARE(" + name + ")"); + } +} + RepeatedEnumFieldGenerator::RepeatedEnumFieldGenerator( const FieldDescriptor* descriptor) : RepeatedFieldGenerator(descriptor) { diff --git a/src/google/protobuf/compiler/objectivec/objectivec_enum_field.h b/src/google/protobuf/compiler/objectivec/objectivec_enum_field.h index 2d5822bb..b629eae8 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_enum_field.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_enum_field.h @@ -47,9 +47,10 @@ class EnumFieldGenerator : public SingleFieldGenerator { virtual void GenerateFieldDescriptionTypeSpecific(io::Printer* printer) const; virtual void GenerateCFunctionDeclarations(io::Printer* printer) const; virtual void GenerateCFunctionImplementations(io::Printer* printer) const; + virtual void DetermineForwardDeclarations(set* fwd_decls) const; protected: - EnumFieldGenerator(const FieldDescriptor* descriptor); + explicit EnumFieldGenerator(const FieldDescriptor* descriptor); virtual ~EnumFieldGenerator(); private: diff --git a/src/google/protobuf/compiler/objectivec/objectivec_extension.cc b/src/google/protobuf/compiler/objectivec/objectivec_extension.cc index 0574cca2..76137c80 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_extension.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_extension.cc @@ -66,7 +66,7 @@ ExtensionGenerator::ExtensionGenerator(const string& root_class_name, } if (descriptor->is_map()) { // NOTE: src/google/protobuf/compiler/plugin.cc makes use of cerr for some - // error case, so it seem to be ok to use as a back door for errors. + // error cases, so it seems to be ok to use as a back door for errors. cerr << "error: Extension is a map<>!" << " That used to be blocked by the compiler." << endl; cerr.flush(); @@ -107,7 +107,7 @@ void ExtensionGenerator::GenerateStaticVariablesInitialization( std::vector options; if (descriptor_->is_repeated()) options.push_back("GPBExtensionRepeated"); - if (descriptor_->options().packed()) options.push_back("GPBExtensionPacked"); + if (descriptor_->is_packed()) options.push_back("GPBExtensionPacked"); if (descriptor_->containing_type()->options().message_set_wire_format()) options.push_back("GPBExtensionSetWireFormat"); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_extension.h b/src/google/protobuf/compiler/objectivec/objectivec_extension.h index d17f5be9..553f0887 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_extension.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_extension.h @@ -47,8 +47,8 @@ namespace objectivec { class ExtensionGenerator { public: - explicit ExtensionGenerator(const string& root_class_name, - const FieldDescriptor* descriptor); + ExtensionGenerator(const string& root_class_name, + const FieldDescriptor* descriptor); ~ExtensionGenerator(); void GenerateMembersHeader(io::Printer* printer); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_field.cc index 93fffe0e..ee5253a5 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_field.cc @@ -80,7 +80,7 @@ void SetCommonFieldVariables(const FieldDescriptor* descriptor, if (descriptor->is_repeated()) field_flags.push_back("GPBFieldRepeated"); if (descriptor->is_required()) field_flags.push_back("GPBFieldRequired"); if (descriptor->is_optional()) field_flags.push_back("GPBFieldOptional"); - if (descriptor->options().packed()) field_flags.push_back("GPBFieldPacked"); + if (descriptor->is_packed()) field_flags.push_back("GPBFieldPacked"); // ObjC custom flags. if (descriptor->has_default_value()) @@ -235,6 +235,11 @@ void FieldGenerator::GenerateCFunctionImplementations( // Nothing } +void FieldGenerator::DetermineForwardDeclarations( + set* fwd_decls) const { + // Nothing +} + void FieldGenerator::GenerateFieldDescription( io::Printer* printer) const { printer->Print( @@ -282,12 +287,16 @@ void FieldGenerator::SetOneofIndexBase(int index_base) { if (descriptor_->containing_oneof() != NULL) { int index = descriptor_->containing_oneof()->index() + index_base; // Flip the sign to mark it as a oneof. - variables_["has_index"] = SimpleItoa(-index);; + variables_["has_index"] = SimpleItoa(-index); } } void FieldGenerator::FinishInitialization(void) { - // Nothing + // If "property_type" wasn't set, make it "storage_type". + if ((variables_.find("property_type") == variables_.end()) && + (variables_.find("storage_type") != variables_.end())) { + variables_["property_type"] = variable("storage_type"); + } } SingleFieldGenerator::SingleFieldGenerator( @@ -313,7 +322,7 @@ void SingleFieldGenerator::GeneratePropertyDeclaration( } printer->Print( variables_, - "@property(nonatomic, readwrite) $storage_type$ $name$;\n" + "@property(nonatomic, readwrite) $property_type$ $name$;\n" "\n"); } @@ -369,12 +378,12 @@ void ObjCObjFieldGenerator::GeneratePropertyDeclaration( } printer->Print( variables_, - "@property(nonatomic, readwrite, $property_storage_attribute$) $storage_type$ *$name$$storage_attribute$;\n"); + "@property(nonatomic, readwrite, $property_storage_attribute$) $property_type$ *$name$$storage_attribute$;\n"); if (IsInitName(variables_.at("name"))) { // 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 printer->Print(variables_, - "- ($storage_type$ *)$name$ GPB_METHOD_FAMILY_NONE;\n"); + "- ($property_type$ *)$name$ GPB_METHOD_FAMILY_NONE;\n"); } printer->Print("\n"); } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_field.h b/src/google/protobuf/compiler/objectivec/objectivec_field.h index c65e73b2..130a52dd 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_field.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_field.h @@ -65,6 +65,8 @@ class FieldGenerator { virtual void GenerateCFunctionDeclarations(io::Printer* printer) const; virtual void GenerateCFunctionImplementations(io::Printer* printer) const; + virtual void DetermineForwardDeclarations(set* fwd_decls) const; + void SetOneofIndexBase(int index_base); string variable(const char* key) const { @@ -79,7 +81,7 @@ class FieldGenerator { string raw_field_name() const { return variable("raw_field_name"); } protected: - FieldGenerator(const FieldDescriptor* descriptor); + explicit FieldGenerator(const FieldDescriptor* descriptor); virtual void FinishInitialization(void); virtual bool WantsHasProperty(void) const = 0; @@ -101,7 +103,7 @@ class SingleFieldGenerator : public FieldGenerator { virtual void GeneratePropertyImplementation(io::Printer* printer) const; protected: - SingleFieldGenerator(const FieldDescriptor* descriptor); + explicit SingleFieldGenerator(const FieldDescriptor* descriptor); virtual bool WantsHasProperty(void) const; private: @@ -117,7 +119,7 @@ class ObjCObjFieldGenerator : public SingleFieldGenerator { virtual void GeneratePropertyDeclaration(io::Printer* printer) const; protected: - ObjCObjFieldGenerator(const FieldDescriptor* descriptor); + explicit ObjCObjFieldGenerator(const FieldDescriptor* descriptor); private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ObjCObjFieldGenerator); @@ -133,7 +135,7 @@ class RepeatedFieldGenerator : public ObjCObjFieldGenerator { virtual void GeneratePropertyImplementation(io::Printer* printer) const; protected: - RepeatedFieldGenerator(const FieldDescriptor* descriptor); + explicit RepeatedFieldGenerator(const FieldDescriptor* descriptor); virtual void FinishInitialization(void); virtual bool WantsHasProperty(void) const; @@ -144,7 +146,7 @@ class RepeatedFieldGenerator : public ObjCObjFieldGenerator { // Convenience class which constructs FieldGenerators for a Descriptor. class FieldGeneratorMap { public: - FieldGeneratorMap(const Descriptor* descriptor); + explicit FieldGeneratorMap(const Descriptor* descriptor); ~FieldGeneratorMap(); const FieldGenerator& get(const FieldDescriptor* field) const; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.cc b/src/google/protobuf/compiler/objectivec/objectivec_file.cc index b3ad448e..d04eee85 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.cc @@ -50,11 +50,17 @@ const int32 GOOGLE_PROTOBUF_OBJC_GEN_VERSION = 30000; namespace compiler { namespace objectivec { + FileGenerator::FileGenerator(const FileDescriptor *file) : file_(file), root_class_name_(FileClassName(file)), is_filtered_(true), - all_extensions_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. + 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 @@ -96,7 +102,9 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { "\n", "filename", file_->name()); - printer->Print("#import \"GPBProtocolBuffers.h\"\n\n"); + printer->Print( + "#import \"GPBProtocolBuffers.h\"\n" + "\n"); // Add some verification that the generated code matches the source the // code is being compiled with. @@ -108,31 +116,34 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { "protoc_gen_objc_version", SimpleItoa(GOOGLE_PROTOBUF_OBJC_GEN_VERSION)); - if (!IsFiltered()) { - const vector &dependency_generators = - DependencyGenerators(); - if (dependency_generators.size() > 0) { - for (vector::const_iterator iter = - dependency_generators.begin(); - iter != dependency_generators.end(); ++iter) { - printer->Print("#import \"$header$.pbobjc.h\"\n", - "header", (*iter)->Path()); - } - printer->Print("\n"); + const vector &dependency_generators = + DependencyGenerators(); + for (vector::const_iterator iter = + dependency_generators.begin(); + iter != dependency_generators.end(); ++iter) { + if ((*iter)->IsPublicDependency()) { + printer->Print("#import \"$header$.pbobjc.h\"\n", + "header", (*iter)->Path()); } } + printer->Print( + "// @@protoc_insertion_point(imports)\n" + "\n"); + printer->Print("CF_EXTERN_C_BEGIN\n\n"); if (!IsFiltered()) { - set dependencies; - DetermineDependencies(&dependencies); - for (set::const_iterator i(dependencies.begin()); - i != dependencies.end(); ++i) { + 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 (dependencies.begin() != dependencies.end()) { + if (fwd_decls.begin() != fwd_decls.end()) { printer->Print("\n"); } } @@ -156,7 +167,14 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { "#pragma mark - $root_class_name$\n" "\n" "@interface $root_class_name$ : GPBRootObject\n" - "@end\n\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_); } @@ -189,33 +207,10 @@ void FileGenerator::GenerateHeader(io::Printer *printer) { } printer->Print("CF_EXTERN_C_END\n"); -} - -void DetermineDependenciesWorker(set *dependencies, - set *seen_files, - const string &classname, - const FileDescriptor *file) { - if (seen_files->find(file->name()) != seen_files->end()) { - // don't infinitely recurse - return; - } - - seen_files->insert(file->name()); - - for (int i = 0; i < file->dependency_count(); i++) { - DetermineDependenciesWorker(dependencies, seen_files, classname, - file->dependency(i)); - } - for (int i = 0; i < file->message_type_count(); i++) { - MessageGenerator(classname, file->message_type(i)) - .DetermineDependencies(dependencies); - } -} -void FileGenerator::DetermineDependencies(set *dependencies) { - set seen_files; - DetermineDependenciesWorker(dependencies, &seen_files, root_class_name_, - file_); + printer->Print( + "\n" + "// @@protoc_insertion_point(global_scope)\n"); } void FileGenerator::GenerateSource(io::Printer *printer) { @@ -225,6 +220,25 @@ void FileGenerator::GenerateSource(io::Printer *printer) { "\n", "filename", file_->name()); + string header_file = Path() + ".pbobjc.h"; + printer->Print( + "#import \"GPBProtocolBuffers_RuntimeSupport.h\"\n" + "#import \"$header_file$\"\n", + "header_file", header_file); + const vector &dependency_generators = + DependencyGenerators(); + for (vector::const_iterator iter = + dependency_generators.begin(); + iter != dependency_generators.end(); ++iter) { + if (!(*iter)->IsPublicDependency()) { + printer->Print("#import \"$header$.pbobjc.h\"\n", + "header", (*iter)->Path()); + } + } + printer->Print( + "// @@protoc_insertion_point(imports)\n" + "\n"); + if (IsFiltered()) { printer->Print( "// File empty because all messages, extensions and enum have been filtered.\n" @@ -232,22 +246,17 @@ void FileGenerator::GenerateSource(io::Printer *printer) { "\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", + "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("#import \"GPBProtocolBuffers_RuntimeSupport.h\"\n\n"); - - string header_file = Path() + ".pbobjc.h"; - printer->Print( - "#import \"$header_file$\"\n" - "\n" "#pragma mark - $root_class_name$\n" "\n" "@implementation $root_class_name$\n\n", - "header_file", header_file, "root_class_name", root_class_name_); bool generated_extensions = false; @@ -326,12 +335,7 @@ void FileGenerator::GenerateSource(io::Printer *printer) { " }\n" " return registry;\n" "}\n" - "\n" - "+ (void)load {\n" - " @autoreleasepool {\n" - " [self extensionRegistry]; // Construct extension registry.\n" - " }\n" - "}\n\n"); + "\n"); } } @@ -374,19 +378,31 @@ void FileGenerator::GenerateSource(io::Printer *printer) { iter != message_generators_.end(); ++iter) { (*iter)->GenerateSource(printer); } + + printer->Print( + "\n" + "// @@protoc_insertion_point(global_scope)\n"); } const string FileGenerator::Path() const { return FilePath(file_); } const vector &FileGenerator::DependencyGenerators() { if (file_->dependency_count() != dependency_generators_.size()) { + set public_import_names; + for (int i = 0; i < file_->public_dependency_count(); i++) { + public_import_names.insert(file_->public_dependency(i)->name()); + } for (int i = 0; i < file_->dependency_count(); i++) { FileGenerator *generator = new FileGenerator(file_->dependency(i)); + const string& name = file_->dependency(i)->name(); + bool public_import = (public_import_names.count(name) != 0); + generator->SetIsPublicDependency(public_import); dependency_generators_.push_back(generator); } } return dependency_generators_; } + } // namespace objectivec } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.h b/src/google/protobuf/compiler/objectivec/objectivec_file.h index fbd08eae..95d17bfd 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.h @@ -66,6 +66,12 @@ class FileGenerator { bool IsFiltered() const { return is_filtered_; } bool AreAllExtensionsFiltered() const { return all_extensions_filtered_; } + bool IsPublicDependency() const { return is_public_dep_; } + + protected: + void SetIsPublicDependency(bool is_public_dep) { + is_public_dep_ = is_public_dep; + } private: const FileDescriptor* file_; @@ -80,15 +86,16 @@ class FileGenerator { vector extension_generators_; bool is_filtered_; bool all_extensions_filtered_; - - void DetermineDependencies(set* dependencies); + bool is_public_dep_; const vector& DependencyGenerators(); GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(FileGenerator); }; + } // namespace objectivec } // namespace compiler } // namespace protobuf } // namespace google + #endif // GOOGLE_PROTOBUF_COMPILER_OBJECTIVEC_FILE_H__ diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index d4675f02..6d6e5959 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -41,7 +41,7 @@ #include // NOTE: src/google/protobuf/compiler/plugin.cc makes use of cerr for some -// error case, so it seem to be ok to use as a back door for errors. +// error cases, so it seems to be ok to use as a back door for errors. namespace google { namespace protobuf { @@ -53,6 +53,11 @@ 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 +// Objective C plugin, test failures were seen on TravisCI because isupper('A') +// was coming back false for some server's locale. This approach avoids any +// such issues. bool IsLower(const char c) { return ('a' <= c && c <= 'z'); @@ -205,10 +210,9 @@ const char* const kReservedWordList[] = { // Only need to add instance methods that may conflict with // method declared in protos. The main cases are methods // that take no arguments, or setFoo:/hasFoo: type methods. - // These are currently in the same order as in GPBMessage.h. - "unknownFields", "extensionRegistry", "isInitialized", - "data", "delimitedData", "serializedSize", - "descriptor", "extensionsCurrentlySet", "clear", "sortedExtensionsInUse", + "clear", "data", "delimitedData", "descriptor", "extensionRegistry", + "extensionsCurrentlySet", "isInitialized", "serializedSize", + "sortedExtensionsInUse", "unknownFields", // MacTypes.h names "Fixed", "Fract", "Size", "LogicalAddress", "PhysicalAddress", "ByteCount", @@ -335,7 +339,32 @@ string FilePath(const FileDescriptor* file) { string FileClassPrefix(const FileDescriptor* file) { // Default is empty string, no need to check has_objc_class_prefix. - return file->options().objc_class_prefix(); + string result = file->options().objc_class_prefix(); + return result; +} + +void ValidateObjCClassPrefix(const FileDescriptor* file) { + string prefix = file->options().objc_class_prefix(); + if (prefix.length() > 0) { + // 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. + if (!IsUpper(prefix[0])) { + cerr << endl + << "protoc:0: warning: Invalid 'option objc_class_prefix = \"" + << prefix << "\";' in '" << file->name() << "';" + << " it should start with a capital letter." + << endl; + cerr.flush(); + } + if (prefix.length() < 3) { + cerr << endl + << "protoc:0: warning: Invalid 'option objc_class_prefix = \"" + << prefix << "\";' in '" << file->name() << "';" + << " Apple recommends they should be at least 3 characters long." + << endl; + cerr.flush(); + } + } } string FileClassName(const FileDescriptor* file) { @@ -468,7 +497,7 @@ string OneofEnumName(const OneofDescriptor* descriptor) { const Descriptor* fieldDescriptor = descriptor->containing_type(); string name = ClassName(fieldDescriptor); name += "_" + UnderscoresToCamelCase(descriptor->name(), true) + "_OneOfCase"; - // No sanitize needed because it the OS never has names that end in OneOfCase. + // No sanitize needed because the OS never has names that end in _OneOfCase. return name; } @@ -560,6 +589,8 @@ string GetCapitalizedType(const FieldDescriptor* field) { return "Message"; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return NULL; } @@ -607,6 +638,8 @@ ObjectiveCType GetObjectiveCType(FieldDescriptor::Type field_type) { return OBJECTIVECTYPE_MESSAGE; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return OBJECTIVECTYPE_INT32; } @@ -683,6 +716,8 @@ string GPBValueFieldName(const FieldDescriptor* field) { return "valueMessage"; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return NULL; } @@ -753,6 +788,8 @@ string DefaultValue(const FieldDescriptor* field) { return "nil"; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return NULL; } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index ab030d15..29168937 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -62,6 +62,9 @@ string FileName(const FileDescriptor* file); // declared in the proto package. string FilePath(const FileDescriptor* file); +// Checks the prefix for a given file and outputs any warnings/errors needed. +void ValidateObjCClassPrefix(const FileDescriptor* file); + // Gets the name of the root class we'll generate in the file. This class // is not meant for external consumption, but instead contains helpers that // the rest of the the classes need diff --git a/src/google/protobuf/compiler/objectivec/objectivec_map_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_map_field.cc index cafdf39d..2987f3db 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_map_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_map_field.cc @@ -76,6 +76,8 @@ const char* MapEntryTypeName(const FieldDescriptor* descriptor, bool isKey) { return "Object"; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return NULL; } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_map_field.h b/src/google/protobuf/compiler/objectivec/objectivec_map_field.h index 8862dc35..173541f2 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_map_field.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_map_field.h @@ -48,7 +48,7 @@ class MapFieldGenerator : public RepeatedFieldGenerator { virtual void GenerateFieldDescriptionTypeSpecific(io::Printer* printer) const; protected: - MapFieldGenerator(const FieldDescriptor* descriptor); + explicit MapFieldGenerator(const FieldDescriptor* descriptor); virtual ~MapFieldGenerator(); private: diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message.cc b/src/google/protobuf/compiler/objectivec/objectivec_message.cc index f6a5852d..52e583bf 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_message.cc @@ -120,6 +120,8 @@ int OrderGroupForFieldDescriptor(const FieldDescriptor* descriptor) { return 1; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return 0; } @@ -188,7 +190,7 @@ MessageGenerator::MessageGenerator(const string& root_classname, extension_generators_.push_back( new ExtensionGenerator(class_name_, descriptor_->extension(i))); } - // No need to oneofs if this message is filtered + // 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); @@ -253,15 +255,24 @@ void MessageGenerator::GenerateStaticVariablesInitialization( } } -void MessageGenerator::DetermineDependencies(set* dependencies) { +void MessageGenerator::DetermineForwardDeclarations(set* fwd_decls) { if (!IsFiltered() && !IsMapEntryMessage(descriptor_)) { - dependencies->insert("@class " + class_name_); + 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 (fieldDescriptor->is_repeated()) { + continue; + } + field_generators_.get(fieldDescriptor) + .DetermineForwardDeclarations(fwd_decls); + } } for (vector::iterator iter = nested_message_generators_.begin(); iter != nested_message_generators_.end(); ++iter) { - (*iter)->DetermineDependencies(dependencies); + (*iter)->DetermineForwardDeclarations(fwd_decls); } } @@ -361,13 +372,13 @@ void MessageGenerator::GenerateMessageHeader(io::Printer* printer) { "classname", class_name_, "comments", message_comments); - vector seen_oneofs(descriptor_->oneof_decl_count(), false); + 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] = true; + seen_oneofs[oneof_index] = 1; oneof_generators_[oneof_index]->GeneratePublicCasePropertyDeclaration( printer); } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message.h b/src/google/protobuf/compiler/objectivec/objectivec_message.h index 5992d0cf..8d03c0b8 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_message.h @@ -63,7 +63,7 @@ class MessageGenerator { void GenerateMessageHeader(io::Printer* printer); void GenerateSource(io::Printer* printer); void GenerateExtensionRegistrationSource(io::Printer* printer); - void DetermineDependencies(set* dependencies); + void DetermineForwardDeclarations(set* fwd_decls); // This only speaks for this message, not sub message/enums. bool IsFiltered() const { return filter_reason_.length() > 0; } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc index 9c4a4e44..2e3bdfdb 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_message_field.cc @@ -65,6 +65,12 @@ MessageFieldGenerator::MessageFieldGenerator(const FieldDescriptor* descriptor) MessageFieldGenerator::~MessageFieldGenerator() {} +void MessageFieldGenerator::DetermineForwardDeclarations( + set* fwd_decls) const { + // Class name is already in "storage_type". + fwd_decls->insert("@class " + variable("storage_type")); +} + bool MessageFieldGenerator::WantsHasProperty(void) const { if (descriptor_->containing_oneof() != NULL) { // If in a oneof, it uses the oneofcase instead of a has bit. diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message_field.h b/src/google/protobuf/compiler/objectivec/objectivec_message_field.h index a1ac2d39..708ea566 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message_field.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_message_field.h @@ -44,10 +44,13 @@ class MessageFieldGenerator : public ObjCObjFieldGenerator { friend FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field); protected: - MessageFieldGenerator(const FieldDescriptor* descriptor); + explicit MessageFieldGenerator(const FieldDescriptor* descriptor); virtual ~MessageFieldGenerator(); virtual bool WantsHasProperty(void) const; + public: + virtual void DetermineForwardDeclarations(set* fwd_decls) const; + private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MessageFieldGenerator); }; @@ -56,7 +59,7 @@ class RepeatedMessageFieldGenerator : public RepeatedFieldGenerator { friend FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field); protected: - RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor); + explicit RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor); virtual ~RepeatedMessageFieldGenerator(); private: diff --git a/src/google/protobuf/compiler/objectivec/objectivec_oneof.h b/src/google/protobuf/compiler/objectivec/objectivec_oneof.h index 77b7f800..bcba82da 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_oneof.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_oneof.h @@ -49,7 +49,7 @@ namespace objectivec { class OneofGenerator { public: - OneofGenerator(const OneofDescriptor* descriptor); + explicit OneofGenerator(const OneofDescriptor* descriptor); ~OneofGenerator(); void SetOneofIndexBase(int index_base); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.cc b/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.cc index 54f94284..c185b66d 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.cc @@ -77,6 +77,8 @@ const char* PrimitiveTypeName(const FieldDescriptor* descriptor) { return NULL; } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return NULL; } @@ -108,6 +110,8 @@ const char* PrimitiveArrayTypeName(const FieldDescriptor* descriptor) { return ""; // Want NSArray } + // Some compilers report reaching end of function even though all cases of + // the enum are handed in the switch. GOOGLE_LOG(FATAL) << "Can't get here."; return NULL; } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.h b/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.h index b3599297..9bb79343 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_primitive_field.h @@ -44,7 +44,7 @@ class PrimitiveFieldGenerator : public SingleFieldGenerator { friend FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field); protected: - PrimitiveFieldGenerator(const FieldDescriptor* descriptor); + explicit PrimitiveFieldGenerator(const FieldDescriptor* descriptor); virtual ~PrimitiveFieldGenerator(); private: @@ -55,7 +55,7 @@ class PrimitiveObjFieldGenerator : public ObjCObjFieldGenerator { friend FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field); protected: - PrimitiveObjFieldGenerator(const FieldDescriptor* descriptor); + explicit PrimitiveObjFieldGenerator(const FieldDescriptor* descriptor); virtual ~PrimitiveObjFieldGenerator(); private: @@ -66,7 +66,7 @@ class RepeatedPrimitiveFieldGenerator : public RepeatedFieldGenerator { friend FieldGenerator* FieldGenerator::Make(const FieldDescriptor* field); protected: - RepeatedPrimitiveFieldGenerator(const FieldDescriptor* descriptor); + explicit RepeatedPrimitiveFieldGenerator(const FieldDescriptor* descriptor); virtual ~RepeatedPrimitiveFieldGenerator(); virtual void FinishInitialization(void); -- cgit v1.2.3