diff --git a/src/google/protobuf/compiler/objectivec/enum_field.cc b/src/google/protobuf/compiler/objectivec/enum_field.cc index 0188743711a3..197168c525ae 100644 --- a/src/google/protobuf/compiler/objectivec/enum_field.cc +++ b/src/google/protobuf/compiler/objectivec/enum_field.cc @@ -32,8 +32,14 @@ #include +#include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include "google/protobuf/compiler/objectivec/field.h" #include "google/protobuf/compiler/objectivec/names.h" +#include "google/protobuf/descriptor.h" #include "google/protobuf/io/printer.h" namespace google { @@ -135,6 +141,13 @@ void EnumFieldGenerator::DetermineForwardDeclarations( } } +void EnumFieldGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + if (descriptor_->file() != descriptor_->enum_type()->file()) { + deps->insert(descriptor_->enum_type()->file()); + } +} + RepeatedEnumFieldGenerator::RepeatedEnumFieldGenerator( const FieldDescriptor* descriptor) : RepeatedFieldGenerator(descriptor) { @@ -153,6 +166,13 @@ void RepeatedEnumFieldGenerator::EmitArrayComment(io::Printer* printer) const { // because `GPBEnumArray` isn't generic (like `NSArray` would be for messages) // and thus doesn't reference the type in the header. +void RepeatedEnumFieldGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + if (descriptor_->file() != descriptor_->enum_type()->file()) { + deps->insert(descriptor_->enum_type()->file()); + } +} + } // namespace objectivec } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/objectivec/enum_field.h b/src/google/protobuf/compiler/objectivec/enum_field.h index a52c23d349e6..a7ded294f8d7 100644 --- a/src/google/protobuf/compiler/objectivec/enum_field.h +++ b/src/google/protobuf/compiler/objectivec/enum_field.h @@ -34,7 +34,9 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" #include "google/protobuf/compiler/objectivec/field.h" +#include "google/protobuf/descriptor.h" namespace google { namespace protobuf { @@ -52,6 +54,8 @@ class EnumFieldGenerator : public SingleFieldGenerator { void GenerateCFunctionImplementations(io::Printer* printer) const override; void DetermineForwardDeclarations(absl::btree_set* fwd_decls, bool include_external_types) const override; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const override; protected: explicit EnumFieldGenerator(const FieldDescriptor* descriptor); @@ -63,6 +67,8 @@ class RepeatedEnumFieldGenerator : public RepeatedFieldGenerator { public: void EmitArrayComment(io::Printer* printer) const override; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const override; protected: explicit RepeatedEnumFieldGenerator(const FieldDescriptor* descriptor); diff --git a/src/google/protobuf/compiler/objectivec/extension.cc b/src/google/protobuf/compiler/objectivec/extension.cc index 28c04fb1568b..69712248d2e2 100644 --- a/src/google/protobuf/compiler/objectivec/extension.cc +++ b/src/google/protobuf/compiler/objectivec/extension.cc @@ -34,9 +34,13 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" +#include "absl/log/absl_check.h" #include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" #include "google/protobuf/compiler/objectivec/helpers.h" #include "google/protobuf/compiler/objectivec/names.h" +#include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/io/printer.h" @@ -128,6 +132,27 @@ void ExtensionGenerator::DetermineObjectiveCClassDefinitions( } } +void ExtensionGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + const Descriptor* extended_type = descriptor_->containing_type(); + if (descriptor_->file() != extended_type->file()) { + deps->insert(extended_type->file()); + } + + const ObjectiveCType objc_type = GetObjectiveCType(descriptor_); + if (objc_type == OBJECTIVECTYPE_MESSAGE) { + const Descriptor* value_msg_descriptor = descriptor_->message_type(); + if (descriptor_->file() != value_msg_descriptor->file()) { + deps->insert(value_msg_descriptor->file()); + } + } else if (objc_type == OBJECTIVECTYPE_ENUM) { + const EnumDescriptor* value_enum_descriptor = descriptor_->enum_type(); + if (descriptor_->file() != value_enum_descriptor->file()) { + deps->insert(value_enum_descriptor->file()); + } + } +} + void ExtensionGenerator::GenerateRegistrationSource( io::Printer* printer) const { printer->Emit({{"full_method_name", full_method_name_}}, diff --git a/src/google/protobuf/compiler/objectivec/extension.h b/src/google/protobuf/compiler/objectivec/extension.h index a2fd8f764475..c05a3a099558 100644 --- a/src/google/protobuf/compiler/objectivec/extension.h +++ b/src/google/protobuf/compiler/objectivec/extension.h @@ -34,6 +34,8 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" +#include "absl/strings/string_view.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/io/printer.h" @@ -56,6 +58,8 @@ class ExtensionGenerator { void GenerateRegistrationSource(io::Printer* printer) const; void DetermineObjectiveCClassDefinitions( absl::btree_set* fwd_decls) const; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const; private: std::string method_name_; diff --git a/src/google/protobuf/compiler/objectivec/field.cc b/src/google/protobuf/compiler/objectivec/field.cc index 7dd80e044c67..4778dec58301 100644 --- a/src/google/protobuf/compiler/objectivec/field.cc +++ b/src/google/protobuf/compiler/objectivec/field.cc @@ -34,16 +34,20 @@ #include #include +#include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" #include "absl/log/absl_check.h" #include "absl/log/absl_log.h" #include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" #include "google/protobuf/compiler/objectivec/enum_field.h" #include "google/protobuf/compiler/objectivec/helpers.h" #include "google/protobuf/compiler/objectivec/map_field.h" #include "google/protobuf/compiler/objectivec/message_field.h" #include "google/protobuf/compiler/objectivec/names.h" #include "google/protobuf/compiler/objectivec/primitive_field.h" +#include "google/protobuf/descriptor.h" #include "google/protobuf/io/printer.h" namespace google { @@ -237,6 +241,11 @@ void FieldGenerator::DetermineObjectiveCClassDefinitions( // Nothing } +void FieldGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + // Nothing +} + void FieldGenerator::GenerateFieldDescription(io::Printer* printer, bool include_default) const { // Printed in the same order as the structure decl. diff --git a/src/google/protobuf/compiler/objectivec/field.h b/src/google/protobuf/compiler/objectivec/field.h index fb77ed14e71f..6cb3cdb42657 100644 --- a/src/google/protobuf/compiler/objectivec/field.h +++ b/src/google/protobuf/compiler/objectivec/field.h @@ -37,7 +37,9 @@ #include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" #include "absl/strings/match.h" +#include "absl/strings/string_view.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/io/printer.h" @@ -73,6 +75,8 @@ class FieldGenerator { bool include_external_types) const; virtual void DetermineObjectiveCClassDefinitions( absl::btree_set* fwd_decls) const; + virtual void DetermineNeededFiles( + absl::flat_hash_set* deps) const; // Used during generation, not intended to be extended by subclasses. void GenerateFieldDescription(io::Printer* printer, diff --git a/src/google/protobuf/compiler/objectivec/file.cc b/src/google/protobuf/compiler/objectivec/file.cc index 6a1320b9b9db..cb146a0aa4c6 100644 --- a/src/google/protobuf/compiler/objectivec/file.cc +++ b/src/google/protobuf/compiler/objectivec/file.cc @@ -486,6 +486,7 @@ void FileGenerator::GenerateFile(io::Printer* p, GeneratedFileType file_type, /* for_bundled_proto = */ is_bundled_proto_); const std::string header_extension(kHeaderExtension); + absl::flat_hash_set file_imports; switch (file_type) { case GeneratedFileType::kHeader: // Generated files bundled with the library get minimal imports, @@ -500,11 +501,13 @@ void FileGenerator::GenerateFile(io::Printer* p, GeneratedFileType file_type, if (HeadersUseForwardDeclarations()) { // #import any headers for "public imports" in the proto file. for (int i = 0; i < file_->public_dependency_count(); i++) { - import_writer.AddFile(file_->public_dependency(i), header_extension); + file_imports.insert(file_->public_dependency(i)); } + } else if (generation_options_.generate_minimal_imports) { + DetermineNeededDeps(&file_imports, PublicDepsHandling::kForceInclude); } else { for (int i = 0; i < file_->dependency_count(); i++) { - import_writer.AddFile(file_->dependency(i), header_extension); + file_imports.insert(file_->dependency(i)); } } break; @@ -512,23 +515,36 @@ void FileGenerator::GenerateFile(io::Printer* p, GeneratedFileType file_type, import_writer.AddRuntimeImport("GPBProtocolBuffers_RuntimeSupport.h"); import_writer.AddFile(file_, header_extension); if (HeadersUseForwardDeclarations()) { - // #import the headers for anything that a plain dependency of this - // proto file (that means they were just an include, not a "public" - // include). - absl::flat_hash_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++) { - const FileDescriptor* dep = file_->dependency(i); - if (!public_import_names.contains(dep->name())) { - import_writer.AddFile(dep, header_extension); + if (generation_options_.generate_minimal_imports) { + DetermineNeededDeps(&file_imports, PublicDepsHandling::kExclude); + } else { + // #import the headers for anything that a plain dependency of this + // proto file (that means they were just an include, not a "public" + // include). + absl::flat_hash_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++) { + const FileDescriptor* dep = file_->dependency(i); + if (!public_import_names.contains(dep->name())) { + file_imports.insert(dep); + } } } } break; } + if (!file_imports.empty()) { + for (int i = 0; i < file_->dependency_count(); i++) { + const FileDescriptor* dep = file_->dependency(i); + if (file_imports.contains(dep)) { + import_writer.AddFile(file_->dependency(i), header_extension); + } + } + } + for (const auto& dep : file_options.extra_files_to_import) { import_writer.AddFile(dep, header_extension); } @@ -752,6 +768,34 @@ void FileGenerator::EmitFileDescription(io::Printer* p) const { p->Emit("\n"); } +void FileGenerator::DetermineNeededDeps( + absl::flat_hash_set* deps, + PublicDepsHandling public_deps_handling) const { + // This logic captures the deps that are needed for types thus removing the + // ones that are only deps because they provide the definitions for custom + // options. If protoc gets something like "import options" then this logic can + // go away as the non "import options" deps would be the ones needed. + + if (public_deps_handling == PublicDepsHandling::kForceInclude) { + for (int i = 0; i < file_->public_dependency_count(); i++) { + deps->insert(file_->public_dependency(i)); + } + } + + for (const auto& generator : message_generators_) { + generator->DetermineNeededFiles(deps); + } + for (const auto& generator : extension_generators_) { + generator->DetermineNeededFiles(deps); + } + + if (public_deps_handling == PublicDepsHandling::kExclude) { + for (int i = 0; i < file_->public_dependency_count(); i++) { + deps->erase(file_); + } + } +} + } // namespace objectivec } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/objectivec/file.h b/src/google/protobuf/compiler/objectivec/file.h index 012afb98c965..7fb3a6785a7c 100644 --- a/src/google/protobuf/compiler/objectivec/file.h +++ b/src/google/protobuf/compiler/objectivec/file.h @@ -121,6 +121,10 @@ class FileGenerator { const std::vector& deps_with_extensions) const; void EmitFileDescription(io::Printer* p) const; + enum class PublicDepsHandling : int { kAsUsed, kForceInclude, kExclude }; + void DetermineNeededDeps(absl::flat_hash_set* deps, + PublicDepsHandling public_deps_handling) const; + bool HeadersUseForwardDeclarations() const { // The bundled protos (WKTs) don't make use of forward declarations. return !is_bundled_proto_ && diff --git a/src/google/protobuf/compiler/objectivec/generator.cc b/src/google/protobuf/compiler/objectivec/generator.cc index 7e4a320dbb7a..4893d07f977b 100644 --- a/src/google/protobuf/compiler/objectivec/generator.cc +++ b/src/google/protobuf/compiler/objectivec/generator.cc @@ -274,6 +274,18 @@ bool ObjectiveCGenerator::GenerateAll( options[i].second); return false; } + } else if (options[i].first == "generate_minimal_imports") { + // Controls if minimal imports should be generated from a files imports. + // Since custom options require imports, they current cause generated + // imports even though there is nothing captured in the generated code, + // this provides smaller imports only for the things referenced. + if (!StringToBool(options[i].second, + &generation_options.generate_minimal_imports)) { + *error = + absl::StrCat("error: Unknown value for generate_minimal_imports: ", + options[i].second); + return false; + } } else if (options[i].first == "experimental_multi_source_generation") { // This is an experimental option, and could be removed or change at any // time; it is not documented in the README.md for that reason. diff --git a/src/google/protobuf/compiler/objectivec/helpers.h b/src/google/protobuf/compiler/objectivec/helpers.h index f963b430777d..8cbc5a20db5e 100644 --- a/src/google/protobuf/compiler/objectivec/helpers.h +++ b/src/google/protobuf/compiler/objectivec/helpers.h @@ -36,6 +36,7 @@ #include #include +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" diff --git a/src/google/protobuf/compiler/objectivec/map_field.cc b/src/google/protobuf/compiler/objectivec/map_field.cc index b7271c59277e..1917ae034815 100644 --- a/src/google/protobuf/compiler/objectivec/map_field.cc +++ b/src/google/protobuf/compiler/objectivec/map_field.cc @@ -34,10 +34,14 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" #include "absl/log/absl_log.h" #include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "google/protobuf/compiler/objectivec/field.h" #include "google/protobuf/compiler/objectivec/helpers.h" #include "google/protobuf/compiler/objectivec/names.h" +#include "google/protobuf/descriptor.h" namespace google { namespace protobuf { @@ -205,6 +209,24 @@ void MapFieldGenerator::DetermineObjectiveCClassDefinitions( } } +void MapFieldGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + const FieldDescriptor* value_descriptor = + descriptor_->message_type()->map_value(); + const ObjectiveCType value_objc_type = GetObjectiveCType(value_descriptor); + if (value_objc_type == OBJECTIVECTYPE_MESSAGE) { + const Descriptor* value_msg_descriptor = value_descriptor->message_type(); + if (descriptor_->file() != value_msg_descriptor->file()) { + deps->insert(value_msg_descriptor->file()); + } + } else if (value_objc_type == OBJECTIVECTYPE_ENUM) { + const EnumDescriptor* value_enum_descriptor = value_descriptor->enum_type(); + if (descriptor_->file() != value_enum_descriptor->file()) { + deps->insert(value_enum_descriptor->file()); + } + } +} + } // namespace objectivec } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/objectivec/map_field.h b/src/google/protobuf/compiler/objectivec/map_field.h index 3863933ff0d5..ccddf3868d7c 100644 --- a/src/google/protobuf/compiler/objectivec/map_field.h +++ b/src/google/protobuf/compiler/objectivec/map_field.h @@ -35,7 +35,9 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" #include "google/protobuf/compiler/objectivec/field.h" +#include "google/protobuf/descriptor.h" namespace google { namespace protobuf { @@ -59,6 +61,8 @@ class MapFieldGenerator : public RepeatedFieldGenerator { absl::btree_set* fwd_decls) const override; void DetermineForwardDeclarations(absl::btree_set* fwd_decls, bool include_external_types) const override; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const override; private: std::unique_ptr value_field_generator_; diff --git a/src/google/protobuf/compiler/objectivec/message.cc b/src/google/protobuf/compiler/objectivec/message.cc index 053f722f48c2..a7e6ff356cb2 100644 --- a/src/google/protobuf/compiler/objectivec/message.cc +++ b/src/google/protobuf/compiler/objectivec/message.cc @@ -36,10 +36,14 @@ #include #include +#include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" #include "absl/log/absl_log.h" #include "absl/strings/escaping.h" #include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" #include "google/protobuf/compiler/objectivec/extension.h" +#include "google/protobuf/compiler/objectivec/field.h" #include "google/protobuf/compiler/objectivec/helpers.h" #include "google/protobuf/compiler/objectivec/names.h" #include "google/protobuf/compiler/objectivec/oneof.h" @@ -278,6 +282,17 @@ void MessageGenerator::DetermineForwardDeclarations( } } +void MessageGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + if (IsMapEntryMessage(descriptor_)) { + return; + } + for (int i = 0; i < descriptor_->field_count(); i++) { + const FieldDescriptor* fieldDescriptor = descriptor_->field(i); + field_generators_.get(fieldDescriptor).DetermineNeededFiles(deps); + } +} + void MessageGenerator::DetermineObjectiveCClassDefinitions( absl::btree_set* fwd_decls) const { if (!IsMapEntryMessage(descriptor_)) { diff --git a/src/google/protobuf/compiler/objectivec/message.h b/src/google/protobuf/compiler/objectivec/message.h index d7fc83758b60..634398e469e5 100644 --- a/src/google/protobuf/compiler/objectivec/message.h +++ b/src/google/protobuf/compiler/objectivec/message.h @@ -37,6 +37,7 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" #include "google/protobuf/compiler/objectivec/field.h" #include "google/protobuf/compiler/objectivec/oneof.h" #include "google/protobuf/descriptor.h" @@ -68,6 +69,8 @@ class MessageGenerator { absl::btree_set* fwd_decls) const; void DetermineForwardDeclarations(absl::btree_set* fwd_decls, bool include_external_types) const; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const; // Checks if the message or a nested message includes a oneof definition. bool IncludesOneOfDefinition() const { return !oneof_generators_.empty(); } diff --git a/src/google/protobuf/compiler/objectivec/message_field.cc b/src/google/protobuf/compiler/objectivec/message_field.cc index 569e8c273bf9..9dbad9f007bf 100644 --- a/src/google/protobuf/compiler/objectivec/message_field.cc +++ b/src/google/protobuf/compiler/objectivec/message_field.cc @@ -34,8 +34,13 @@ #include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include "google/protobuf/compiler/objectivec/field.h" #include "google/protobuf/compiler/objectivec/helpers.h" #include "google/protobuf/compiler/objectivec/names.h" +#include "google/protobuf/descriptor.h" namespace google { namespace protobuf { @@ -86,6 +91,13 @@ void MessageFieldGenerator::DetermineObjectiveCClassDefinitions( fwd_decls->insert(ObjCClassDeclaration(variable("storage_type"))); } +void MessageFieldGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + if (descriptor_->file() != descriptor_->message_type()->file()) { + deps->insert(descriptor_->message_type()->file()); + } +} + RepeatedMessageFieldGenerator::RepeatedMessageFieldGenerator( const FieldDescriptor* descriptor) : RepeatedFieldGenerator(descriptor) { @@ -117,6 +129,13 @@ void RepeatedMessageFieldGenerator::DetermineObjectiveCClassDefinitions( fwd_decls->insert(ObjCClassDeclaration(variable("storage_type"))); } +void RepeatedMessageFieldGenerator::DetermineNeededFiles( + absl::flat_hash_set* deps) const { + if (descriptor_->file() != descriptor_->message_type()->file()) { + deps->insert(descriptor_->message_type()->file()); + } +} + } // namespace objectivec } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/objectivec/message_field.h b/src/google/protobuf/compiler/objectivec/message_field.h index 40028a0a7f0f..50fb175fe6a5 100644 --- a/src/google/protobuf/compiler/objectivec/message_field.h +++ b/src/google/protobuf/compiler/objectivec/message_field.h @@ -34,7 +34,9 @@ #include #include "absl/container/btree_set.h" +#include "absl/container/flat_hash_set.h" #include "google/protobuf/compiler/objectivec/field.h" +#include "google/protobuf/descriptor.h" namespace google { namespace protobuf { @@ -56,6 +58,8 @@ class MessageFieldGenerator : public ObjCObjFieldGenerator { bool include_external_types) const override; void DetermineObjectiveCClassDefinitions( absl::btree_set* fwd_decls) const override; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const override; }; class RepeatedMessageFieldGenerator : public RepeatedFieldGenerator { @@ -74,6 +78,8 @@ class RepeatedMessageFieldGenerator : public RepeatedFieldGenerator { bool include_external_types) const override; void DetermineObjectiveCClassDefinitions( absl::btree_set* fwd_decls) const override; + void DetermineNeededFiles( + absl::flat_hash_set* deps) const override; }; } // namespace objectivec diff --git a/src/google/protobuf/compiler/objectivec/options.h b/src/google/protobuf/compiler/objectivec/options.h index f597245c3529..af9e3a5a4b79 100644 --- a/src/google/protobuf/compiler/objectivec/options.h +++ b/src/google/protobuf/compiler/objectivec/options.h @@ -49,6 +49,8 @@ struct GenerationOptions { // TODO(thomasvl): Removing this so there is one less path to validate in // code generation. bool strip_custom_options = true; + // TODO(thomasvl): Eventually flip this default to true. + bool generate_minimal_imports = false; bool experimental_multi_source_generation = false; };