From e2416caf906a680a27bd89a91f14b2cdcaaa1634 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Tue, 25 Nov 2014 10:37:57 -0500 Subject: [PATCH 01/40] Fix java compilation issues when processing large .proto files Fix issues 579 and 501 on the code.google.com issues list. Specifically, large .proto files lead to too much static code, leading to a compilation error from javac: "code too large". This divides the code used in static initialization into multiple methods to avoid that error. Also, this incorporates the fix in issue 501 on the code.google.com issues list to call registry.add only once per extension. --- .../protobuf/compiler/java/java_extension.cc | 8 +- .../protobuf/compiler/java/java_extension.h | 8 +- .../protobuf/compiler/java/java_file.cc | 92 ++++++++++++++++--- .../protobuf/compiler/java/java_message.cc | 18 +++- .../protobuf/compiler/java/java_message.h | 6 +- 5 files changed, 107 insertions(+), 25 deletions(-) diff --git a/src/google/protobuf/compiler/java/java_extension.cc b/src/google/protobuf/compiler/java/java_extension.cc index 27cf416b20..4db7085e01 100644 --- a/src/google/protobuf/compiler/java/java_extension.cc +++ b/src/google/protobuf/compiler/java/java_extension.cc @@ -181,8 +181,9 @@ void ImmutableExtensionGenerator::Generate(io::Printer* printer) { } } -void ImmutableExtensionGenerator::GenerateNonNestedInitializationCode( +int ImmutableExtensionGenerator::GenerateNonNestedInitializationCode( io::Printer* printer) { + int bytecode_estimate = 0; if (descriptor_->extension_scope() == NULL && HasDescriptorMethods(descriptor_->file())) { // Only applies to non-nested, non-lite extensions. @@ -190,15 +191,18 @@ void ImmutableExtensionGenerator::GenerateNonNestedInitializationCode( "$name$.internalInit(descriptor.getExtensions().get($index$));\n", "name", UnderscoresToCamelCase(descriptor_), "index", SimpleItoa(descriptor_->index())); + bytecode_estimate += 21; } + return bytecode_estimate; } -void ImmutableExtensionGenerator::GenerateRegistrationCode( +int ImmutableExtensionGenerator::GenerateRegistrationCode( io::Printer* printer) { printer->Print( "registry.add($scope$.$name$);\n", "scope", scope_, "name", UnderscoresToCamelCase(descriptor_)); + return 7; } } // namespace java diff --git a/src/google/protobuf/compiler/java/java_extension.h b/src/google/protobuf/compiler/java/java_extension.h index f1701fb571..fac5ba00c9 100644 --- a/src/google/protobuf/compiler/java/java_extension.h +++ b/src/google/protobuf/compiler/java/java_extension.h @@ -67,8 +67,8 @@ class ExtensionGenerator { virtual ~ExtensionGenerator() {} virtual void Generate(io::Printer* printer) = 0; - virtual void GenerateNonNestedInitializationCode(io::Printer* printer) = 0; - virtual void GenerateRegistrationCode(io::Printer* printer) = 0; + virtual int GenerateNonNestedInitializationCode(io::Printer* printer) = 0; + virtual int GenerateRegistrationCode(io::Printer* printer) = 0; protected: static void InitTemplateVars(const FieldDescriptor* descriptor, @@ -88,8 +88,8 @@ class ImmutableExtensionGenerator : public ExtensionGenerator { virtual ~ImmutableExtensionGenerator(); virtual void Generate(io::Printer* printer); - virtual void GenerateNonNestedInitializationCode(io::Printer* printer); - virtual void GenerateRegistrationCode(io::Printer* printer); + virtual int GenerateNonNestedInitializationCode(io::Printer* printer); + virtual int GenerateRegistrationCode(io::Printer* printer); protected: const FieldDescriptor* descriptor_; diff --git a/src/google/protobuf/compiler/java/java_file.cc b/src/google/protobuf/compiler/java/java_file.cc index f1e3cf6788..9f285774d0 100644 --- a/src/google/protobuf/compiler/java/java_file.cc +++ b/src/google/protobuf/compiler/java/java_file.cc @@ -35,6 +35,7 @@ #include #include +#include #ifndef _SHARED_PTR_H #include #endif @@ -62,6 +63,19 @@ namespace java { namespace { +struct FieldDescriptorCompare { + bool operator ()(const FieldDescriptor* f1, const FieldDescriptor* f2) { + if(f1== NULL) { + return false; + } + if(f2 == NULL) { + return true; + } + return f1->full_name() < f2->full_name(); + } +}; + +typedef std::set FieldDescriptorSet; // Recursively searches the given message to collect extensions. // Returns true if all the extensions can be recognized. The extensions will be @@ -69,7 +83,7 @@ namespace { // Returns false when there are unknown fields, in which case the data in the // extensions output parameter is not reliable and should be discarded. bool CollectExtensions(const Message& message, - vector* extensions) { + FieldDescriptorSet* extensions) { const Reflection* reflection = message.GetReflection(); // There are unknown fields that could be extensions, thus this call fails. @@ -79,7 +93,7 @@ bool CollectExtensions(const Message& message, reflection->ListFields(message, &fields); for (int i = 0; i < fields.size(); i++) { - if (fields[i]->is_extension()) extensions->push_back(fields[i]); + if (fields[i]->is_extension()) extensions->insert(fields[i]); if (GetJavaType(fields[i]) == JAVATYPE_MESSAGE) { if (fields[i]->is_repeated()) { @@ -106,7 +120,7 @@ bool CollectExtensions(const Message& message, // in order to handle this case. void CollectExtensions(const FileDescriptorProto& file_proto, const DescriptorPool& alternate_pool, - vector* extensions, + FieldDescriptorSet* extensions, const string& file_data) { if (!CollectExtensions(file_proto, extensions)) { // There are unknown fields in the file_proto, which are probably @@ -139,6 +153,36 @@ void CollectExtensions(const FileDescriptorProto& file_proto, } } +// Our static initialization methods can become very, very large. +// So large that if we aren't careful we end up blowing the JVM's +// 64K bytes of bytecode/method. Fortunately, since these static +// methods are executed only once near the beginning of a program, +// there's usually plenty of stack space available and we can +// extend our methods by simply chaining them to another method +// with a tail call. This inserts the sequence call-next-method, +// end this one, begin-next-method as needed. +void MaybeRestartJavaMethod(io::Printer* printer, + int *bytecode_estimate, + int *method_num, + const char *chain_statement, + const char *method_decl) { + + // The goal here is to stay under 64K bytes of jvm bytecode/method, + // since otherwise we hit a hardcoded limit in the jvm and javac will + // then fail with the error "code too large". This limit lets our + // estimates be off by a factor of two and still we're okay. + static const int bytesPerMethod = 1<<15; // aka 32K + + if ((*bytecode_estimate) > bytesPerMethod) { + ++(*method_num); + printer->Print(chain_statement, "method_num", SimpleItoa(*method_num)); + printer->Outdent(); + printer->Print("}\n"); + printer->Print(method_decl, "method_num", SimpleItoa(*method_num)); + printer->Indent(); + *bytecode_estimate = 0; + } +} } // namespace @@ -270,9 +314,16 @@ void FileGenerator::Generate(io::Printer* printer) { printer->Print( "static {\n"); printer->Indent(); + int bytecode_estimate = 0; + int method_num = 0; for (int i = 0; i < file_->message_type_count(); i++) { - message_generators_[i]->GenerateStaticVariableInitializers(printer); + bytecode_estimate += message_generators_[i]->GenerateStaticVariableInitializers(printer); + MaybeRestartJavaMethod( + printer, + &bytecode_estimate, &method_num, + "_clinit_autosplit_$method_num$();\n", + "private static void _clinit_autosplit_$method_num$() {\n"); } printer->Outdent(); @@ -303,12 +354,24 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable( SharedCodeGenerator shared_code_generator(file_); shared_code_generator.GenerateDescriptors(printer); + int bytecode_estimate = 0; + int method_num = 0; for (int i = 0; i < file_->message_type_count(); i++) { - message_generators_[i]->GenerateStaticVariableInitializers(printer); + bytecode_estimate += message_generators_[i]->GenerateStaticVariableInitializers(printer); + MaybeRestartJavaMethod( + printer, + &bytecode_estimate, &method_num, + "_clinit_autosplit_dinit_$method_num$();\n", + "private static void _clinit_autosplit_dinit_$method_num$() {\n"); } for (int i = 0; i < file_->extension_count(); i++) { - extension_generators_[i]->GenerateNonNestedInitializationCode(printer); + bytecode_estimate += extension_generators_[i]->GenerateNonNestedInitializationCode(printer); + MaybeRestartJavaMethod( + printer, + &bytecode_estimate, &method_num, + "_clinit_autosplit_dinit_$method_num$();\n", + "private static void _clinit_autosplit_dinit_$method_num$() {\n"); } // Proto compiler builds a DescriptorPool, which holds all the descriptors to @@ -330,7 +393,7 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable( file_->CopyTo(&file_proto); string file_data; file_proto.SerializeToString(&file_data); - vector extensions; + FieldDescriptorSet extensions; CollectExtensions(file_proto, *file_->pool(), &extensions, file_data); if (extensions.size() > 0) { @@ -339,10 +402,17 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable( printer->Print( "com.google.protobuf.ExtensionRegistry registry =\n" " com.google.protobuf.ExtensionRegistry.newInstance();\n"); - for (int i = 0; i < extensions.size(); i++) { + FieldDescriptorSet::iterator it; + for (it = extensions.begin(); it != extensions.end(); it++) { google::protobuf::scoped_ptr generator( - generator_factory_->NewExtensionGenerator(extensions[i])); - generator->GenerateRegistrationCode(printer); + generator_factory_->NewExtensionGenerator(*it)); + bytecode_estimate += generator->GenerateRegistrationCode(printer); + MaybeRestartJavaMethod( + printer, + &bytecode_estimate, &method_num, + "_clinit_autosplit_dinit_$method_num$(registry);\n", + "private static void _clinit_autosplit_dinit_$method_num$(\n" + " com.google.protobuf.ExtensionRegistry registry) {\n"); } printer->Print( "com.google.protobuf.Descriptors.FileDescriptor\n" @@ -394,7 +464,7 @@ void FileGenerator::GenerateDescriptorInitializationCodeForMutable(io::Printer* file_->CopyTo(&file_proto); string file_data; file_proto.SerializeToString(&file_data); - vector extensions; + FieldDescriptorSet extensions; CollectExtensions(file_proto, *file_->pool(), &extensions, file_data); if (extensions.size() > 0) { diff --git a/src/google/protobuf/compiler/java/java_message.cc b/src/google/protobuf/compiler/java/java_message.cc index 1171b718b6..ed94d6e414 100644 --- a/src/google/protobuf/compiler/java/java_message.cc +++ b/src/google/protobuf/compiler/java/java_message.cc @@ -124,7 +124,7 @@ void ImmutableMessageGenerator::GenerateStaticVariables(io::Printer* printer) { // The descriptor for this type. printer->Print(vars, - "$private$static final com.google.protobuf.Descriptors.Descriptor\n" + "$private$static com.google.protobuf.Descriptors.Descriptor\n" " internal_$identifier$_descriptor;\n"); // And the FieldAccessorTable. @@ -139,8 +139,9 @@ void ImmutableMessageGenerator::GenerateStaticVariables(io::Printer* printer) { } } -void ImmutableMessageGenerator::GenerateStaticVariableInitializers( +int ImmutableMessageGenerator::GenerateStaticVariableInitializers( io::Printer* printer) { + int bytecode_estimate = 0; if (HasDescriptorMethods(descriptor_)) { map vars; vars["identifier"] = UniqueFileScopeIdentifier(descriptor_); @@ -156,22 +157,25 @@ void ImmutableMessageGenerator::GenerateStaticVariableInitializers( printer->Print(vars, "internal_$identifier$_descriptor =\n" " getDescriptor().getMessageTypes().get($index$);\n"); + bytecode_estimate += 30; } else { printer->Print(vars, "internal_$identifier$_descriptor =\n" " internal_$parent$_descriptor.getNestedTypes().get($index$);\n"); + bytecode_estimate += 30; } // And the FieldAccessorTable. - GenerateFieldAccessorTableInitializer(printer); + bytecode_estimate += GenerateFieldAccessorTableInitializer(printer); } // Generate static member initializers for all nested types. for (int i = 0; i < descriptor_->nested_type_count(); i++) { // TODO(kenton): Reuse MessageGenerator objects? - ImmutableMessageGenerator(descriptor_->nested_type(i), context_) + bytecode_estimate += ImmutableMessageGenerator(descriptor_->nested_type(i), context_) .GenerateStaticVariableInitializers(printer); } + return bytecode_estimate; } void ImmutableMessageGenerator:: @@ -191,8 +195,9 @@ GenerateFieldAccessorTable(io::Printer* printer) { " internal_$identifier$_fieldAccessorTable;\n"); } -void ImmutableMessageGenerator:: +int ImmutableMessageGenerator:: GenerateFieldAccessorTableInitializer(io::Printer* printer) { + int bytecode_estimate = 10; printer->Print( "internal_$identifier$_fieldAccessorTable = new\n" " com.google.protobuf.GeneratedMessage.FieldAccessorTable(\n" @@ -203,6 +208,7 @@ GenerateFieldAccessorTableInitializer(io::Printer* printer) { for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* field = descriptor_->field(i); const FieldGeneratorInfo* info = context_->GetFieldGeneratorInfo(field); + bytecode_estimate += 6; printer->Print( "\"$field_name$\", ", "field_name", info->capitalized_name); @@ -210,11 +216,13 @@ GenerateFieldAccessorTableInitializer(io::Printer* printer) { for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { const OneofDescriptor* oneof = descriptor_->oneof_decl(i); const OneofGeneratorInfo* info = context_->GetOneofGeneratorInfo(oneof); + bytecode_estimate += 6; printer->Print( "\"$oneof_name$\", ", "oneof_name", info->capitalized_name); } printer->Print("});\n"); + return bytecode_estimate; } // =================================================================== diff --git a/src/google/protobuf/compiler/java/java_message.h b/src/google/protobuf/compiler/java/java_message.h index 91eb28763c..91e529ccfd 100644 --- a/src/google/protobuf/compiler/java/java_message.h +++ b/src/google/protobuf/compiler/java/java_message.h @@ -68,7 +68,7 @@ class MessageGenerator { // Output code which initializes the static variables generated by // GenerateStaticVariables(). - virtual void GenerateStaticVariableInitializers(io::Printer* printer) = 0; + virtual int GenerateStaticVariableInitializers(io::Printer* printer) = 0; // Generate the class itself. virtual void Generate(io::Printer* printer) = 0; @@ -97,7 +97,7 @@ class ImmutableMessageGenerator : public MessageGenerator { virtual void GenerateInterface(io::Printer* printer); virtual void GenerateExtensionRegistrationCode(io::Printer* printer); virtual void GenerateStaticVariables(io::Printer* printer); - virtual void GenerateStaticVariableInitializers(io::Printer* printer); + virtual int GenerateStaticVariableInitializers(io::Printer* printer); private: enum UseMemoization { @@ -106,7 +106,7 @@ class ImmutableMessageGenerator : public MessageGenerator { }; void GenerateFieldAccessorTable(io::Printer* printer); - void GenerateFieldAccessorTableInitializer(io::Printer* printer); + int GenerateFieldAccessorTableInitializer(io::Printer* printer); void GenerateMessageSerializationMethods(io::Printer* printer); void GenerateParseFromMethods(io::Printer* printer); From 153a226a2aa4c98cce932350ff74b8b3eb089e28 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Wed, 8 Apr 2015 09:53:26 -0400 Subject: [PATCH 02/40] CR feedback: spacing, document return values --- src/google/protobuf/compiler/java/java_extension.h | 4 ++++ src/google/protobuf/compiler/java/java_file.cc | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/java/java_extension.h b/src/google/protobuf/compiler/java/java_extension.h index fac5ba00c9..bdd4226379 100644 --- a/src/google/protobuf/compiler/java/java_extension.h +++ b/src/google/protobuf/compiler/java/java_extension.h @@ -67,7 +67,11 @@ class ExtensionGenerator { virtual ~ExtensionGenerator() {} virtual void Generate(io::Printer* printer) = 0; + + // Returns an estimate of the number of bytes the printed code will compile to virtual int GenerateNonNestedInitializationCode(io::Printer* printer) = 0; + + // Returns an estimate of the number of bytes the printed code will compile to virtual int GenerateRegistrationCode(io::Printer* printer) = 0; protected: diff --git a/src/google/protobuf/compiler/java/java_file.cc b/src/google/protobuf/compiler/java/java_file.cc index 9f285774d0..4a1f452988 100644 --- a/src/google/protobuf/compiler/java/java_file.cc +++ b/src/google/protobuf/compiler/java/java_file.cc @@ -65,7 +65,7 @@ namespace { struct FieldDescriptorCompare { bool operator ()(const FieldDescriptor* f1, const FieldDescriptor* f2) { - if(f1== NULL) { + if(f1 == NULL) { return false; } if(f2 == NULL) { From 0f1393d66ed0ceb81ea784e050b7b58524b53331 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Thu, 9 Apr 2015 12:20:55 -0400 Subject: [PATCH 03/40] More 'int' return value documentation --- src/google/protobuf/compiler/java/java_message.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/java/java_message.h b/src/google/protobuf/compiler/java/java_message.h index 91e529ccfd..4c4c137a81 100644 --- a/src/google/protobuf/compiler/java/java_message.h +++ b/src/google/protobuf/compiler/java/java_message.h @@ -67,7 +67,7 @@ class MessageGenerator { virtual void GenerateStaticVariables(io::Printer* printer) = 0; // Output code which initializes the static variables generated by - // GenerateStaticVariables(). + // GenerateStaticVariables(). Returns an estimate of bytecode size. virtual int GenerateStaticVariableInitializers(io::Printer* printer) = 0; // Generate the class itself. @@ -97,6 +97,8 @@ class ImmutableMessageGenerator : public MessageGenerator { virtual void GenerateInterface(io::Printer* printer); virtual void GenerateExtensionRegistrationCode(io::Printer* printer); virtual void GenerateStaticVariables(io::Printer* printer); + + // Returns an estimate of the number of bytes the printed code will compile to virtual int GenerateStaticVariableInitializers(io::Printer* printer); private: @@ -106,6 +108,8 @@ class ImmutableMessageGenerator : public MessageGenerator { }; void GenerateFieldAccessorTable(io::Printer* printer); + + // Returns an estimate of the number of bytes the printed code will compile to int GenerateFieldAccessorTableInitializer(io::Printer* printer); void GenerateMessageSerializationMethods(io::Printer* printer); From 420f938bac63af49c00292c7c09eb0d237ce4e0b Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 16 Apr 2015 12:50:39 -0700 Subject: [PATCH 04/40] Added conformance test support for Java. Change-Id: I4c81808e6ace77d2b5737a43417045321b0b10f0 --- conformance/ConformanceJava.java | 120 +++++++++++++++++++++++++ conformance/Makefile.am | 31 +++++-- conformance/conformance.proto | 1 + conformance/conformance_test_runner.cc | 4 +- 4 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 conformance/ConformanceJava.java diff --git a/conformance/ConformanceJava.java b/conformance/ConformanceJava.java new file mode 100644 index 0000000000..c1a53141fc --- /dev/null +++ b/conformance/ConformanceJava.java @@ -0,0 +1,120 @@ + +import com.google.protobuf.conformance.Conformance; +import com.google.protobuf.InvalidProtocolBufferException; + +class ConformanceJava { + private int testCount = 0; + + private boolean readFromStdin(byte[] buf, int len) throws Exception { + int ofs = 0; + while (len > 0) { + int read = System.in.read(buf, ofs, len); + if (read == -1) { + return false; // EOF + } + ofs += read; + len -= read; + } + + return true; + } + + private void writeToStdout(byte[] buf) throws Exception { + System.out.write(buf); + } + + // Returns -1 on EOF (the actual values will always be positive). + private int readLittleEndianIntFromStdin() throws Exception { + byte[] buf = new byte[4]; + if (!readFromStdin(buf, 4)) { + return -1; + } + return buf[0] | (buf[1] << 1) | (buf[2] << 2) | (buf[3] << 3); + } + + private void writeLittleEndianIntToStdout(int val) throws Exception { + byte[] buf = new byte[4]; + buf[0] = (byte)val; + buf[1] = (byte)(val >> 8); + buf[2] = (byte)(val >> 16); + buf[3] = (byte)(val >> 24); + writeToStdout(buf); + } + + private Conformance.ConformanceResponse doTest(Conformance.ConformanceRequest request) { + Conformance.TestAllTypes testMessage; + + switch (request.getPayloadCase()) { + case PROTOBUF_PAYLOAD: { + try { + testMessage = Conformance.TestAllTypes.parseFrom(request.getProtobufPayload()); + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder().setParseError(e.getMessage()).build(); + } + break; + } + case JSON_PAYLOAD: { + return Conformance.ConformanceResponse.newBuilder().setRuntimeError("JSON not yet supported.").build(); + } + case PAYLOAD_NOT_SET: { + throw new RuntimeException("Request didn't have payload."); + } + + default: { + throw new RuntimeException("Unexpected payload case."); + } + } + + switch (request.getRequestedOutput()) { + case UNSPECIFIED: + throw new RuntimeException("Unspecified output format."); + + case PROTOBUF: + return Conformance.ConformanceResponse.newBuilder().setProtobufPayload(testMessage.toByteString()).build(); + + case JSON: + return Conformance.ConformanceResponse.newBuilder().setRuntimeError("JSON not yet supported.").build(); + + default: { + throw new RuntimeException("Unexpected request output."); + } + } + } + + private boolean doTestIo() throws Exception { + int bytes = readLittleEndianIntFromStdin(); + + if (bytes == -1) { + return false; // EOF + } + + byte[] serializedInput = new byte[bytes]; + + if (!readFromStdin(serializedInput, bytes)) { + throw new RuntimeException("Unexpected EOF from test program."); + } + + Conformance.ConformanceRequest request = + Conformance.ConformanceRequest.parseFrom(serializedInput); + Conformance.ConformanceResponse response = doTest(request); + byte[] serializedOutput = response.toByteArray(); + + writeLittleEndianIntToStdout(serializedOutput.length); + writeToStdout(serializedOutput); + + return true; + } + + public void run() throws Exception { + while (doTestIo()) { + // Empty. + } + + System.err.println("ConformanceJava: received EOF from test runner after " + + this.testCount + " tests"); + } + + public static void main(String[] args) throws Exception { + new ConformanceJava().run(); + } +} diff --git a/conformance/Makefile.am b/conformance/Makefile.am index 0c4eae758a..59bb257674 100644 --- a/conformance/Makefile.am +++ b/conformance/Makefile.am @@ -21,30 +21,43 @@ conformance_cpp_CPPFLAGS = -I$(top_srcdir)/src if USE_EXTERNAL_PROTOC -unittest_proto_middleman: $(protoc_inputs) - $(PROTOC) -I$(srcdir) --cpp_out=. $^ - touch unittest_proto_middleman +protoc_middleman: $(protoc_inputs) + $(PROTOC) -I$(srcdir) --cpp_out=. --java_out=. $^ + touch protoc_middleman else # We have to cd to $(srcdir) before executing protoc because $(protoc_inputs) is # relative to srcdir, which may not be the same as the current directory when # building out-of-tree. -unittest_proto_middleman: $(top_srcdir)/src/protoc$(EXEEXT) $(protoc_inputs) - oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. --cpp_out=$$oldpwd $(protoc_inputs) ) - touch unittest_proto_middleman +protoc_middleman: $(top_srcdir)/src/protoc$(EXEEXT) $(protoc_inputs) + oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. --cpp_out=$$oldpwd --java_out=$$oldpwd $(protoc_inputs) ) + touch protoc_middleman endif -$(protoc_outputs): unittest_proto_middleman +$(protoc_outputs): protoc_middleman BUILT_SOURCES = $(protoc_outputs) -CLEANFILES = $(protoc_outputs) unittest_proto_middleman +CLEANFILES = $(protoc_outputs) protoc_middleman javac_middleman conformance-java MAINTAINERCLEANFILES = \ Makefile.in +javac_middleman: ConformanceJava.java protoc_middleman + javac ConformanceJava.java com/google/protobuf/conformance/Conformance.java + @touch javac_middleman + +conformance-java: javac_middleman + @echo "Writing shortcut script conformance-java..." + @echo '#! /bin/sh' > conformance-java + @echo 'java -classpath .:$$CLASSPATH ConformanceJava "$$@"' >> conformance-java + @chmod +x conformance-java + # Targets for actually running tests. -test_cpp: unittest_proto_middleman conformance-test-runner conformance-cpp +test_cpp: protoc_middleman conformance-test-runner conformance-cpp ./conformance-test-runner ./conformance-cpp + +test_java: protoc_middleman conformance-test-runner conformance-java + ./conformance-test-runner ./conformance-java diff --git a/conformance/conformance.proto b/conformance/conformance.proto index 0fb66cf932..7b67689851 100644 --- a/conformance/conformance.proto +++ b/conformance/conformance.proto @@ -30,6 +30,7 @@ syntax = "proto3"; package conformance; +option java_package = "com.google.protobuf.conformance"; // This defines the conformance testing protocol. This protocol exists between // the conformance test suite itself and the code being tested. For each test, diff --git a/conformance/conformance_test_runner.cc b/conformance/conformance_test_runner.cc index 21277700bd..ca5877bde7 100644 --- a/conformance/conformance_test_runner.cc +++ b/conformance/conformance_test_runner.cc @@ -48,9 +48,9 @@ // Every test consists of a ConformanceRequest/ConformanceResponse // request/reply pair. The protocol on the pipe is simply: // -// 1. tester sends 4-byte length N +// 1. tester sends 4-byte length N (little endian) // 2. tester sends N bytes representing a ConformanceRequest proto -// 3. testee sends 4-byte length M +// 3. testee sends 4-byte length M (little endian) // 4. testee sends M bytes representing a ConformanceResponse proto #include From 90a7ed6fcc0891b0cf1d92c5c418990e35a6b5a7 Mon Sep 17 00:00:00 2001 From: Kun Zhang Date: Thu, 16 Apr 2015 17:30:07 -0700 Subject: [PATCH 05/40] A few fix-ups. 1. make google/protobuf/stubs/pbconfig.h before making protoc, otherwise it won't build a freshly checked-out code. 2. Document the build environments that have been tested to work. 3. Add support for MINGW64 --- protoc-artifacts/README.md | 10 +++++++--- protoc-artifacts/build-protoc.sh | 5 ++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/protoc-artifacts/README.md b/protoc-artifacts/README.md index 3a530197cd..1ed1435edf 100644 --- a/protoc-artifacts/README.md +++ b/protoc-artifacts/README.md @@ -63,9 +63,6 @@ deployment for all platforms. Currently the following platforms are supported: - MSYS with MinGW32 (x86_32 only) - MacOSX (x86_32 and x86_64) -Remove any ``SNAPSHOT`` or ``pre`` suffix from the version string before -deploying. - Use the following command to deploy artifacts for the host platform to a staging repository. ``` @@ -118,3 +115,10 @@ stored: ``` + +### Tested build environments +We have succesfully built artifacts on the following environments: +- Linux x86_32 and x86_64: Ubuntu 14.04.2 64-bit +- Windows x86_32: MSYS with ``mingw32-gcc-g++ 4.8.1-4`` on Windows 7 64-bit +- Windows x86_64: Cygwin64 with ``mingw64-x86_64-gcc-g++ 4.8.3-1`` on Windows 7 64-bit +- Mac OS X x86_32 and x86_64: Mac OS X 10.9.5 diff --git a/protoc-artifacts/build-protoc.sh b/protoc-artifacts/build-protoc.sh index 96ca97c281..2f67c50837 100755 --- a/protoc-artifacts/build-protoc.sh +++ b/protoc-artifacts/build-protoc.sh @@ -158,6 +158,9 @@ if [[ "$(uname)" == CYGWIN* ]]; then elif [[ "$(uname)" == MINGW32* ]]; then assertEq "$OS" windows $LINENO assertEq "$ARCH" x86_32 $LINENO +elif [[ "$(uname)" == MINGW64* ]]; then + assertEq "$OS" windows $LINENO + assertEq "$ARCH" x86_64 $LINENO elif [[ "$(uname)" == Linux* ]]; then if [[ "$OS" == linux ]]; then if [[ "$ARCH" == x86_64 ]]; then @@ -209,7 +212,7 @@ export CXXFLAGS LDFLAGS TARGET_FILE=target/protoc.exe cd "$WORKING_DIR"/.. && ./configure $CONFIGURE_ARGS && - cd src && make clean && make $MAKE_TARGET && + cd src && make clean && make google/protobuf/stubs/pbconfig.h $MAKE_TARGET && cd "$WORKING_DIR" && mkdir -p target && (cp ../src/protoc $TARGET_FILE || cp ../src/protoc.exe $TARGET_FILE) || exit 1 From ca9d1a053a8590caa1a1f81491b0381f052fa734 Mon Sep 17 00:00:00 2001 From: Feng Xiao Date: Mon, 20 Apr 2015 11:30:31 -0700 Subject: [PATCH 06/40] Include all well-known type protos. Change-Id: I122f1cee71a8a739ea603e52582cb0fa9698f0ed --- src/google/protobuf/any.proto | 12 +- src/google/protobuf/api.proto | 104 ++++++++++++ src/google/protobuf/duration.proto | 10 +- src/google/protobuf/empty.proto | 49 ++++++ src/google/protobuf/source_context.proto | 45 ++++++ src/google/protobuf/struct.proto | 17 +- src/google/protobuf/timestamp.proto | 13 +- src/google/protobuf/type.proto | 196 +++++++++++++++++++++++ 8 files changed, 425 insertions(+), 21 deletions(-) create mode 100644 src/google/protobuf/api.proto create mode 100644 src/google/protobuf/empty.proto create mode 100644 src/google/protobuf/source_context.proto create mode 100644 src/google/protobuf/type.proto diff --git a/src/google/protobuf/any.proto b/src/google/protobuf/any.proto index bf2aa0a930..3aba15b3d1 100644 --- a/src/google/protobuf/any.proto +++ b/src/google/protobuf/any.proto @@ -40,7 +40,8 @@ option java_package = "com.google.protobuf"; // `Any` contains an arbitrary serialized message along with a URL // that describes the type of the serialized message. // -// +// The proto runtimes and/or compiler will eventually +// provide utilities to pack/unpack Any values (projected Q1/15). // // # JSON // The JSON representation of an `Any` value uses the regular @@ -76,21 +77,24 @@ message Any { // For URLs which use the schema `http`, `https`, or no schema, the // following restrictions and interpretations apply: // - // * If no schema is provided, https is assumed. + // * If no schema is provided, `https` is assumed. // * The last segment of the URL's path must represent the fully // qualified name of the type (as in `path/google.protobuf.Duration`). // * An HTTP GET on the URL must yield a [google.protobuf.Type][google.protobuf.Type] // value in binary format, or produce an error. // * Applications are allowed to cache lookup results based on the // URL, or have them precompiled into a binary to avoid any - // lookup. Therefore, binary compatibility need to be preserved + // lookup. Therefore, binary compatibility needs to be preserved // on changes to types. (Use versioned type names to manage // breaking changes.) // // Schemas other than `http`, `https` (or the empty schema) might be // used with implementation specific semantics. // - // + // Types originating from the `google.*` package + // namespace should use `type.googleapis.com/full.type.name` (without + // schema and path). A type service will eventually become available which + // serves those URLs (projected Q2/15). string type_url = 1; // Must be valid serialized data of the above specified type. diff --git a/src/google/protobuf/api.proto b/src/google/protobuf/api.proto new file mode 100644 index 0000000000..00857f55f9 --- /dev/null +++ b/src/google/protobuf/api.proto @@ -0,0 +1,104 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto3"; + +package google.protobuf; + +import "google/protobuf/source_context.proto"; +import "google/protobuf/type.proto"; + +option java_multiple_files = true; +option java_outer_classname = "ApiProto"; +option java_package = "com.google.protobuf"; + + +// Api is a light-weight descriptor for a protocol buffer service. +message Api { + // The fully qualified name of this api, including package name + // followed by the api's simple name. + string name = 1; + + // The methods of this api, in unspecified order. + repeated Method methods = 2; + + // Any metadata attached to the API. + repeated Option options = 3; + + // A version string for this api. If specified, must have the form + // `major-version.minor-version`, as in `1.10`. If the minor version + // is omitted, it defaults to zero. If the entire version field is + // empty, the major version is derived from the package name, as + // outlined below. If the field is not empty, the version in the + // package name will be verified to be consistent with what is + // provided here. + // + // The versioning schema uses [semantic + // versioning](http://semver.org) where the major version number + // indicates a breaking change and the minor version an additive, + // non-breaking change. Both version numbers are signals to users + // what to expect from different versions, and should be carefully + // chosen based on the product plan. + // + // The major version is also reflected in the package name of the + // API, which must end in `v`, as in + // `google.feature.v1`. For major versions 0 and 1, the suffix can + // be omitted. Zero major versions must only be used for + // experimental, none-GA apis. + // + // See also: [design doc](http://go/api-versioning). + // + // + string version = 4; + + // Source context for the protocol buffer service represented by this + // message. + SourceContext source_context = 5; +} + +// Method represents a method of an api. +message Method { + // The simple name of this method. + string name = 1; + + // A URL of the input message type. + string request_type_url = 2; + + // If true, the request is streamed. + bool request_streaming = 3; + + // The URL of the output message type. + string response_type_url = 4; + + // If true, the response is streamed. + bool response_streaming = 5; + + // Any metadata attached to the method. + repeated Option options = 6; +} diff --git a/src/google/protobuf/duration.proto b/src/google/protobuf/duration.proto index 868c732fb4..c6dd4adc45 100644 --- a/src/google/protobuf/duration.proto +++ b/src/google/protobuf/duration.proto @@ -44,7 +44,7 @@ option java_package = "com.google.protobuf"; // two Timestamp values is a Duration and it can be added or subtracted // from a Timestamp. Range is approximately +-10,000 years. // -// Example 1: compute Duration from two Timestamps in pseudo code. +// Example 1: Compute Duration from two Timestamps in pseudo code. // // Timestamp start = ...; // Timestamp end = ...; @@ -61,7 +61,7 @@ option java_package = "com.google.protobuf"; // duration.nanos += 1000000000; // } // -// Example 2: compute Timestamp from Timestamp + Duration in pseudo code. +// Example 2: Compute Timestamp from Timestamp + Duration in pseudo code. // // Timestamp start = ...; // Duration duration = ...; @@ -85,9 +85,9 @@ message Duration { // Signed fractions of a second at nanosecond resolution of the span // of time. Durations less than one second are represented with a 0 - // seconds field and a positive or negative nanos field. For durations - // of one second or more, a non-zero value for the nanos field must be - // of the same sign as the seconds field. Must be from -999,999,999 + // `seconds` field and a positive or negative `nanos` field. For durations + // of one second or more, a non-zero value for the `nanos` field must be + // of the same sign as the `seconds` field. Must be from -999,999,999 // to +999,999,999 inclusive. int32 nanos = 2; } diff --git a/src/google/protobuf/empty.proto b/src/google/protobuf/empty.proto new file mode 100644 index 0000000000..46087d59b8 --- /dev/null +++ b/src/google/protobuf/empty.proto @@ -0,0 +1,49 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto3"; + +package google.protobuf; + +option java_multiple_files = true; +option java_outer_classname = "EmptyProto"; +option java_package = "com.google.protobuf"; + + +// A generic empty message that you can re-use to avoid defining duplicated +// empty messages in your APIs. A typical example is to use it as the request +// or the response type of an API method. For instance: +// +// service Foo { +// rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty); +// } +// +message Empty { + +} diff --git a/src/google/protobuf/source_context.proto b/src/google/protobuf/source_context.proto new file mode 100644 index 0000000000..a3874d6add --- /dev/null +++ b/src/google/protobuf/source_context.proto @@ -0,0 +1,45 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto3"; + +package google.protobuf; + +option java_multiple_files = true; +option java_outer_classname = "SourceContextProto"; +option java_package = "com.google.protobuf"; + + +// `SourceContext` represents information about the source of a +// protobuf element, like the file in which it is defined. +message SourceContext { + // The path-qualified name of the .proto file that contained the associated + // protobuf element. For example: `"google/protobuf/source.proto"`. + string file_name = 1; +} diff --git a/src/google/protobuf/struct.proto b/src/google/protobuf/struct.proto index 9f27eb4354..3a90ff372a 100644 --- a/src/google/protobuf/struct.proto +++ b/src/google/protobuf/struct.proto @@ -56,28 +56,33 @@ message Value { oneof kind { // Represents a null value. NullValue null_value = 1; + // Represents a double value. double number_value = 2; + // Represents a string value. string string_value = 3; + // Represents a boolean value. bool bool_value = 4; + // Represents a structured value. Struct struct_value = 5; + // Represents a repeated `Value`. ListValue list_value = 6; } } +// `ListValue` is a wrapper around a repeated field of values. +message ListValue { + // Repeated field of dynamically typed values. + repeated Value values = 1; +} + // `NullValue` is a singleton enumeration to represent the null // value for the `Value` type union. enum NullValue { // Null value. NULL_VALUE = 0; } - -// `ListValue` is a wrapper around a repeated field of values. -message ListValue { - // Repeated field of dynamically typed values. - repeated Value values = 1; -} diff --git a/src/google/protobuf/timestamp.proto b/src/google/protobuf/timestamp.proto index ac8e009cfc..899562292a 100644 --- a/src/google/protobuf/timestamp.proto +++ b/src/google/protobuf/timestamp.proto @@ -46,15 +46,16 @@ option java_package = "com.google.protobuf"; // table is needed for interpretation. Range is from // 0001-01-01T00:00:00Z to 9999-12-31T23:59:59.999999999Z. // By restricting to that range, we ensure that we can convert to -// and from RFC 3339 date strings. (See https://www.ietf.org/rfc/rfc3339.txt.) +// and from RFC 3339 date strings. +// See [https://www.ietf.org/rfc/rfc3339.txt](https://www.ietf.org/rfc/rfc3339.txt). // -// Example 1: compute Timestamp from POSIX `time()`. +// Example 1: Compute Timestamp from POSIX `time()`. // // Timestamp timestamp; // timestamp.set_seconds(time(NULL)); // timestamp.set_nanos(0); // -// Example 2: compute Timestamp from POSIX `gettimeofday()`. +// Example 2: Compute Timestamp from POSIX `gettimeofday()`. // // struct timeval tv; // gettimeofday(&tv, NULL); @@ -63,7 +64,7 @@ option java_package = "com.google.protobuf"; // timestamp.set_seconds(tv.tv_sec); // timestamp.set_nanos(tv.tv_usec * 1000); // -// Example 3: compute Timestamp from Win32 `GetSystemTimeAsFileTime()`. +// Example 3: Compute Timestamp from Win32 `GetSystemTimeAsFileTime()`. // // FILETIME ft; // GetSystemTimeAsFileTime(&ft); @@ -75,14 +76,14 @@ option java_package = "com.google.protobuf"; // timestamp.set_seconds((INT64) ((ticks / 10000000) - 11644473600LL)); // timestamp.set_nanos((INT32) ((ticks % 10000000) * 100)); // -// Example 4: compute Timestamp from Java `System.currentTimeMillis()`. +// Example 4: Compute Timestamp from Java `System.currentTimeMillis()`. // // long millis = System.currentTimeMillis(); // // Timestamp timestamp = Timestamp.newBuilder().setSeconds(millis / 1000) // .setNanos((int) ((millis % 1000) * 1000000)).build(); // -// Example 5: compute Timestamp from Python `datetime.datetime`. +// Example 5: Compute Timestamp from Python `datetime.datetime`. // // now = datetime.datetime.utcnow() // seconds = int(time.mktime(now.timetuple())) diff --git a/src/google/protobuf/type.proto b/src/google/protobuf/type.proto new file mode 100644 index 0000000000..04165c81d7 --- /dev/null +++ b/src/google/protobuf/type.proto @@ -0,0 +1,196 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto3"; + +package google.protobuf; + +import "google/protobuf/any.proto"; +import "google/protobuf/source_context.proto"; + +option java_multiple_files = true; +option java_outer_classname = "TypeProto"; +option java_package = "com.google.protobuf"; + + +// A light-weight descriptor for a proto message type. +message Type { + // The fully qualified message name. + string name = 1; + + // The list of fields. + repeated Field fields = 2; + + // The list of oneof definitions. + // The list of oneofs declared in this Type + repeated string oneofs = 3; + + // The proto options. + repeated Option options = 4; + + // The source context. + SourceContext source_context = 5; +} + +// Field represents a single field of a message type. +message Field { + // Kind represents a basic field type. + enum Kind { + // Field type unknown. + TYPE_UNKNOWN = 0; + + // Field type double. + TYPE_DOUBLE = 1; + + // Field type float. + TYPE_FLOAT = 2; + + // Field type int64. + TYPE_INT64 = 3; + + // Field type uint64. + TYPE_UINT64 = 4; + + // Field type int32. + TYPE_INT32 = 5; + + // Field type fixed64. + TYPE_FIXED64 = 6; + + // Field type fixed32. + TYPE_FIXED32 = 7; + + // Field type bool. + TYPE_BOOL = 8; + + // Field type string. + TYPE_STRING = 9; + + // Field type message. + TYPE_MESSAGE = 11; + + // Field type bytes. + TYPE_BYTES = 12; + + // Field type uint32. + TYPE_UINT32 = 13; + + // Field type enum. + TYPE_ENUM = 14; + + // Field type sfixed32. + TYPE_SFIXED32 = 15; + + // Field type sfixed64. + TYPE_SFIXED64 = 16; + + // Field type sint32. + TYPE_SINT32 = 17; + + // Field type sint64. + TYPE_SINT64 = 18; + } + + // Cardinality represents whether a field is optional, required, or + // repeated. + enum Cardinality { + // The field cardinality is unknown. Typically an error condition. + CARDINALITY_UNKNOWN = 0; + + // For optional fields. + CARDINALITY_OPTIONAL = 1; + + // For required fields. Not used for proto3. + CARDINALITY_REQUIRED = 2; + + // For repeated fields. + CARDINALITY_REPEATED = 3; + } + + // The field kind. + Kind kind = 1; + + // The field cardinality, i.e. optional/required/repeated. + Cardinality cardinality = 2; + + // The proto field number. + int32 number = 3; + + // The field name. + string name = 4; + + // The type URL (without the scheme) when the type is MESSAGE or ENUM, + // such as `type.googleapis.com/google.protobuf.Empty`. + string type_url = 6; + + // Index in Type.oneofs. Starts at 1. Zero means no oneof mapping. + int32 oneof_index = 7; + + // Whether to use alternative packed wire representation. + bool packed = 8; + + // The proto options. + repeated Option options = 9; +} + +// Enum type definition. +message Enum { + // Enum type name. + string name = 1; + + // Enum value definitions. + repeated EnumValue enumvalue = 2; + + // Proto options for the enum type. + repeated Option options = 3; + + // The source context. + SourceContext source_context = 4; +} + +// Enum value definition. +message EnumValue { + // Enum value name. + string name = 1; + + // Enum value number. + int32 number = 2; + + // Proto options for the enum value. + repeated Option options = 3; +} + +// Proto option attached to messages/fields/enums etc. +message Option { + // Proto option name. + string name = 1; + + // Proto option value. + Any value = 2; +} From 7ddbf870086578ad0fc648ed5865813007cfef06 Mon Sep 17 00:00:00 2001 From: Feng Xiao Date: Thu, 23 Apr 2015 20:23:58 -0700 Subject: [PATCH 07/40] Include well-known type protos in "make install". --- src/Makefile.am | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 7811b0ae49..1e4516fb76 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -24,7 +24,17 @@ AM_LDFLAGS = $(PTHREAD_CFLAGS) # If I say "dist_include_DATA", automake complains that $(includedir) is not # a "legitimate" directory for DATA. Screw you, automake. protodir = $(includedir) -nobase_dist_proto_DATA = google/protobuf/descriptor.proto \ +nobase_dist_proto_DATA = google/protobuf/descriptor.proto \ + google/protobuf/any.proto \ + google/protobuf/api.proto \ + google/protobuf/duration.proto \ + google/protobuf/empty.proto \ + google/protobuf/field_mask.proto \ + google/protobuf/source_context.proto \ + google/protobuf/struct.proto \ + google/protobuf/timestamp.proto \ + google/protobuf/type.proto \ + google/protobuf/wrappers.proto \ google/protobuf/compiler/plugin.proto # Not sure why these don't get cleaned automatically. From d6c9f644ac62ec83196a75a47655a33a87875628 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 24 Apr 2015 15:34:40 -0700 Subject: [PATCH 08/40] internal changes Change-Id: I66f216c70a19f44637090878d7e442d4d0f8991b --- src/google/protobuf/arena.cc | 7 + src/google/protobuf/arena.h | 5 + src/google/protobuf/descriptor.h | 4 + .../protobuf/preserve_unknown_enum_test.cc | 4 +- src/google/protobuf/repeated_field.h | 2 +- .../stubs/atomicops_internals_pnacl.h | 176 +++++++++++++++++- src/google/protobuf/stubs/common.h | 42 ++++- src/google/protobuf/stubs/hash.h | 11 +- src/google/protobuf/stubs/platform_macros.h | 6 + src/google/protobuf/stubs/type_traits.h | 4 + 10 files changed, 246 insertions(+), 15 deletions(-) diff --git a/src/google/protobuf/arena.cc b/src/google/protobuf/arena.cc index bda3741385..f7059d2601 100755 --- a/src/google/protobuf/arena.cc +++ b/src/google/protobuf/arena.cc @@ -29,6 +29,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #ifdef ADDRESS_SANITIZER #include @@ -43,6 +44,12 @@ Arena::ThreadCache& Arena::thread_cache() { static GOOGLE_THREAD_LOCAL ThreadCache thread_cache_ = { -1, NULL }; return thread_cache_; } +#elif defined(GOOGLE_PROTOBUF_OS_ANDROID) || defined(GOOGLE_PROTOBUF_OS_IPHONE) +Arena::ThreadCache& Arena::thread_cache() { + static internal::ThreadLocalStorage* thread_cache_ = + new internal::ThreadLocalStorage(); + return *thread_cache_->Get(); +} #else GOOGLE_THREAD_LOCAL Arena::ThreadCache Arena::thread_cache_ = { -1, NULL }; #endif diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index bb15e80c92..b48bef925a 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h @@ -373,6 +373,11 @@ class LIBPROTOBUF_EXPORT Arena { // Thread local variables cannot be exposed through DLL interface but we can // wrap them in static functions. static ThreadCache& thread_cache(); +#elif defined(GOOGLE_PROTOBUF_OS_ANDROID) || defined(GOOGLE_PROTOBUF_OS_IPHONE) + // Android ndk does not support __thread keyword so we use a custom thread + // local storage class we implemented. + // iOS also does not support the __thread keyword. + static ThreadCache& thread_cache(); #else static GOOGLE_THREAD_LOCAL ThreadCache thread_cache_; static ThreadCache& thread_cache() { return thread_cache_; } diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 0e264f5458..c0a48cea9c 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -59,6 +59,10 @@ #include #include +// TYPE_BOOL is defined in the MacOS's ConditionalMacros.h. +#ifdef TYPE_BOOL +#undef TYPE_BOOL +#endif // TYPE_BOOL namespace google { namespace protobuf { diff --git a/src/google/protobuf/preserve_unknown_enum_test.cc b/src/google/protobuf/preserve_unknown_enum_test.cc index 9f8703aec4..1673e8afae 100644 --- a/src/google/protobuf/preserve_unknown_enum_test.cc +++ b/src/google/protobuf/preserve_unknown_enum_test.cc @@ -246,8 +246,6 @@ TEST(PreserveUnknownEnumTest, Proto2CatchesUnknownValues) { protobuf_unittest::TestAllTypes message; // proto2 message const google::protobuf::Reflection* r = message.GetReflection(); const google::protobuf::Descriptor* d = message.GetDescriptor(); - const google::protobuf::FieldDescriptor* singular_field = - d->FindFieldByName("optional_nested_enum"); const google::protobuf::FieldDescriptor* repeated_field = d->FindFieldByName("repeated_nested_enum"); // Add one element to the repeated field so that we can test @@ -258,6 +256,8 @@ TEST(PreserveUnknownEnumTest, Proto2CatchesUnknownValues) { r->AddEnum(&message, repeated_field, enum_value); #ifdef PROTOBUF_HAS_DEATH_TEST + const google::protobuf::FieldDescriptor* singular_field = + d->FindFieldByName("optional_nested_enum"); // Enum-field integer-based setters GOOGLE_DCHECK-fail on invalid values, in order to // remain consistent with proto2 generated code. EXPECT_DEBUG_DEATH({ diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index f5f5d3f416..7bfdc40a5e 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -626,7 +626,7 @@ DEFINE_SPECIALIZATIONS_FOR_BASE_PROTO_TYPES(inline, MessageLite); DECLARE_SPECIALIZATIONS_FOR_BASE_PROTO_TYPES(Message); -#undef DECLARE_SPECIALIZATIONS_FOR_BASE_CLASSES +#undef DECLARE_SPECIALIZATIONS_FOR_BASE_PROTO_TYPES template <> inline const MessageLite& GenericTypeHandler::default_instance() { diff --git a/src/google/protobuf/stubs/atomicops_internals_pnacl.h b/src/google/protobuf/stubs/atomicops_internals_pnacl.h index b10ac02c40..3b314fd0c9 100644 --- a/src/google/protobuf/stubs/atomicops_internals_pnacl.h +++ b/src/google/protobuf/stubs/atomicops_internals_pnacl.h @@ -33,39 +33,197 @@ #ifndef GOOGLE_PROTOBUF_ATOMICOPS_INTERNALS_PNACL_H_ #define GOOGLE_PROTOBUF_ATOMICOPS_INTERNALS_PNACL_H_ +#include + namespace google { namespace protobuf { namespace internal { +// This implementation is transitional and maintains the original API for +// atomicops.h. This requires casting memory locations to the atomic types, and +// assumes that the API and the C++11 implementation are layout-compatible, +// which isn't true for all implementations or hardware platforms. The static +// assertion should detect this issue, were it to fire then this header +// shouldn't be used. +// +// TODO(jfb) If this header manages to stay committed then the API should be +// modified, and all call sites updated. +typedef volatile std::atomic* AtomicLocation32; +static_assert(sizeof(*(AtomicLocation32) nullptr) == sizeof(Atomic32), + "incompatible 32-bit atomic layout"); + +inline void MemoryBarrier() { +#if defined(__GLIBCXX__) + // Work around libstdc++ bug 51038 where atomic_thread_fence was declared but + // not defined, leading to the linker complaining about undefined references. + __atomic_thread_fence(std::memory_order_seq_cst); +#else + std::atomic_thread_fence(std::memory_order_seq_cst); +#endif +} + inline Atomic32 NoBarrier_CompareAndSwap(volatile Atomic32* ptr, Atomic32 old_value, Atomic32 new_value) { - return __sync_val_compare_and_swap(ptr, old_value, new_value); + ((AtomicLocation32)ptr) + ->compare_exchange_strong(old_value, + new_value, + std::memory_order_relaxed, + std::memory_order_relaxed); + return old_value; } -inline void MemoryBarrier() { - __sync_synchronize(); +inline Atomic32 NoBarrier_AtomicExchange(volatile Atomic32* ptr, + Atomic32 new_value) { + return ((AtomicLocation32)ptr) + ->exchange(new_value, std::memory_order_relaxed); +} + +inline Atomic32 NoBarrier_AtomicIncrement(volatile Atomic32* ptr, + Atomic32 increment) { + return increment + + ((AtomicLocation32)ptr) + ->fetch_add(increment, std::memory_order_relaxed); +} + +inline Atomic32 Barrier_AtomicIncrement(volatile Atomic32* ptr, + Atomic32 increment) { + return increment + ((AtomicLocation32)ptr)->fetch_add(increment); } inline Atomic32 Acquire_CompareAndSwap(volatile Atomic32* ptr, Atomic32 old_value, Atomic32 new_value) { - Atomic32 ret = NoBarrier_CompareAndSwap(ptr, old_value, new_value); + ((AtomicLocation32)ptr) + ->compare_exchange_strong(old_value, + new_value, + std::memory_order_acquire, + std::memory_order_acquire); + return old_value; +} + +inline Atomic32 Release_CompareAndSwap(volatile Atomic32* ptr, + Atomic32 old_value, + Atomic32 new_value) { + ((AtomicLocation32)ptr) + ->compare_exchange_strong(old_value, + new_value, + std::memory_order_release, + std::memory_order_relaxed); + return old_value; +} + +inline void NoBarrier_Store(volatile Atomic32* ptr, Atomic32 value) { + ((AtomicLocation32)ptr)->store(value, std::memory_order_relaxed); +} + +inline void Acquire_Store(volatile Atomic32* ptr, Atomic32 value) { + ((AtomicLocation32)ptr)->store(value, std::memory_order_relaxed); MemoryBarrier(); - return ret; } inline void Release_Store(volatile Atomic32* ptr, Atomic32 value) { - MemoryBarrier(); - *ptr = value; + ((AtomicLocation32)ptr)->store(value, std::memory_order_release); +} + +inline Atomic32 NoBarrier_Load(volatile const Atomic32* ptr) { + return ((AtomicLocation32)ptr)->load(std::memory_order_relaxed); } inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) { - Atomic32 value = *ptr; + return ((AtomicLocation32)ptr)->load(std::memory_order_acquire); +} + +inline Atomic32 Release_Load(volatile const Atomic32* ptr) { + MemoryBarrier(); + return ((AtomicLocation32)ptr)->load(std::memory_order_relaxed); +} + +#if defined(GOOGLE_PROTOBUF_ARCH_64_BIT) + +typedef volatile std::atomic* AtomicLocation64; +static_assert(sizeof(*(AtomicLocation64) nullptr) == sizeof(Atomic64), + "incompatible 64-bit atomic layout"); + +inline Atomic64 NoBarrier_CompareAndSwap(volatile Atomic64* ptr, + Atomic64 old_value, + Atomic64 new_value) { + ((AtomicLocation64)ptr) + ->compare_exchange_strong(old_value, + new_value, + std::memory_order_relaxed, + std::memory_order_relaxed); + return old_value; +} + +inline Atomic64 NoBarrier_AtomicExchange(volatile Atomic64* ptr, + Atomic64 new_value) { + return ((AtomicLocation64)ptr) + ->exchange(new_value, std::memory_order_relaxed); +} + +inline Atomic64 NoBarrier_AtomicIncrement(volatile Atomic64* ptr, + Atomic64 increment) { + return increment + + ((AtomicLocation64)ptr) + ->fetch_add(increment, std::memory_order_relaxed); +} + +inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr, + Atomic64 increment) { + return increment + ((AtomicLocation64)ptr)->fetch_add(increment); +} + +inline Atomic64 Acquire_CompareAndSwap(volatile Atomic64* ptr, + Atomic64 old_value, + Atomic64 new_value) { + ((AtomicLocation64)ptr) + ->compare_exchange_strong(old_value, + new_value, + std::memory_order_acquire, + std::memory_order_acquire); + return old_value; +} + +inline Atomic64 Release_CompareAndSwap(volatile Atomic64* ptr, + Atomic64 old_value, + Atomic64 new_value) { + ((AtomicLocation64)ptr) + ->compare_exchange_strong(old_value, + new_value, + std::memory_order_release, + std::memory_order_relaxed); + return old_value; +} + +inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) { + ((AtomicLocation64)ptr)->store(value, std::memory_order_relaxed); +} + +inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) { + ((AtomicLocation64)ptr)->store(value, std::memory_order_relaxed); MemoryBarrier(); - return value; } +inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) { + ((AtomicLocation64)ptr)->store(value, std::memory_order_release); +} + +inline Atomic64 NoBarrier_Load(volatile const Atomic64* ptr) { + return ((AtomicLocation64)ptr)->load(std::memory_order_relaxed); +} + +inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) { + return ((AtomicLocation64)ptr)->load(std::memory_order_acquire); +} + +inline Atomic64 Release_Load(volatile const Atomic64* ptr) { + MemoryBarrier(); + return ((AtomicLocation64)ptr)->load(std::memory_order_relaxed); +} + +#endif // defined(GOOGLE_PROTOBUF_ARCH_64_BIT) + } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/stubs/common.h b/src/google/protobuf/stubs/common.h index c3f735a225..37123c7b17 100644 --- a/src/google/protobuf/stubs/common.h +++ b/src/google/protobuf/stubs/common.h @@ -62,6 +62,14 @@ #include #endif +#if defined(__APPLE__) +#include // for TARGET_OS_IPHONE +#endif + +#if defined(__ANDROID__) || defined(GOOGLE_PROTOBUF_OS_ANDROID) || (defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE) || defined(GOOGLE_PROTOBUF_OS_IPHONE) +#include +#endif + #if defined(_WIN32) && defined(GetMessage) // Allow GetMessage to be used as a valid method name in protobuf classes. // windows.h defines GetMessage() as a macro. Let's re-define it as an inline @@ -157,7 +165,7 @@ std::string LIBPROTOBUF_EXPORT VersionString(int version); typedef unsigned int uint; #ifdef _MSC_VER -typedef __int8 int8; +typedef signed __int8 int8; typedef __int16 int16; typedef __int32 int32; typedef __int64 int64; @@ -1158,6 +1166,38 @@ class LIBPROTOBUF_EXPORT MutexLockMaybe { GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MutexLockMaybe); }; +#if defined(__ANDROID__) || defined(GOOGLE_PROTOBUF_OS_ANDROID) || (defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE) || defined(GOOGLE_PROTOBUF_OS_IPHONE) +// Android ndk does not support the __thread keyword very well yet. Here +// we use pthread_key_create()/pthread_getspecific()/... methods for +// TLS support on android. +// iOS also does not support the __thread keyword. +template +class ThreadLocalStorage { + public: + ThreadLocalStorage() { + pthread_key_create(&key_, &ThreadLocalStorage::Delete); + } + ~ThreadLocalStorage() { + pthread_key_delete(key_); + } + T* Get() { + T* result = static_cast(pthread_getspecific(key_)); + if (result == NULL) { + result = new T(); + pthread_setspecific(key_, result); + } + return result; + } + private: + static void Delete(void* value) { + delete static_cast(value); + } + pthread_key_t key_; + + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ThreadLocalStorage); +}; +#endif + } // namespace internal // We made these internal so that they would show up as such in the docs, diff --git a/src/google/protobuf/stubs/hash.h b/src/google/protobuf/stubs/hash.h index 82d5052eca..11bc2b3718 100755 --- a/src/google/protobuf/stubs/hash.h +++ b/src/google/protobuf/stubs/hash.h @@ -172,6 +172,13 @@ struct hash { } }; +template<> +struct hash { + size_t operator()(bool x) const { + return static_cast(x); + } +}; + template , typename EqualKey = std::equal_to, @@ -204,7 +211,7 @@ struct hash { static const size_t bucket_size = 4; static const size_t min_buckets = 8; - inline size_t operator()(const string& a, const string& b) const { + inline bool operator()(const string& a, const string& b) const { return a < b; } }; @@ -222,7 +229,7 @@ struct hash > { static const size_t bucket_size = 4; static const size_t min_buckets = 8; - inline size_t operator()(const pair& a, + inline bool operator()(const pair& a, const pair& b) const { return a < b; } diff --git a/src/google/protobuf/stubs/platform_macros.h b/src/google/protobuf/stubs/platform_macros.h index 1ff09b83c9..2ce7fc8f2a 100644 --- a/src/google/protobuf/stubs/platform_macros.h +++ b/src/google/protobuf/stubs/platform_macros.h @@ -95,12 +95,18 @@ GOOGLE_PROTOBUF_PLATFORM_ERROR #if defined(__APPLE__) #define GOOGLE_PROTOBUF_OS_APPLE +#include +#if TARGET_OS_IPHONE +#define GOOGLE_PROTOBUF_OS_IPHONE +#endif #elif defined(__native_client__) #define GOOGLE_PROTOBUF_OS_NACL #elif defined(sun) #define GOOGLE_PROTOBUF_OS_SOLARIS #elif defined(_AIX) #define GOOGLE_PROTOBUF_OS_AIX +#elif defined(__ANDROID__) +#define GOOGLE_PROTOBUF_OS_ANDROID #endif #undef GOOGLE_PROTOBUF_PLATFORM_ERROR diff --git a/src/google/protobuf/stubs/type_traits.h b/src/google/protobuf/stubs/type_traits.h index b58cae3f3e..36a8f3b1ff 100644 --- a/src/google/protobuf/stubs/type_traits.h +++ b/src/google/protobuf/stubs/type_traits.h @@ -73,6 +73,10 @@ struct is_base_of { typedef char (&yes)[1]; typedef char (&no)[2]; + // BEGIN GOOGLE LOCAL MODIFICATION -- check is a #define on Mac. + #undef check + // END GOOGLE LOCAL MODIFICATION + static yes check(const B*); static no check(const void*); From 9a121853236a8c77693f14d21d27987e5832176a Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Mon, 27 Apr 2015 14:47:04 -0700 Subject: [PATCH 09/40] Handle un-handled cases of TestOneof2 in unittest Change-Id: I8f082ee3117ed873ab472a539867ff7cae58e8fd --- src/google/protobuf/compiler/cpp/cpp_unittest.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/google/protobuf/compiler/cpp/cpp_unittest.cc b/src/google/protobuf/compiler/cpp/cpp_unittest.cc index cc758cf5ee..9d14a924f0 100644 --- a/src/google/protobuf/compiler/cpp/cpp_unittest.cc +++ b/src/google/protobuf/compiler/cpp/cpp_unittest.cc @@ -1392,6 +1392,12 @@ class OneofTest : public testing::Test { case unittest::TestOneof2::kFooString: EXPECT_TRUE(message.has_foo_string()); break; + case unittest::TestOneof2::kFooCord: + EXPECT_TRUE(message.has_foo_cord()); + break; + case unittest::TestOneof2::kFooStringPiece: + EXPECT_TRUE(message.has_foo_string_piece()); + break; case unittest::TestOneof2::kFooBytes: EXPECT_TRUE(message.has_foo_bytes()); break; @@ -1404,6 +1410,9 @@ class OneofTest : public testing::Test { case unittest::TestOneof2::kFoogroup: EXPECT_TRUE(message.has_foogroup()); break; + case unittest::TestOneof2::kFooLazyMessage: + EXPECT_TRUE(message.has_foo_lazy_message()); + break; case unittest::TestOneof2::FOO_NOT_SET: break; } From 46bd60b92f593a62e150c93cb8761148f6830b47 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Mon, 27 Apr 2015 17:44:45 -0700 Subject: [PATCH 10/40] Modify directory to use in test Change-Id: Iede1dc4e70427663e0d3c5304fa86b3aebf492b7 --- .../protobuf/compiler/command_line_interface_unittest.cc | 6 ++++++ src/google/protobuf/testing/googletest.cc | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index a2cbbdc614..e284c7915b 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -288,6 +288,7 @@ void CommandLineInterfaceTest::Run(const string& command) { if (!disallow_plugins_) { cli_.AllowPlugins("prefix-"); +#ifndef GOOGLE_THIRD_PARTY_PROTOBUF const char* possible_paths[] = { // When building with shared libraries, libtool hides the real executable // in .libs and puts a fake wrapper in the current directory. @@ -316,6 +317,11 @@ void CommandLineInterfaceTest::Run(const string& command) { } if (plugin_path.empty()) { +#else + string plugin_path = "third_party/protobuf/test_plugin"; + + if (access(plugin_path.c_str(), F_OK) != 0) { +#endif // GOOGLE_THIRD_PARTY_PROTOBUF GOOGLE_LOG(ERROR) << "Plugin executable not found. Plugin tests are likely to fail."; } else { diff --git a/src/google/protobuf/testing/googletest.cc b/src/google/protobuf/testing/googletest.cc index d72fa5c00d..5f6a199af1 100644 --- a/src/google/protobuf/testing/googletest.cc +++ b/src/google/protobuf/testing/googletest.cc @@ -65,6 +65,7 @@ namespace protobuf { #endif string TestSourceDir() { +#ifndef GOOGLE_THIRD_PARTY_PROTOBUF #ifdef _MSC_VER // Look for the "src" directory. string prefix = "."; @@ -88,6 +89,9 @@ string TestSourceDir() { return result; } #endif +#else + return "third_party/protobuf/src"; +#endif // GOOGLE_THIRD_PARTY_PROTOBUF } namespace { From 4b4770280dc2aead50708b783825cf4d24b6d407 Mon Sep 17 00:00:00 2001 From: Behzad Tabibian Date: Tue, 28 Apr 2015 11:09:05 +0200 Subject: [PATCH 11/40] adding globals() to exec call for python3 compatibility --- python/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/setup.py b/python/setup.py index c18818e21d..07cc56e7a7 100755 --- a/python/setup.py +++ b/python/setup.py @@ -52,7 +52,7 @@ def GetVersion(): """ with open(os.path.join('google', 'protobuf', '__init__.py')) as version_file: - exec(version_file.read()) + exec(version_file.read(),globals()) return __version__ From ec2f2445546b3e63e4102c3541d4330e97cef07e Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Fri, 3 Oct 2014 14:41:43 -0700 Subject: [PATCH 12/40] Fix bug with large extension field numbers. Previously, extensions with field numbers greater than 268435455 would result in a compile time error in generated code that looks something like this: Foo.java:3178: error: integer number too large: 3346754610 3346754610); This is because we were trying to represent the tag number (an unsigned int) using a java int constant, but java int constants are signed, and can't exceed Integer.MAX_VALUE. Fixed by declaring it as a long instead, and casting it down to an int in the implementation. This is safe, because the tag value always fits in 32 bis. Change-Id: If2017bacb4e20af667eaeaf9b65ddc2c30a7709f --- .../com/google/protobuf/nano/Extension.java | 22 ++++++++++++------- .../google/protobuf/nano/Extension.java.rej | 13 +++++++++++ .../nano/unittest_extension_nano.proto | 3 +++ .../compiler/javanano/javanano_extension.cc | 2 +- 4 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej diff --git a/javanano/src/main/java/com/google/protobuf/nano/Extension.java b/javanano/src/main/java/com/google/protobuf/nano/Extension.java index c29b030fb6..2d091f015b 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/Extension.java +++ b/javanano/src/main/java/com/google/protobuf/nano/Extension.java @@ -74,6 +74,11 @@ public class Extension, T> { public static final int TYPE_SINT32 = InternalNano.TYPE_SINT32; public static final int TYPE_SINT64 = InternalNano.TYPE_SINT64; + // Note: these create...() methods take a long for the tag parameter, + // because tags are represented as unsigned longs, and these values exist + // in generated code as long values. However, they can fit in 32-bits, so + // it's safe to cast them to int without loss of precision. + /** * Creates an {@code Extension} of the given message type and tag number. * Should be used by the generated code only. @@ -81,8 +86,8 @@ public class Extension, T> { * @param type {@link #TYPE_MESSAGE} or {@link #TYPE_GROUP} */ public static , T extends MessageNano> - Extension createMessageTyped(int type, Class clazz, int tag) { - return new Extension(type, clazz, tag, false); + Extension createMessageTyped(int type, Class clazz, long tag) { + return new Extension(type, clazz, (int) tag, false); } /** @@ -92,8 +97,8 @@ public class Extension, T> { * @param type {@link #TYPE_MESSAGE} or {@link #TYPE_GROUP} */ public static , T extends MessageNano> - Extension createRepeatedMessageTyped(int type, Class clazz, int tag) { - return new Extension(type, clazz, tag, true); + Extension createRepeatedMessageTyped(int type, Class clazz, long tag) { + return new Extension(type, clazz, (int) tag, true); } /** @@ -104,8 +109,8 @@ public class Extension, T> { * @param clazz the boxed Java type of this extension */ public static , T> - Extension createPrimitiveTyped(int type, Class clazz, int tag) { - return new PrimitiveExtension(type, clazz, tag, false, 0, 0); + Extension createPrimitiveTyped(int type, Class clazz, long tag) { + return new PrimitiveExtension(type, clazz, (int) tag, false, 0, 0); } /** @@ -117,8 +122,9 @@ public class Extension, T> { */ public static , T> Extension createRepeatedPrimitiveTyped( - int type, Class clazz, int tag, int nonPackedTag, int packedTag) { - return new PrimitiveExtension(type, clazz, tag, true, nonPackedTag, packedTag); + int type, Class clazz, long tag, long nonPackedTag, long packedTag) { + return new PrimitiveExtension(type, clazz, (int) tag, true, + (int) nonPackedTag, (int) packedTag); } /** diff --git a/javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej b/javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej new file mode 100644 index 0000000000..465495a70b --- /dev/null +++ b/javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej @@ -0,0 +1,13 @@ +diff a/javanano/src/main/java/com/google/protobuf/nano/Extension.java b/javanano/src/main/java/com/google/protobuf/nano/Extension.java (rejected hunks) +@@ -74,6 +74,11 @@ public class Extension, T> { + public static final int TYPE_SINT32 = 17; + public static final int TYPE_SINT64 = 18; + ++ // Note: these create...() methods take a long for the tag parameter, ++ // because tags are represented as unsigned longs, and these values exist ++ // in generated code as long values. However, they can fit in 32-bits, so ++ // it's safe to cast them to int without loss of precision. ++ + /** + * Creates an {@code Extension} of the given message type and tag number. + * Should be used by the generated code only. diff --git a/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto b/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto index 2a678a8075..d1c5766dca 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto +++ b/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto @@ -21,6 +21,9 @@ message AnotherMessage { message ContainerMessage { extend ExtendableMessage { optional bool another_thing = 100; + // The largest permitted field number, per + // https://developers.google.com/protocol-buffers/docs/proto#simple + optional bool large_field_number = 536870911; } } diff --git a/src/google/protobuf/compiler/javanano/javanano_extension.cc b/src/google/protobuf/compiler/javanano/javanano_extension.cc index 754ed550d3..0b9d1d8de7 100644 --- a/src/google/protobuf/compiler/javanano/javanano_extension.cc +++ b/src/google/protobuf/compiler/javanano/javanano_extension.cc @@ -140,7 +140,7 @@ void ExtensionGenerator::Generate(io::Printer* printer) const { " com.google.protobuf.nano.Extension.create$repeated$$ext_type$(\n" " com.google.protobuf.nano.Extension.$type$,\n" " $class$.class,\n" - " $tag_params$);\n"); + " $tag_params$L);\n"); } } // namespace javanano From 54511f701fae491ccffcfbe03760e8466741d6a6 Mon Sep 17 00:00:00 2001 From: Charles Munger Date: Sun, 28 Dec 2014 17:49:06 -0800 Subject: [PATCH 13/40] Optimize measurement and serialization of nano protos. Measuring the serialized size of nano protos is now a zero-alloc operation, and serializing a proto now allocates no memory (other than the output buffer) instead of O(total length of strings). Change-Id: Id5e2ac3bdc4ac56c0bf13d725472da3a00c9baec Signed-off-by: Charles Munger --- .../nano/CodedOutputByteBufferNano.java | 238 ++++++++++++++++-- .../com/google/protobuf/nano/NanoTest.java | 36 +++ 2 files changed, 251 insertions(+), 23 deletions(-) diff --git a/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java b/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java index 3590718308..d0bc07e430 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java @@ -31,6 +31,9 @@ package com.google.protobuf.nano; import java.io.IOException; +import java.nio.BufferOverflowException; +import java.nio.ByteBuffer; +import java.nio.ReadOnlyBufferException; /** * Encodes and writes protocol message fields. @@ -47,15 +50,17 @@ import java.io.IOException; * @author kneton@google.com Kenton Varda */ public final class CodedOutputByteBufferNano { - private final byte[] buffer; - private final int limit; - private int position; + /* max bytes per java UTF-16 char in UTF-8 */ + private static final int MAX_UTF8_EXPANSION = 3; + private final ByteBuffer buffer; private CodedOutputByteBufferNano(final byte[] buffer, final int offset, final int length) { + this(ByteBuffer.wrap(buffer, offset, length)); + } + + private CodedOutputByteBufferNano(final ByteBuffer buffer) { this.buffer = buffer; - position = offset; - limit = offset + length; } /** @@ -287,14 +292,204 @@ public final class CodedOutputByteBufferNano { /** Write a {@code string} field to the stream. */ public void writeStringNoTag(final String value) throws IOException { - // Unfortunately there does not appear to be any way to tell Java to encode - // UTF-8 directly into our buffer, so we have to let it create its own byte - // array and then copy. - final byte[] bytes = value.getBytes(InternalNano.UTF_8); - writeRawVarint32(bytes.length); - writeRawBytes(bytes); + // UTF-8 byte length of the string is at least its UTF-16 code unit length (value.length()), + // and at most 3 times of it. Optimize for the case where we know this length results in a + // constant varint length - saves measuring length of the string. + try { + final int minLengthVarIntSize = computeRawVarint32Size(value.length()); + final int maxLengthVarIntSize = computeRawVarint32Size(value.length() * MAX_UTF8_EXPANSION); + if (minLengthVarIntSize == maxLengthVarIntSize) { + int oldPosition = buffer.position(); + buffer.position(oldPosition + minLengthVarIntSize); + encode(value, buffer); + int newPosition = buffer.position(); + buffer.position(oldPosition); + writeRawVarint32(newPosition - oldPosition - minLengthVarIntSize); + buffer.position(newPosition); + } else { + writeRawVarint32(encodedLength(value)); + encode(value, buffer); + } + } catch (BufferOverflowException e) { + throw new OutOfSpaceException(buffer.position(), buffer.limit()); + } + } + + // These UTF-8 handling methods are copied from Guava's Utf8 class. + /** + * Returns the number of bytes in the UTF-8-encoded form of {@code sequence}. For a string, + * this method is equivalent to {@code string.getBytes(UTF_8).length}, but is more efficient in + * both time and space. + * + * @throws IllegalArgumentException if {@code sequence} contains ill-formed UTF-16 (unpaired + * surrogates) + */ + private static int encodedLength(CharSequence sequence) { + // Warning to maintainers: this implementation is highly optimized. + int utf16Length = sequence.length(); + int utf8Length = utf16Length; + int i = 0; + + // This loop optimizes for pure ASCII. + while (i < utf16Length && sequence.charAt(i) < 0x80) { + i++; + } + + // This loop optimizes for chars less than 0x800. + for (; i < utf16Length; i++) { + char c = sequence.charAt(i); + if (c < 0x800) { + utf8Length += ((0x7f - c) >>> 31); // branch free! + } else { + utf8Length += encodedLengthGeneral(sequence, i); + break; + } + } + + if (utf8Length < utf16Length) { + // Necessary and sufficient condition for overflow because of maximum 3x expansion + throw new IllegalArgumentException("UTF-8 length does not fit in int: " + + (utf8Length + (1L << 32))); + } + return utf8Length; + } + + private static int encodedLengthGeneral(CharSequence sequence, int start) { + int utf16Length = sequence.length(); + int utf8Length = 0; + for (int i = start; i < utf16Length; i++) { + char c = sequence.charAt(i); + if (c < 0x800) { + utf8Length += (0x7f - c) >>> 31; // branch free! + } else { + utf8Length += 2; + // jdk7+: if (Character.isSurrogate(c)) { + if (Character.MIN_SURROGATE <= c && c <= Character.MAX_SURROGATE) { + // Check that we have a well-formed surrogate pair. + int cp = Character.codePointAt(sequence, i); + if (cp < Character.MIN_SUPPLEMENTARY_CODE_POINT) { + throw new IllegalArgumentException("Unpaired surrogate at index " + i); + } + i++; + } + } + } + return utf8Length; } + /** + * Encodes {@code sequence} into UTF-8, in {@code byteBuffer}. For a string, this method is + * equivalent to {@code buffer.put(string.getBytes(UTF_8))}, but is more efficient in both time + * and space. Bytes are written starting at the current position. This method requires paired + * surrogates, and therefore does not support chunking. + * + *

To ensure sufficient space in the output buffer, either call {@link #encodedLength} to + * compute the exact amount needed, or leave room for {@code 3 * sequence.length()}, which is the + * largest possible number of bytes that any input can be encoded to. + * + * @throws IllegalArgumentException if {@code sequence} contains ill-formed UTF-16 (unpaired + * surrogates) + * @throws BufferOverflowException if {@code sequence} encoded in UTF-8 does not fit in + * {@code byteBuffer}'s remaining space. + * @throws ReadOnlyBufferException if {@code byteBuffer} is a read-only buffer. + */ + private static void encode(CharSequence sequence, ByteBuffer byteBuffer) { + if (byteBuffer.isReadOnly()) { + throw new ReadOnlyBufferException(); + } else if (byteBuffer.hasArray()) { + try { + int encoded = encode(sequence, + byteBuffer.array(), + byteBuffer.arrayOffset() + byteBuffer.position(), + byteBuffer.remaining()); + byteBuffer.position(encoded - byteBuffer.arrayOffset()); + } catch (ArrayIndexOutOfBoundsException e) { + BufferOverflowException boe = new BufferOverflowException(); + boe.initCause(e); + throw boe; + } + } else { + encodeDirect(sequence, byteBuffer); + } + } + + private static void encodeDirect(CharSequence sequence, ByteBuffer byteBuffer) { + int utf16Length = sequence.length(); + for (int i = 0; i < utf16Length; i++) { + final char c = sequence.charAt(i); + if (c < 0x80) { // ASCII + byteBuffer.put((byte) c); + } else if (c < 0x800) { // 11 bits, two UTF-8 bytes + byteBuffer.put((byte) ((0xF << 6) | (c >>> 6))); + byteBuffer.put((byte) (0x80 | (0x3F & c))); + } else if (c < Character.MIN_SURROGATE || Character.MAX_SURROGATE < c) { + // Maximium single-char code point is 0xFFFF, 16 bits, three UTF-8 bytes + byteBuffer.put((byte) ((0xF << 5) | (c >>> 12))); + byteBuffer.put((byte) (0x80 | (0x3F & (c >>> 6)))); + byteBuffer.put((byte) (0x80 | (0x3F & c))); + } else { + final char low; + if (i + 1 == sequence.length() + || !Character.isSurrogatePair(c, (low = sequence.charAt(++i)))) { + throw new IllegalArgumentException("Unpaired surrogate at index " + (i - 1)); + } + int codePoint = Character.toCodePoint(c, low); + byteBuffer.put((byte) ((0xF << 4) | (codePoint >>> 18))); + byteBuffer.put((byte) (0x80 | (0x3F & (codePoint >>> 12)))); + byteBuffer.put((byte) (0x80 | (0x3F & (codePoint >>> 6)))); + byteBuffer.put((byte) (0x80 | (0x3F & codePoint))); + } + } + } + + private static int encode(CharSequence sequence, byte[] bytes, int offset, int length) { + int utf16Length = sequence.length(); + int j = offset; + int i = 0; + int limit = offset + length; + // Designed to take advantage of + // https://wikis.oracle.com/display/HotSpotInternals/RangeCheckElimination + for (char c; i < utf16Length && i + j < limit && (c = sequence.charAt(i)) < 0x80; i++) { + bytes[j + i] = (byte) c; + } + if (i == utf16Length) { + return j + utf16Length; + } + j += i; + for (char c; i < utf16Length; i++) { + c = sequence.charAt(i); + if (c < 0x80 && j < limit) { + bytes[j++] = (byte) c; + } else if (c < 0x800 && j <= limit - 2) { // 11 bits, two UTF-8 bytes + bytes[j++] = (byte) ((0xF << 6) | (c >>> 6)); + bytes[j++] = (byte) (0x80 | (0x3F & c)); + } else if ((c < Character.MIN_SURROGATE || Character.MAX_SURROGATE < c) && j <= limit - 3) { + // Maximum single-char code point is 0xFFFF, 16 bits, three UTF-8 bytes + bytes[j++] = (byte) ((0xF << 5) | (c >>> 12)); + bytes[j++] = (byte) (0x80 | (0x3F & (c >>> 6))); + bytes[j++] = (byte) (0x80 | (0x3F & c)); + } else if (j <= limit - 4) { + // Minimum code point represented by a surrogate pair is 0x10000, 17 bits, four UTF-8 bytes + final char low; + if (i + 1 == sequence.length() + || !Character.isSurrogatePair(c, (low = sequence.charAt(++i)))) { + throw new IllegalArgumentException("Unpaired surrogate at index " + (i - 1)); + } + int codePoint = Character.toCodePoint(c, low); + bytes[j++] = (byte) ((0xF << 4) | (codePoint >>> 18)); + bytes[j++] = (byte) (0x80 | (0x3F & (codePoint >>> 12))); + bytes[j++] = (byte) (0x80 | (0x3F & (codePoint >>> 6))); + bytes[j++] = (byte) (0x80 | (0x3F & codePoint)); + } else { + throw new ArrayIndexOutOfBoundsException("Failed writing " + c + " at index " + j); + } + } + return j; + } + + // End guava UTF-8 methods + + /** Write a {@code group} field to the stream. */ public void writeGroupNoTag(final MessageNano value) throws IOException { value.writeTo(this); @@ -602,9 +797,8 @@ public final class CodedOutputByteBufferNano { * {@code string} field. */ public static int computeStringSizeNoTag(final String value) { - final byte[] bytes = value.getBytes(InternalNano.UTF_8); - return computeRawVarint32Size(bytes.length) + - bytes.length; + final int length = encodedLength(value); + return computeRawVarint32Size(length) + length; } /** @@ -687,7 +881,7 @@ public final class CodedOutputByteBufferNano { * Otherwise, throws {@code UnsupportedOperationException}. */ public int spaceLeft() { - return limit - position; + return buffer.remaining(); } /** @@ -720,12 +914,12 @@ public final class CodedOutputByteBufferNano { /** Write a single byte. */ public void writeRawByte(final byte value) throws IOException { - if (position == limit) { + if (!buffer.hasRemaining()) { // We're writing to a single buffer. - throw new OutOfSpaceException(position, limit); + throw new OutOfSpaceException(buffer.position(), buffer.limit()); } - buffer[position++] = value; + buffer.put(value); } /** Write a single byte, represented by an integer value. */ @@ -741,13 +935,11 @@ public final class CodedOutputByteBufferNano { /** Write part of an array of bytes. */ public void writeRawBytes(final byte[] value, int offset, int length) throws IOException { - if (limit - position >= length) { - // We have room in the current buffer. - System.arraycopy(value, offset, buffer, position, length); - position += length; + if (buffer.remaining() >= length) { + buffer.put(value, offset, length); } else { // We're writing to a single buffer. - throw new OutOfSpaceException(position, limit); + throw new OutOfSpaceException(buffer.position(), buffer.limit()); } } diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java index 7de84310c0..d60e94ff31 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java +++ b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java @@ -2300,6 +2300,42 @@ public class NanoTest extends TestCase { } } + public void testDifferentStringLengthsNano() throws Exception { + // Test string serialization roundtrip using strings of the following lengths, + // with ASCII and Unicode characters requiring different UTF-8 byte counts per + // char, hence causing the length delimiter varint to sometimes require more + // bytes for the Unicode strings than the ASCII string of the same length. + int[] lengths = new int[] { + 0, + 1, + (1 << 4) - 1, // 1 byte for ASCII and Unicode + (1 << 7) - 1, // 1 byte for ASCII, 2 bytes for Unicode + (1 << 11) - 1, // 2 bytes for ASCII and Unicode + (1 << 14) - 1, // 2 bytes for ASCII, 3 bytes for Unicode + (1 << 17) - 1, // 3 bytes for ASCII and Unicode + }; + for (int i : lengths) { + testEncodingOfString('q', i); // 1 byte per char + testEncodingOfString('\u07FF', i); // 2 bytes per char + testEncodingOfString('\u0981', i); // 3 bytes per char + } + } + + private void testEncodingOfString(char c, int length) throws InvalidProtocolBufferNanoException { + TestAllTypesNano testAllTypesNano = new TestAllTypesNano(); + final String fullString = fullString(c, length); + testAllTypesNano.optionalString = fullString; + final TestAllTypesNano resultNano = new TestAllTypesNano(); + MessageNano.mergeFrom(resultNano, MessageNano.toByteArray(testAllTypesNano)); + assertEquals(fullString, resultNano.optionalString); + } + + private String fullString(char c, int length) { + char[] result = new char[length]; + Arrays.fill(result, c); + return new String(result); + } + public void testNanoWithHasParseFrom() throws Exception { TestAllTypesNanoHas msg = null; // Test false on creation, after clear and upon empty parse. From 9ffaa50d55bf377f4693eae45483e26ca136f878 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Wed, 7 Jan 2015 18:02:01 -0800 Subject: [PATCH 14/40] Fix comments. Forgot to update these in https://android-review.googlesource.com/#/c/109809/ Change-Id: I53f838e2f134f53964161d9620d5ead00c4a3939 --- .../src/main/java/com/google/protobuf/nano/Extension.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/javanano/src/main/java/com/google/protobuf/nano/Extension.java b/javanano/src/main/java/com/google/protobuf/nano/Extension.java index 2d091f015b..1063476614 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/Extension.java +++ b/javanano/src/main/java/com/google/protobuf/nano/Extension.java @@ -75,7 +75,7 @@ public class Extension, T> { public static final int TYPE_SINT64 = InternalNano.TYPE_SINT64; // Note: these create...() methods take a long for the tag parameter, - // because tags are represented as unsigned longs, and these values exist + // because tags are represented as unsigned ints, and these values exist // in generated code as long values. However, they can fit in 32-bits, so // it's safe to cast them to int without loss of precision. @@ -142,7 +142,7 @@ public class Extension, T> { protected final Class clazz; /** - * Tag number of this extension. + * Tag number of this extension. The data should be viewed as an unsigned 32-bit value. */ public final int tag; From fb96026b8deb79aa023c9f5c460582e8fea8f331 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Fri, 9 Jan 2015 13:32:38 -0800 Subject: [PATCH 15/40] When no clear() is generated, still initialize fields. https://android-review.googlesource.com/#/c/67890/ removed field initialization from the ctor, making it just call clear() instead. When I added the generate_clear option back (as part of the reftypes compat mode) in https://android-review.googlesource.com/#/c/109530/, I forgot to ensure that what clear() used to do was inlined in the constructor. This change fixes NPEs that are happening for users of reftypes_compat_mode who rely on unset repeated fields being empty arrays rather than null. Change-Id: Idb58746c60f4a4054b7ebb5c3b0e76b16ff88184 --- javanano/pom.xml | 9 ++++++ .../com/google/protobuf/nano/NanoTest.java | 6 ++++ .../compiler/javanano/javanano_message.cc | 32 ++++++++++++------- .../compiler/javanano/javanano_message.h | 1 + 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/javanano/pom.xml b/javanano/pom.xml index 7a2be9df24..64ed4372a9 100644 --- a/javanano/pom.xml +++ b/javanano/pom.xml @@ -139,6 +139,15 @@ + + + + + target/generated-test-sources diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java index d60e94ff31..c81846e57c 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java +++ b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java @@ -36,6 +36,7 @@ import com.google.protobuf.nano.NanoAccessorsOuterClass.TestNanoAccessors; import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas; import com.google.protobuf.nano.NanoOuterClass.TestAllTypesNano; import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano; +import com.google.protobuf.nano.NanoReferenceTypesCompat; import com.google.protobuf.nano.UnittestSimpleNano.SimpleMessageNano; import com.google.protobuf.nano.UnittestSingleNano.SingleMessageNano; import com.google.protobuf.nano.testext.Extensions; @@ -4381,6 +4382,11 @@ public class NanoTest extends TestCase { assertMapSet(testMap.sfixed64ToSfixed64Field, int64Values, int64Values); } + public void testRepeatedFieldInitializedInReftypesCompatMode() { + NanoReferenceTypesCompat.TestAllTypesNano proto = new NanoReferenceTypesCompat.TestAllTypesNano(); + assertNotNull(proto.repeatedString); + } + private void assertRepeatedPackablesEqual( NanoRepeatedPackables.NonPacked nonPacked, NanoRepeatedPackables.Packed packed) { // Not using MessageNano.equals() -- that belongs to a separate test. diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index 707f6b844d..d5cbe9ce6b 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -288,14 +288,18 @@ void MessageGenerator::Generate(io::Printer* printer) { } printer->Print("}\n"); } else { + printer->Print( + "\n" + "public $classname$() {\n", + "classname", descriptor_->name()); if (params_.generate_clear()) { - printer->Print( - "\n" - "public $classname$() {\n" - " clear();\n" - "}\n", - "classname", descriptor_->name()); + printer->Print(" clear();\n"); + } else { + printer->Indent(); + GenerateFieldInitializers(printer); + printer->Outdent(); } + printer->Print("}\n"); } // Other methods in this class @@ -495,6 +499,15 @@ void MessageGenerator::GenerateClear(io::Printer* printer) { "classname", descriptor_->name()); printer->Indent(); + GenerateFieldInitializers(printer); + + printer->Outdent(); + printer->Print( + " return this;\n" + "}\n"); +} + +void MessageGenerator::GenerateFieldInitializers(io::Printer* printer) { // Clear bit fields. int totalInts = (field_generators_.total_bits() + 31) / 32; for (int i = 0; i < totalInts; i++) { @@ -520,12 +533,7 @@ void MessageGenerator::GenerateClear(io::Printer* printer) { if (params_.store_unknown_fields()) { printer->Print("unknownFieldData = null;\n"); } - - printer->Outdent(); - printer->Print( - " cachedSize = -1;\n" - " return this;\n" - "}\n"); + printer->Print("cachedSize = -1;\n"); } void MessageGenerator::GenerateEquals(io::Printer* printer) { diff --git a/src/google/protobuf/compiler/javanano/javanano_message.h b/src/google/protobuf/compiler/javanano/javanano_message.h index 6f25a3a041..8504aa83b4 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.h +++ b/src/google/protobuf/compiler/javanano/javanano_message.h @@ -77,6 +77,7 @@ class MessageGenerator { const FieldDescriptor* field); void GenerateClear(io::Printer* printer); + void GenerateFieldInitializers(io::Printer* printer); void GenerateEquals(io::Printer* printer); void GenerateHashCode(io::Printer* printer); From d099a88685bf4b2df1689eb4cc56e75065cb87b1 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Wed, 1 Oct 2014 13:33:40 -0700 Subject: [PATCH 16/40] Add clone() method support for nano. Upstreamed from Another Place (cr/57247854). Change-Id: I2aaf59544c0f5ae21a51891d8a5eeda1dc722c90 --- javanano/pom.xml | 6 +-- .../protobuf/nano/ExtendableMessageNano.java | 7 +++ .../com/google/protobuf/nano/FieldArray.java | 17 +++++- .../com/google/protobuf/nano/FieldData.java | 52 ++++++++++++++++++- .../google/protobuf/nano/InternalNano.java | 8 +++ .../com/google/protobuf/nano/MessageNano.java | 8 +++ .../protobuf/nano/UnknownFieldData.java | 4 ++ .../com/google/protobuf/nano/NanoTest.java | 20 +++++++ .../nano/unittest_extension_nano.proto | 1 + .../compiler/javanano/javanano_enum_field.cc | 8 +++ .../compiler/javanano/javanano_enum_field.h | 1 + .../compiler/javanano/javanano_field.h | 1 + .../compiler/javanano/javanano_generator.cc | 2 + .../compiler/javanano/javanano_message.cc | 44 ++++++++++++++-- .../compiler/javanano/javanano_message.h | 1 + .../javanano/javanano_message_field.cc | 29 +++++++++++ .../javanano/javanano_message_field.h | 3 ++ .../compiler/javanano/javanano_params.h | 11 +++- .../javanano/javanano_primitive_field.cc | 8 +++ .../javanano/javanano_primitive_field.h | 1 + 20 files changed, 222 insertions(+), 10 deletions(-) diff --git a/javanano/pom.xml b/javanano/pom.xml index 64ed4372a9..3d8cfb9f1b 100644 --- a/javanano/pom.xml +++ b/javanano/pom.xml @@ -97,19 +97,19 @@ - + - + - + diff --git a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java index b979390d68..aeacbbf3e1 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java @@ -184,4 +184,11 @@ public abstract class ExtendableMessageNano> return (unknownFieldData == null || unknownFieldData.isEmpty() ? 0 : unknownFieldData.hashCode()); } + + @Override + public M clone() throws CloneNotSupportedException { + M cloned = (M) super.clone(); + InternalNano.cloneUnknownFieldData(this, cloned); + return cloned; + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java index cdb66da200..c2044f6a54 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java +++ b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java @@ -37,7 +37,7 @@ package com.google.protobuf.nano; * * Based on {@link android.support.v4.util.SpareArrayCompat}. */ -class FieldArray { +class FieldArray implements Cloneable { private static final FieldData DELETED = new FieldData(); private boolean mGarbage = false; @@ -270,4 +270,19 @@ class FieldArray { } return true; } + + @Override + public final FieldArray clone() { + // Trigger GC so we compact and don't copy DELETED elements. + int size = size(); + FieldArray clone = new FieldArray(size); + System.arraycopy(mFieldNumbers, 0, clone.mFieldNumbers, 0, size); + for (int i = 0; i < size; i++) { + if (mData[i] != null) { + clone.mData[i] = mData[i].clone(); + } + } + clone.mSize = size; + return clone; + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/FieldData.java b/javanano/src/main/java/com/google/protobuf/nano/FieldData.java index 21ead88b90..ebebabc8e5 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/FieldData.java +++ b/javanano/src/main/java/com/google/protobuf/nano/FieldData.java @@ -39,7 +39,7 @@ import java.util.List; * Stores unknown fields. These might be extensions or fields that the generated API doesn't * know about yet. */ -class FieldData { +class FieldData implements Cloneable { private Extension cachedExtension; private Object value; /** The serialised values for this object. Will be cleared if getValue is called */ @@ -187,4 +187,54 @@ class FieldData { return result; } + @Override + public final FieldData clone() { + FieldData clone = new FieldData(); + try { + clone.cachedExtension = cachedExtension; + if (unknownFieldData == null) { + clone.unknownFieldData = null; + } else { + clone.unknownFieldData.addAll(unknownFieldData); + } + + // Whether we need to deep clone value depends on its type. Primitive reference types + // (e.g. Integer, Long etc.) are ok, since they're immutable. We need to clone arrays + // and messages. + if (value == null) { + // No cloning required. + } else if (value instanceof MessageNano) { + clone.value = ((MessageNano) value).clone(); + } else if (value instanceof byte[]) { + clone.value = ((byte[]) value).clone(); + } else if (value instanceof byte[][]) { + byte[][] valueArray = (byte[][]) value; + byte[][] cloneArray = new byte[valueArray.length][]; + clone.value = cloneArray; + for (int i = 0; i < valueArray.length; i++) { + cloneArray[i] = valueArray[i].clone(); + } + } else if (value instanceof boolean[]) { + clone.value = ((boolean[]) value).clone(); + } else if (value instanceof int[]) { + clone.value = ((int[]) value).clone(); + } else if (value instanceof long[]) { + clone.value = ((long[]) value).clone(); + } else if (value instanceof float[]) { + clone.value = ((float[]) value).clone(); + } else if (value instanceof double[]) { + clone.value = ((double[]) value).clone(); + } else if (value instanceof MessageNano[]) { + MessageNano[] valueArray = (MessageNano[]) value; + MessageNano[] cloneArray = new MessageNano[valueArray.length]; + clone.value = cloneArray; + for (int i = 0; i < valueArray.length; i++) { + cloneArray[i] = valueArray[i].clone(); + } + } + return clone; + } catch (CloneNotSupportedException e) { + throw new AssertionError(e); + } + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java b/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java index e7ba8d1262..f1263df580 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java @@ -536,4 +536,12 @@ public final class InternalNano { } return o.hashCode(); } + + // This avoids having to make FieldArray public. + public static void cloneUnknownFieldData(ExtendableMessageNano original, + ExtendableMessageNano cloned) { + if (original.unknownFieldData != null) { + cloned.unknownFieldData = (FieldArray) original.unknownFieldData.clone(); + } + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java b/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java index 81e5857154..2347502702 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java @@ -187,4 +187,12 @@ public abstract class MessageNano { public String toString() { return MessageNanoPrinter.print(this); } + + /** + * Provides support for cloning. This only works if you specify the generate_clone method. + */ + @Override + public MessageNano clone() throws CloneNotSupportedException { + return (MessageNano) super.clone(); + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java b/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java index a17fccf3a6..b1678d1b93 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java +++ b/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java @@ -42,6 +42,10 @@ import java.util.Arrays; final class UnknownFieldData { final int tag; + /** + * Important: this should be treated as immutable, even though it's possible + * to change the array values. + */ final byte[] bytes; UnknownFieldData(int tag, byte[] bytes) { diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java index c81846e57c..91cc385f4a 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java +++ b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java @@ -3023,6 +3023,10 @@ public class NanoTest extends TestCase { assertTrue(Arrays.equals(floats, message.getExtension(RepeatedExtensions.repeatedFloat))); assertTrue(Arrays.equals(doubles, message.getExtension(RepeatedExtensions.repeatedDouble))); assertTrue(Arrays.equals(enums, message.getExtension(RepeatedExtensions.repeatedEnum))); + + // Clone the message and ensure it's still equal. + Extensions.ExtendableMessage clone = message.clone(); + assertEquals(clone, message); } public void testNullExtensions() throws Exception { @@ -4406,6 +4410,22 @@ public class NanoTest extends TestCase { assertTrue(Arrays.equals(nonPacked.enums, packed.enums)); } + public void testClone() throws Exception { + // A simple message. + AnotherMessage anotherMessage = new AnotherMessage(); + anotherMessage.string = "Hello"; + anotherMessage.value = true; + anotherMessage.integers = new int[] { 1, 2, 3 }; + + AnotherMessage clone = anotherMessage.clone(); + assertEquals(clone, anotherMessage); + + // Verify it was a deep clone - changes to the clone shouldn't affect the + // original. + clone.integers[1] = 100; + assertFalse(clone.equals(anotherMessage)); + } + private void assertHasWireData(MessageNano message, boolean expected) { byte[] bytes = MessageNano.toByteArray(message); int wireLength = bytes.length; diff --git a/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto b/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto index d1c5766dca..ca56b3dd45 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto +++ b/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto @@ -16,6 +16,7 @@ enum AnEnum { message AnotherMessage { optional string string = 1; optional bool value = 2; + repeated int32 integers = 3; } message ContainerMessage { diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc index 8a59d3233c..99b316bf82 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc @@ -498,6 +498,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void RepeatedEnumFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " cloned.$name$ = this.$name$.clone();\n" + "}\n"); +} + void RepeatedEnumFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.h b/src/google/protobuf/compiler/javanano/javanano_enum_field.h index 00adc61f0e..b94790d6bd 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.h @@ -106,6 +106,7 @@ class RepeatedEnumFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: void GenerateRepeatedDataSizeCode(io::Printer* printer) const; diff --git a/src/google/protobuf/compiler/javanano/javanano_field.h b/src/google/protobuf/compiler/javanano/javanano_field.h index c2cf091ca2..57c221f47c 100644 --- a/src/google/protobuf/compiler/javanano/javanano_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_field.h @@ -83,6 +83,7 @@ class FieldGenerator { virtual void GenerateSerializedSizeCode(io::Printer* printer) const = 0; virtual void GenerateEqualsCode(io::Printer* printer) const = 0; virtual void GenerateHashCodeCode(io::Printer* printer) const = 0; + virtual void GenerateFixClonedCode(io::Printer* printer) const {} protected: const Params& params_; diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index b5fbcd5fff..5ccac946fe 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -152,6 +152,8 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, params.set_ignore_services(option_value == "true"); } else if (option_name == "parcelable_messages") { params.set_parcelable_messages(option_value == "true"); + } else if (option_name == "generate_clone") { + params.set_generate_clone(option_value == "true"); } else { *error = "Ignore unknown javanano generator option: " + option_name; } diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index d5cbe9ce6b..b28ec082d7 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -136,18 +136,23 @@ void MessageGenerator::Generate(io::Printer* printer) { } if (params_.store_unknown_fields() && params_.parcelable_messages()) { printer->Print( - " com.google.protobuf.nano.android.ParcelableExtendableMessageNano<$classname$> {\n", + " com.google.protobuf.nano.android.ParcelableExtendableMessageNano<$classname$>", "classname", descriptor_->name()); } else if (params_.store_unknown_fields()) { printer->Print( - " com.google.protobuf.nano.ExtendableMessageNano<$classname$> {\n", + " com.google.protobuf.nano.ExtendableMessageNano<$classname$>", "classname", descriptor_->name()); } else if (params_.parcelable_messages()) { printer->Print( - " com.google.protobuf.nano.android.ParcelableMessageNano {\n"); + " com.google.protobuf.nano.android.ParcelableMessageNano"); } else { printer->Print( - " com.google.protobuf.nano.MessageNano {\n"); + " com.google.protobuf.nano.MessageNano"); + } + if (params_.generate_clone()) { + printer->Print(" implements java.lang.Cloneable {\n"); + } else { + printer->Print(" {\n"); } printer->Indent(); @@ -306,6 +311,10 @@ void MessageGenerator::Generate(io::Printer* printer) { GenerateClear(printer); + if (params_.generate_clone()) { + GenerateClone(printer); + } + if (params_.generate_equals()) { GenerateEquals(printer); GenerateHashCode(printer); @@ -536,6 +545,33 @@ void MessageGenerator::GenerateFieldInitializers(io::Printer* printer) { printer->Print("cachedSize = -1;\n"); } +void MessageGenerator::GenerateClone(io::Printer* printer) { + printer->Print( + "@Override\n" + "public $classname$ clone() {\n", + "classname", descriptor_->name()); + printer->Indent(); + + printer->Print( + "$classname$ cloned;\n" + "try {\n" + " cloned = ($classname$) super.clone();\n" + "} catch (java.lang.CloneNotSupportedException e) {\n" + " throw new java.lang.AssertionError(e);\n" + "}\n", + "classname", descriptor_->name()); + + for (int i = 0; i < descriptor_->field_count(); i++) { + field_generators_.get(descriptor_->field(i)).GenerateFixClonedCode(printer); + } + + printer->Outdent(); + printer->Print( + " return cloned;\n" + "}\n" + "\n"); +} + void MessageGenerator::GenerateEquals(io::Printer* printer) { // Don't override if there are no fields. We could generate an // equals method that compares types, but often empty messages diff --git a/src/google/protobuf/compiler/javanano/javanano_message.h b/src/google/protobuf/compiler/javanano/javanano_message.h index 8504aa83b4..281ec64fdd 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.h +++ b/src/google/protobuf/compiler/javanano/javanano_message.h @@ -80,6 +80,7 @@ class MessageGenerator { void GenerateFieldInitializers(io::Printer* printer); void GenerateEquals(io::Printer* printer); void GenerateHashCode(io::Printer* printer); + void GenerateClone(io::Printer* printer); const Params& params_; const Descriptor* descriptor_; diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc index 181c40605c..d1d04b52a4 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc @@ -126,6 +126,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void MessageFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null) {\n" + " cloned.$name$ = this.$name$.clone();\n" + "}\n"); +} + void MessageFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { printer->Print(variables_, @@ -212,6 +220,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void MessageOneofFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$oneof_name$ != null) {\n" + " cloned.$oneof_name$ = this.$oneof_name$.clone();\n" + "}\n"); +} + void MessageOneofFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { GenerateOneofFieldEquals(descriptor_, variables_, printer); @@ -312,6 +328,19 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void RepeatedMessageFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " cloned.$name$ = new $type$[this.$name$.length];\n" + " for (int i = 0; i < this.$name$.length; i++) {\n" + " if (this.$name$[i] != null) {\n" + " cloned.$name$[i] = this.$name$[i].clone();\n" + " }\n" + " }\n" + "}\n"); +} + void RepeatedMessageFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.h b/src/google/protobuf/compiler/javanano/javanano_message_field.h index 6c615f5e3f..e074735c70 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.h @@ -58,6 +58,7 @@ class MessageFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: const FieldDescriptor* descriptor_; @@ -80,6 +81,7 @@ class MessageOneofFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: const FieldDescriptor* descriptor_; @@ -102,6 +104,7 @@ class RepeatedMessageFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: const FieldDescriptor* descriptor_; diff --git a/src/google/protobuf/compiler/javanano/javanano_params.h b/src/google/protobuf/compiler/javanano/javanano_params.h index 4691f360bc..77bc717d68 100644 --- a/src/google/protobuf/compiler/javanano/javanano_params.h +++ b/src/google/protobuf/compiler/javanano/javanano_params.h @@ -66,6 +66,7 @@ class Params { bool parcelable_messages_; bool reftypes_primitive_enums_; bool generate_clear_; + bool generate_clone_; public: Params(const string & base_name) : @@ -81,7 +82,8 @@ class Params { ignore_services_(false), parcelable_messages_(false), reftypes_primitive_enums_(false), - generate_clear_(true) { + generate_clear_(true), + generate_clone_(false) { } const string& base_name() const { @@ -231,6 +233,13 @@ class Params { bool generate_clear() const { return generate_clear_; } + + void set_generate_clone(bool value) { + generate_clone_ = value; + } + bool generate_clone() const { + return generate_clone_; + } }; } // namespace javanano diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc index 41bad0a67e..978abf2ce4 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc @@ -364,6 +364,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { } } +void RepeatedPrimitiveFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " cloned.$name$ = this.$name$.clone();\n" + "}\n"); +} + void PrimitiveFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { // We define equality as serialized form equality. If generate_has(), diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h index ca7116ff7e..a01981dd48 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h @@ -131,6 +131,7 @@ class RepeatedPrimitiveFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: void GenerateRepeatedDataSizeCode(io::Printer* printer) const; From 714dba3ab8f1b608573a33bb3ed4051ceb955b5b Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Wed, 21 Jan 2015 13:02:59 -0800 Subject: [PATCH 17/40] Exclude cachedSize when printing fields. Change-Id: I85563b74237d38c1e447b7286f5f6e62d57e3d63 --- .../java/com/google/protobuf/nano/MessageNanoPrinter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/javanano/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java b/javanano/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java index c4b2ad3d0a..d9500bb948 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java +++ b/javanano/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java @@ -109,6 +109,10 @@ public final class MessageNanoPrinter { for (Field field : clazz.getFields()) { int modifiers = field.getModifiers(); String fieldName = field.getName(); + if ("cachedSize".equals(fieldName)) { + // TODO(bduff): perhaps cachedSize should have a more obscure name. + continue; + } if ((modifiers & Modifier.PUBLIC) == Modifier.PUBLIC && (modifiers & Modifier.STATIC) != Modifier.STATIC From 2712c421362994b4617e865ff977cc1c8f37f3d0 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Tue, 13 Jan 2015 11:58:16 -0800 Subject: [PATCH 18/40] DO NOT MERGE Overloading createMessageTyped to accept a tag as an integer. When building, some instances expect createMessageTyped to have the signature (int, Class, long), while others expect (int, Class, int). Simply having the former signature meant that builds expecting the latter would fail. This is a cherrypick of change b2a9d4321578139677c146ce37eba5e27e8f5c79 from master. Change-Id: Ib02dbf66173510f4edea32c7b43e82c1a7a38aa2 --- .../java/com/google/protobuf/nano/Extension.java | 13 +++++++++++++ .../com/google/protobuf/nano/Extension.java.rej | 13 ------------- 2 files changed, 13 insertions(+), 13 deletions(-) delete mode 100644 javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej diff --git a/javanano/src/main/java/com/google/protobuf/nano/Extension.java b/javanano/src/main/java/com/google/protobuf/nano/Extension.java index 1063476614..c458f9b1fc 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/Extension.java +++ b/javanano/src/main/java/com/google/protobuf/nano/Extension.java @@ -74,6 +74,19 @@ public class Extension, T> { public static final int TYPE_SINT32 = InternalNano.TYPE_SINT32; public static final int TYPE_SINT64 = InternalNano.TYPE_SINT64; + /** + * Creates an {@code Extension} of the given message type and tag number. + * Should be used by the generated code only. + * + * @param type {@link #TYPE_MESSAGE} or {@link #TYPE_GROUP} + * @deprecated use {@link #createMessageTyped(int, Class, long)} instead. + */ + @Deprecated + public static , T extends MessageNano> + Extension createMessageTyped(int type, Class clazz, int tag) { + return new Extension(type, clazz, tag, false); + } + // Note: these create...() methods take a long for the tag parameter, // because tags are represented as unsigned ints, and these values exist // in generated code as long values. However, they can fit in 32-bits, so diff --git a/javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej b/javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej deleted file mode 100644 index 465495a70b..0000000000 --- a/javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej +++ /dev/null @@ -1,13 +0,0 @@ -diff a/javanano/src/main/java/com/google/protobuf/nano/Extension.java b/javanano/src/main/java/com/google/protobuf/nano/Extension.java (rejected hunks) -@@ -74,6 +74,11 @@ public class Extension, T> { - public static final int TYPE_SINT32 = 17; - public static final int TYPE_SINT64 = 18; - -+ // Note: these create...() methods take a long for the tag parameter, -+ // because tags are represented as unsigned longs, and these values exist -+ // in generated code as long values. However, they can fit in 32-bits, so -+ // it's safe to cast them to int without loss of precision. -+ - /** - * Creates an {@code Extension} of the given message type and tag number. - * Should be used by the generated code only. From 41f0294cd6dc8aed2b3591d3ada9db8debd6a0d6 Mon Sep 17 00:00:00 2001 From: Shai Barack Date: Tue, 17 Feb 2015 09:44:48 -0800 Subject: [PATCH 19/40] Add reset() and position() to CodedOutputByteBufferNano. Change-Id: Ie2a9e36276ac35e10b3f8d379b5742d50a0374e9 --- .../nano/CodedOutputByteBufferNano.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java b/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java index d0bc07e430..91fa3353df 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java @@ -898,6 +898,23 @@ public final class CodedOutputByteBufferNano { } } + /** + * Returns the position within the internal buffer. + */ + public int position() { + return buffer.position(); + } + + /** + * Resets the position within the internal buffer to zero. + * + * @see #position + * @see #spaceLeft + */ + public void reset() { + buffer.clear(); + } + /** * If you create a CodedOutputStream around a simple flat array, you must * not attempt to write more bytes than the array has space. Otherwise, From ec19be2f3c7a95f3b7d6d0ff7055daead9284d8d Mon Sep 17 00:00:00 2001 From: Jeff Davidson Date: Wed, 11 Feb 2015 13:12:14 -0800 Subject: [PATCH 20/40] Generate @IntDef annotations for nanoproto enums. @IntDef is a support library annotation which allows build tools to determine the valid set of values for a given integer field when that field is intended to be restricted like an enum. This avoids the overhead of enums while still allowing for compile-time type checking in most circumstances. Change-Id: Iee02e0b49a8e069f6456572f538e0a0d301fdfd5 --- javanano/README.md | 23 +++++++++++ .../compiler/javanano/javanano_enum.cc | 40 +++++++++++++++++-- .../compiler/javanano/javanano_enum_field.cc | 24 +++++++++-- .../compiler/javanano/javanano_generator.cc | 2 + .../compiler/javanano/javanano_params.h | 11 ++++- 5 files changed, 91 insertions(+), 9 deletions(-) diff --git a/javanano/README.md b/javanano/README.md index 7d696aa7e8..e19b90b1c2 100644 --- a/javanano/README.md +++ b/javanano/README.md @@ -145,6 +145,7 @@ optional_field_style -> default or accessors enum_style -> c or java ignore_services -> true or false parcelable_messages -> true or false +generate_intdefs -> true or false ``` **java_package=\|\** (no default) @@ -302,6 +303,28 @@ parcelable_messages -> true or false Android-specific option to generate Parcelable messages. +**generate_intdefs={true,false}** (default: false) + Android-specific option to generate @IntDef annotations for enums. + + If turned on, an '@IntDef' annotation (a public @interface) will be + generated for each enum, and every integer parameter and return + value in the generated code meant for this enum will be annotated + with it. This interface is generated with the same name and at the + same place as the enum members' container interfaces described + above under 'enum_style=java', regardless of the enum_style option + used. When this is combined with enum_style=java, the interface + will be both the '@IntDef' annotation and the container of the enum + members; otherwise the interface has an empty body. + + Your app must declare a compile-time dependency on the + android-support-annotations library. + + For more information on how these @IntDef annotations help with + compile-time type safety, see: + https://sites.google.com/a/android.com/tools/tech-docs/support-annotations + and + https://developer.android.com/reference/android/support/annotation/IntDef.html + To use nano protobufs within the Android repo: ---------------------------------------------- diff --git a/src/google/protobuf/compiler/javanano/javanano_enum.cc b/src/google/protobuf/compiler/javanano/javanano_enum.cc index f934b05f0d..c6e8dfe90d 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum.cc @@ -73,13 +73,45 @@ void EnumGenerator::Generate(io::Printer* printer) { "// enum $classname$\n", "classname", descriptor_->name()); + const string classname = RenameJavaKeywords(descriptor_->name()); + // Start of container interface + // If generating intdefs, we use the container interface as the intdef if + // present. Otherwise, we just make an empty @interface parallel to the + // constants. + bool use_intdef = params_.generate_intdefs(); bool use_shell_class = params_.java_enum_style(); - if (use_shell_class) { - printer->Print( - "public interface $classname$ {\n", - "classname", RenameJavaKeywords(descriptor_->name())); + if (use_intdef) { + // @IntDef annotation so tools can enforce correctness + // Annotations will be discarded by the compiler + printer->Print("@java.lang.annotation.Retention(" + "java.lang.annotation.RetentionPolicy.SOURCE)\n" + "@android.support.annotation.IntDef({\n"); printer->Indent(); + for (int i = 0; i < canonical_values_.size(); i++) { + const string constant_name = + RenameJavaKeywords(canonical_values_[i]->name()); + if (use_shell_class) { + printer->Print("$classname$.$name$,\n", + "classname", classname, + "name", constant_name); + } else { + printer->Print("$name$,\n", "name", constant_name); + } + } + printer->Outdent(); + printer->Print("})\n"); + } + if (use_shell_class || use_intdef) { + printer->Print( + "public $at_for_intdef$interface $classname$ {\n", + "classname", classname, + "at_for_intdef", use_intdef ? "@" : ""); + if (use_shell_class) { + printer->Indent(); + } else { + printer->Print("}\n\n"); + } } // Canonical values diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc index 99b316bf82..7666db38a2 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc @@ -76,6 +76,10 @@ void SetEnumVariables(const Params& params, internal::WireFormatLite::MakeTag(descriptor->number(), internal::WireFormat::WireTypeForFieldType(descriptor->type()))); (*variables)["message_name"] = descriptor->containing_type()->name(); + const EnumDescriptor* enum_type = descriptor->enum_type(); + (*variables)["message_type_intdef"] = "@" + + ToJavaName(params, enum_type->name(), true, + enum_type->containing_type(), enum_type->file()); } void LoadEnumValues(const Params& params, @@ -116,8 +120,10 @@ EnumFieldGenerator::~EnumFieldGenerator() {} void EnumFieldGenerator:: GenerateMembers(io::Printer* printer, bool /* unused lazy_init */) const { - printer->Print(variables_, - "public $type$ $name$;\n"); + if (params_.generate_intdefs()) { + printer->Print(variables_, "$message_type_intdef$\n"); + } + printer->Print(variables_, "public $type$ $name$;\n"); if (params_.generate_has()) { printer->Print(variables_, @@ -256,12 +262,22 @@ AccessorEnumFieldGenerator::~AccessorEnumFieldGenerator() {} void AccessorEnumFieldGenerator:: GenerateMembers(io::Printer* printer, bool /* unused lazy_init */) const { + printer->Print(variables_, "private int $name$_;\n"); + if (params_.generate_intdefs()) { + printer->Print(variables_, "$message_type_intdef$\n"); + } printer->Print(variables_, - "private int $name$_;\n" "public int get$capitalized_name$() {\n" " return $name$_;\n" "}\n" - "public $message_name$ set$capitalized_name$(int value) {\n" + "public $message_name$ set$capitalized_name$("); + if (params_.generate_intdefs()) { + printer->Print(variables_, + "\n" + " $message_type_intdef$ "); + } + printer->Print(variables_, + "int value) {\n" " $name$_ = value;\n" " $set_has$;\n" " return this;\n" diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index 5ccac946fe..67b257485a 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -154,6 +154,8 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, params.set_parcelable_messages(option_value == "true"); } else if (option_name == "generate_clone") { params.set_generate_clone(option_value == "true"); + } else if (option_name == "generate_intdefs") { + params.set_generate_intdefs(option_value == "true"); } else { *error = "Ignore unknown javanano generator option: " + option_name; } diff --git a/src/google/protobuf/compiler/javanano/javanano_params.h b/src/google/protobuf/compiler/javanano/javanano_params.h index 77bc717d68..e3b4bb9368 100644 --- a/src/google/protobuf/compiler/javanano/javanano_params.h +++ b/src/google/protobuf/compiler/javanano/javanano_params.h @@ -67,6 +67,7 @@ class Params { bool reftypes_primitive_enums_; bool generate_clear_; bool generate_clone_; + bool generate_intdefs_; public: Params(const string & base_name) : @@ -83,7 +84,8 @@ class Params { parcelable_messages_(false), reftypes_primitive_enums_(false), generate_clear_(true), - generate_clone_(false) { + generate_clone_(false), + generate_intdefs_(false) { } const string& base_name() const { @@ -240,6 +242,13 @@ class Params { bool generate_clone() const { return generate_clone_; } + + void set_generate_intdefs(bool value) { + generate_intdefs_ = value; + } + bool generate_intdefs() const { + return generate_intdefs_; + } }; } // namespace javanano From dac7e02d2b9942953481bbe88241d4bf914ef30c Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Sat, 21 Feb 2015 15:22:43 -0800 Subject: [PATCH 21/40] Expose generate_clear as an option. I wasn't able to get the clear() method to inline into the constructor when optimizations are on in proguard. As a result, every message has an extra superfluous kept method assuming the app never uses clear() directly. There are a couple of instances where setting this option false is necessary in order to get code dexing successfully without hitting the method limit, e.g. https://goto.google.com/tltzq In this example, I tried turning on the method/inlining/unique and method/inlining/short optimizations before resorting to adding the generate_clear option, but the method count did not decrease. The clear() methods were contributing over a thousand extra methods. Change-Id: If6a9651d6a59cdf70b1040d8248779710ac73105 --- src/google/protobuf/compiler/javanano/javanano_generator.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index 67b257485a..ad215cb79e 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -156,6 +156,8 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, params.set_generate_clone(option_value == "true"); } else if (option_name == "generate_intdefs") { params.set_generate_intdefs(option_value == "true"); + } else if (option_name == "generate_clear") { + params.set_generate_clear(option_value == "true"); } else { *error = "Ignore unknown javanano generator option: " + option_name; } From a69b461e1eee43a277839825f1153b8260a28e87 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Fri, 20 Mar 2015 11:53:33 -0700 Subject: [PATCH 22/40] Inline unknownFieldData{Equals,HashCode} to generated code. It turns out dex (apparently) was inlining these protected final methods from ExtendableMessageNano into every message class. Removing these methods from the base class and inlining their code reduces the method count by 2 methods / message when the store_unknown_fields option is on. Change-Id: I0aa09f2016d39939c4c8b8219601793b8fab301f --- .../protobuf/nano/ExtendableMessageNano.java | 25 ------------------- .../com/google/protobuf/nano/FieldArray.java | 19 ++++++++------ .../compiler/javanano/javanano_message.cc | 10 ++++++-- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java index aeacbbf3e1..87973d76f0 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java @@ -160,31 +160,6 @@ public abstract class ExtendableMessageNano> return true; } - /** - * Returns whether the stored unknown field data in this message is equivalent to that in the - * other message. - * - * @param other the other message. - * @return whether the two sets of unknown field data are equal. - */ - protected final boolean unknownFieldDataEquals(M other) { - if (unknownFieldData == null || unknownFieldData.isEmpty()) { - return other.unknownFieldData == null || other.unknownFieldData.isEmpty(); - } else { - return unknownFieldData.equals(other.unknownFieldData); - } - } - - /** - * Computes the hashcode representing the unknown field data stored in this message. - * - * @return the hashcode for the unknown field data. - */ - protected final int unknownFieldDataHashCode() { - return (unknownFieldData == null || unknownFieldData.isEmpty() - ? 0 : unknownFieldData.hashCode()); - } - @Override public M clone() throws CloneNotSupportedException { M cloned = (M) super.clone(); diff --git a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java index c2044f6a54..eca9c0d9c1 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java +++ b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java @@ -35,9 +35,12 @@ package com.google.protobuf.nano; * A custom version of {@link android.util.SparseArray} with the minimal API * for storing {@link FieldData} objects. * + *

This class is an internal implementation detail of nano and should not + * be called directly by clients. + * * Based on {@link android.support.v4.util.SpareArrayCompat}. */ -class FieldArray implements Cloneable { +public final class FieldArray implements Cloneable { private static final FieldData DELETED = new FieldData(); private boolean mGarbage = false; @@ -48,7 +51,7 @@ class FieldArray implements Cloneable { /** * Creates a new FieldArray containing no fields. */ - public FieldArray() { + FieldArray() { this(10); } @@ -57,7 +60,7 @@ class FieldArray implements Cloneable { * require any additional memory allocation to store the specified * number of mappings. */ - public FieldArray(int initialCapacity) { + FieldArray(int initialCapacity) { initialCapacity = idealIntArraySize(initialCapacity); mFieldNumbers = new int[initialCapacity]; mData = new FieldData[initialCapacity]; @@ -68,7 +71,7 @@ class FieldArray implements Cloneable { * Gets the FieldData mapped from the specified fieldNumber, or null * if no such mapping has been made. */ - public FieldData get(int fieldNumber) { + FieldData get(int fieldNumber) { int i = binarySearch(fieldNumber); if (i < 0 || mData[i] == DELETED) { @@ -81,7 +84,7 @@ class FieldArray implements Cloneable { /** * Removes the data from the specified fieldNumber, if there was any. */ - public void remove(int fieldNumber) { + void remove(int fieldNumber) { int i = binarySearch(fieldNumber); if (i >= 0 && mData[i] != DELETED) { @@ -118,7 +121,7 @@ class FieldArray implements Cloneable { * Adds a mapping from the specified fieldNumber to the specified data, * replacing the previous mapping if there was one. */ - public void put(int fieldNumber, FieldData data) { + void put(int fieldNumber, FieldData data) { int i = binarySearch(fieldNumber); if (i >= 0) { @@ -167,7 +170,7 @@ class FieldArray implements Cloneable { * Returns the number of key-value mappings that this FieldArray * currently stores. */ - public int size() { + int size() { if (mGarbage) { gc(); } @@ -184,7 +187,7 @@ class FieldArray implements Cloneable { * the value from the indexth key-value mapping that this * FieldArray stores. */ - public FieldData dataAt(int index) { + FieldData dataAt(int index) { if (mGarbage) { gc(); } diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index b28ec082d7..060d25f0b7 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -612,7 +612,11 @@ void MessageGenerator::GenerateEquals(io::Printer* printer) { if (params_.store_unknown_fields()) { printer->Print( - "return unknownFieldDataEquals(other);\n"); + "if (unknownFieldData == null || unknownFieldData.isEmpty()) {\n" + " return other.unknownFieldData == null || other.unknownFieldData.isEmpty();\n" + "} else {\n" + " return unknownFieldData.equals(other.unknownFieldData);\n" + "}"); } else { printer->Print( "return true;\n"); @@ -642,7 +646,9 @@ void MessageGenerator::GenerateHashCode(io::Printer* printer) { if (params_.store_unknown_fields()) { printer->Print( - "result = 31 * result + unknownFieldDataHashCode();\n"); + "result = 31 * result + \n" + " (unknownFieldData == null || unknownFieldData.isEmpty() ? 0 : \n" + " unknownFieldData.hashCode());\n"); } printer->Print("return result;\n"); From 6732dd7e58e3adaf217f20f227e6ff7e72cb6b22 Mon Sep 17 00:00:00 2001 From: Charles Munger Date: Tue, 21 Apr 2015 14:35:46 -0700 Subject: [PATCH 23/40] Throw OutOfSpaceException instead of IllegalArgumentException. When a MessageNano containing a String is serialized into a buffer that is too small to contain it, and the buffer's boundary happens to be where the string field's length delimiting varint is serialized, and the string's length and 3*length have the same length when encoded as a varint, an IllegalArgumentException is thrown rather than an OutOfSpaceException. Github issue: https://github.com/google/protobuf/issues/292 Change-Id: If478d68cf15bfd0662252d008e42b2bf1ff1c75e --- .../nano/CodedOutputByteBufferNano.java | 11 ++++++++++- .../com/google/protobuf/nano/NanoTest.java | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java b/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java index 91fa3353df..b1b0c53a1e 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/CodedOutputByteBufferNano.java @@ -300,6 +300,12 @@ public final class CodedOutputByteBufferNano { final int maxLengthVarIntSize = computeRawVarint32Size(value.length() * MAX_UTF8_EXPANSION); if (minLengthVarIntSize == maxLengthVarIntSize) { int oldPosition = buffer.position(); + // Buffer.position, when passed a position that is past its limit, throws + // IllegalArgumentException, and this class is documented to throw + // OutOfSpaceException instead. + if (buffer.remaining() < minLengthVarIntSize) { + throw new OutOfSpaceException(oldPosition + minLengthVarIntSize, buffer.limit()); + } buffer.position(oldPosition + minLengthVarIntSize); encode(value, buffer); int newPosition = buffer.position(); @@ -311,7 +317,10 @@ public final class CodedOutputByteBufferNano { encode(value, buffer); } } catch (BufferOverflowException e) { - throw new OutOfSpaceException(buffer.position(), buffer.limit()); + final OutOfSpaceException outOfSpaceException = new OutOfSpaceException(buffer.position(), + buffer.limit()); + outOfSpaceException.initCause(e); + throw outOfSpaceException; } } diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java index 91cc385f4a..3a75777ac6 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java +++ b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java @@ -31,6 +31,7 @@ package com.google.protobuf.nano; import com.google.protobuf.nano.MapTestProto.TestMap; +import com.google.protobuf.nano.CodedOutputByteBufferNano; import com.google.protobuf.nano.MapTestProto.TestMap.MessageValue; import com.google.protobuf.nano.NanoAccessorsOuterClass.TestNanoAccessors; import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas; @@ -2322,6 +2323,23 @@ public class NanoTest extends TestCase { } } + /** Regression test for https://github.com/google/protobuf/issues/292 */ + public void testCorrectExceptionThrowWhenEncodingStringsWithoutEnoughSpace() throws Exception { + String testCase = "Foooooooo"; + assertEquals(CodedOutputByteBufferNano.computeRawVarint32Size(testCase.length()), + CodedOutputByteBufferNano.computeRawVarint32Size(testCase.length() * 3)); + assertEquals(11, CodedOutputByteBufferNano.computeStringSize(1, testCase)); + // Tag is one byte, varint describing string length is 1 byte, string length is 9 bytes. + // An array of size 1 will cause a failure when trying to write the varint. + for (int i = 0; i < 11; i++) { + CodedOutputByteBufferNano bufferNano = CodedOutputByteBufferNano.newInstance(new byte[i]); + try { + bufferNano.writeString(1, testCase); + fail("Should have thrown an out of space exception"); + } catch (CodedOutputByteBufferNano.OutOfSpaceException expected) {} + } + } + private void testEncodingOfString(char c, int length) throws InvalidProtocolBufferNanoException { TestAllTypesNano testAllTypesNano = new TestAllTypesNano(); final String fullString = fullString(c, length); From 9d546c85bda48c59ba10e240afbf731bd0775bc4 Mon Sep 17 00:00:00 2001 From: Jeff Davidson Date: Thu, 2 Apr 2015 14:46:35 -0700 Subject: [PATCH 24/40] Generate a CREATOR for each Parcelable message. This is less ideal from a dex count perspective because it requires a new variable for each message, and because most apps have proguard rules that will ensure that CREATOR classes are retained. However, it is required to be able to use nano protos inside of AIDL files, as the autogenerated AIDL code fails to compile otherwise. This is a substantial benefit as it allows for backwards-compatible parameters and return types in AIDL methods along the lines of safeparcel. Bug: 19084705 Change-Id: I66a2c0424b96cf8ff6b631b186cc4f9407dfc1f4 --- .../protobuf/compiler/javanano/javanano_message.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index 060d25f0b7..a41da5ae33 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -156,6 +156,17 @@ void MessageGenerator::Generate(io::Printer* printer) { } printer->Indent(); + if (params_.parcelable_messages()) { + printer->Print( + "\n" + "// Used by Parcelable\n" + "@SuppressWarnings({\"unused\"})\n" + "public static final android.os.Parcelable.Creator<$classname$> CREATOR =\n" + " new com.google.protobuf.nano.android.ParcelableMessageNanoCreator<\n" + " $classname$>($classname$.class);\n", + "classname", descriptor_->name()); + } + // Nested types and extensions for (int i = 0; i < descriptor_->extension_count(); i++) { ExtensionGenerator(descriptor_->extension(i), params_).Generate(printer); From efa666cf0ad77827ac0b15a159e2c85e9991d77c Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Thu, 30 Apr 2015 10:29:47 -0700 Subject: [PATCH 25/40] Generate pbconfig.h in generate_descriptor_proto.sh --- generate_descriptor_proto.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/generate_descriptor_proto.sh b/generate_descriptor_proto.sh index b25a3c6ad3..2dad36abc1 100755 --- a/generate_descriptor_proto.sh +++ b/generate_descriptor_proto.sh @@ -27,6 +27,7 @@ __EOF__ fi cd src +make $@ google/protobuf/stubs/pbconfig.h CORE_PROTO_IS_CORRECT=0 while [ $CORE_PROTO_IS_CORRECT -ne 1 ] do From 761cfa08e6cc60039a0bf490d75078d6738724ae Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Fri, 1 May 2015 08:48:56 -0700 Subject: [PATCH 26/40] build cleanups * update docs to simplify build steps * Gemfile.lock seemed to have an older version specified * do not check in the pkg dir --- ruby/.gitignore | 1 + ruby/Gemfile.lock | 2 +- ruby/README.md | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ruby/.gitignore b/ruby/.gitignore index 80c978f2ec..93e2450257 100644 --- a/ruby/.gitignore +++ b/ruby/.gitignore @@ -4,3 +4,4 @@ tags lib/google/protobuf_java.jar protobuf-jruby.iml target/ +pkg/ diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index 89deb47d89..6f34927688 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - google-protobuf (3.0.0.alpha.2) + google-protobuf (3.0.0.alpha.3.1.pre) GEM remote: https://rubygems.org/ diff --git a/ruby/README.md b/ruby/README.md index d2fa76abd8..9ae3ac360d 100644 --- a/ruby/README.md +++ b/ruby/README.md @@ -76,7 +76,7 @@ Then install the required Ruby gems: Then build the Gem: $ rake gem - $ gem install pkg/protobuf-$VERSION.gem + $ gem install `ls pkg/google-protobuf-*.gem` To run the specs: From c70b6058eaae4fa5b1af577c548e6809a53dfd98 Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Fri, 1 May 2015 08:54:18 -0700 Subject: [PATCH 27/40] add size alias for length starting to make `RepeatedField` quack like an array additional changes: * make sure gemspec gets all ruby code files * add homepage in gem spec removes one of the warnings, and the gem spec authors are pushing everyone to include a homepage in the gem * remove excess whitespace in test suite to bring formatting inline with the rest of the file --- ruby/.gitignore | 1 + ruby/google-protobuf.gemspec | 3 +- ruby/lib/google/protobuf.rb | 2 ++ ruby/lib/google/protobuf/repeated_field.rb | 40 ++++++++++++++++++++++ ruby/tests/basic.rb | 14 ++++++-- 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 ruby/lib/google/protobuf/repeated_field.rb diff --git a/ruby/.gitignore b/ruby/.gitignore index 93e2450257..bd8745dd85 100644 --- a/ruby/.gitignore +++ b/ruby/.gitignore @@ -5,3 +5,4 @@ lib/google/protobuf_java.jar protobuf-jruby.iml target/ pkg/ +tmp/ diff --git a/ruby/google-protobuf.gemspec b/ruby/google-protobuf.gemspec index abbbde35ff..28cdebf500 100644 --- a/ruby/google-protobuf.gemspec +++ b/ruby/google-protobuf.gemspec @@ -4,10 +4,11 @@ Gem::Specification.new do |s| s.licenses = ["BSD"] s.summary = "Protocol Buffers" s.description = "Protocol Buffers are Google's data interchange format." + s.homepage = "https://developers.google.com/protocol-buffers" s.authors = ["Protobuf Authors"] s.email = "protobuf@googlegroups.com" s.require_paths = ["lib"] - s.files = ["lib/google/protobuf.rb"] + s.files = `git ls-files -z`.split("\x0").find_all{|f| f =~ /lib\/.+\.rb/} unless RUBY_PLATFORM == "java" s.files += `git ls-files "*.c" "*.h" extconf.rb Makefile`.split s.extensions= ["ext/google/protobuf_c/extconf.rb"] diff --git a/ruby/lib/google/protobuf.rb b/ruby/lib/google/protobuf.rb index 75869dd8fc..72797245f1 100644 --- a/ruby/lib/google/protobuf.rb +++ b/ruby/lib/google/protobuf.rb @@ -34,3 +34,5 @@ if RUBY_PLATFORM == "java" else require 'google/protobuf_c' end + +require 'google/protobuf/repeated_field' diff --git a/ruby/lib/google/protobuf/repeated_field.rb b/ruby/lib/google/protobuf/repeated_field.rb new file mode 100644 index 0000000000..5b934e56ed --- /dev/null +++ b/ruby/lib/google/protobuf/repeated_field.rb @@ -0,0 +1,40 @@ +# Protocol Buffers - Google's data interchange format +# Copyright 2008 Google Inc. All rights reserved. +# https://developers.google.com/protocol-buffers/ +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +# add syntatic sugar on top of the core library +module Google + module Protobuf + class RepeatedField + + alias_method :size, :length + + end + end +end diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 1c3fb62cdf..307374e3ae 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -377,6 +377,18 @@ module BasicTest end end + def test_rptfield_array_ducktyping + l = Google::Protobuf::RepeatedField.new(:int32) + length_methods = %w(count length size) + length_methods.each do |lm| + assert l.send(lm) == 0 + end + l.push 4 + length_methods.each do |lm| + assert l.send(lm) == 1 + end + end + def test_map_basic # allowed key types: # :int32, :int64, :uint32, :uint64, :bool, :string, :bytes. @@ -827,7 +839,6 @@ module BasicTest assert m['a.b'] == 4 end - def test_int_ranges m = TestMessage.new @@ -933,7 +944,6 @@ module BasicTest assert_raise RangeError do m.optional_uint64 = 1.5 end - end def test_stress_test From d55733c76ee1db702529f38f602548ffe48a4ab1 Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Fri, 1 May 2015 11:54:29 -0700 Subject: [PATCH 28/40] return nil if array index indicie is out of bounds ruby arrays don't throw an exception; they return nil. Lets do the same! this fix also includes the ability to use negative array indicies --- ruby/README.md | 5 ++- ruby/Rakefile | 3 ++ ruby/ext/google/protobuf_c/repeated_field.c | 20 ++++++--- ruby/pom.xml | 2 +- .../google/protobuf/jruby/RubyMessage.java | 14 +++--- .../protobuf/jruby/RubyRepeatedField.java | 24 ++++++++--- ruby/tests/basic.rb | 43 +++++++++++++++++-- 7 files changed, 85 insertions(+), 26 deletions(-) diff --git a/ruby/README.md b/ruby/README.md index 9ae3ac360d..16474322e4 100644 --- a/ruby/README.md +++ b/ruby/README.md @@ -63,7 +63,7 @@ To build this Ruby extension, you will need: To Build the JRuby extension, you will need: * Maven -* The latest version of the protobuf java library +* The latest version of the protobuf java library (see ../java/README.md) * Install JRuby via rbenv or RVM First switch to the desired platform with rbenv or RVM. @@ -75,7 +75,8 @@ Then install the required Ruby gems: Then build the Gem: - $ rake gem + $ rake + $ rake clobber_package gem $ gem install `ls pkg/google-protobuf-*.gem` To run the specs: diff --git a/ruby/Rakefile b/ruby/Rakefile index 7c1d8495d3..c25103d819 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -6,6 +6,9 @@ require "rake/testtask" spec = Gem::Specification.load("google-protobuf.gemspec") if RUBY_PLATFORM == "java" + if `which mvn` == '' + raise ArgumentError, "maven needs to be installed" + end task :clean do system("mvn clean") end diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c index 8cf2e29b58..5148ee87f5 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.c +++ b/ruby/ext/google/protobuf_c/repeated_field.c @@ -47,6 +47,15 @@ RepeatedField* ruby_to_RepeatedField(VALUE _self) { return self; } +static int index_position(VALUE _index, RepeatedField* repeated_field) { + int index = NUM2INT(_index); + if (index < 0 && repeated_field->size > 0) { + index = repeated_field->size + index; + } + return index; +} + + /* * call-seq: * RepeatedField.each(&block) @@ -74,8 +83,7 @@ VALUE RepeatedField_each(VALUE _self) { * call-seq: * RepeatedField.[](index) => value * - * Accesses the element at the given index. Throws an exception on out-of-bounds - * errors. + * Accesses the element at the given index. Returns nil on out-of-bounds */ VALUE RepeatedField_index(VALUE _self, VALUE _index) { RepeatedField* self = ruby_to_RepeatedField(_self); @@ -83,9 +91,9 @@ VALUE RepeatedField_index(VALUE _self, VALUE _index) { upb_fieldtype_t field_type = self->field_type; VALUE field_type_class = self->field_type_class; - int index = NUM2INT(_index); + int index = index_position(_index, self); if (index < 0 || index >= self->size) { - rb_raise(rb_eRangeError, "Index out of range"); + return Qnil; } void* memory = (void *) (((uint8_t *)self->elements) + index * element_size); @@ -105,9 +113,9 @@ VALUE RepeatedField_index_set(VALUE _self, VALUE _index, VALUE val) { VALUE field_type_class = self->field_type_class; int element_size = native_slot_size(field_type); - int index = NUM2INT(_index); + int index = index_position(_index, self); if (index < 0 || index >= (INT_MAX - 1)) { - rb_raise(rb_eRangeError, "Index out of range"); + return Qnil; } if (index >= self->size) { RepeatedField_reserve(self, index + 1); diff --git a/ruby/pom.xml b/ruby/pom.xml index 1630fe847d..01f0e16b6f 100644 --- a/ruby/pom.xml +++ b/ruby/pom.xml @@ -78,7 +78,7 @@ com.google.protobuf protobuf-java - 3.0.0-pre + 3.0.0-alpha-3-pre diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 04bc0b765d..20e825e2f8 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -246,16 +246,15 @@ public class RubyMessage extends RubyObject { public IRubyObject dup(ThreadContext context) { RubyMessage dup = (RubyMessage) metaClass.newInstance(context, Block.NULL_BLOCK); IRubyObject value; - for (Descriptors.FieldDescriptor fieldDescriptor : builder.getAllFields().keySet()) { + for (Descriptors.FieldDescriptor fieldDescriptor : this.descriptor.getFields()) { if (fieldDescriptor.isRepeated()) { - dup.repeatedFields.put(fieldDescriptor, getRepeatedField(context, fieldDescriptor)); - } else if (builder.hasField(fieldDescriptor)) { - dup.fields.put(fieldDescriptor, wrapField(context, fieldDescriptor, builder.getField(fieldDescriptor))); + dup.addRepeatedField(fieldDescriptor, this.getRepeatedField(context, fieldDescriptor)); + } else if (fields.containsKey(fieldDescriptor)) { + dup.fields.put(fieldDescriptor, fields.get(fieldDescriptor)); + } else if (this.builder.hasField(fieldDescriptor)) { + dup.fields.put(fieldDescriptor, wrapField(context, fieldDescriptor, this.builder.getField(fieldDescriptor))); } } - for (Descriptors.FieldDescriptor fieldDescriptor : fields.keySet()) { - dup.fields.put(fieldDescriptor, fields.get(fieldDescriptor)); - } for (Descriptors.FieldDescriptor fieldDescriptor : maps.keySet()) { dup.maps.put(fieldDescriptor, maps.get(fieldDescriptor)); } @@ -411,6 +410,7 @@ public class RubyMessage extends RubyObject { for (int i = 0; i < count; i++) { ret.push(context, wrapField(context, fieldDescriptor, this.builder.getRepeatedField(fieldDescriptor, i))); } + addRepeatedField(fieldDescriptor, ret); return ret; } diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java index 9788317a5f..84bf895626 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java @@ -108,8 +108,9 @@ public class RubyRepeatedField extends RubyObject { */ @JRubyMethod(name = "[]=") public IRubyObject indexSet(ThreadContext context, IRubyObject index, IRubyObject value) { + int arrIndex = normalizeArrayIndex(index); Utils.checkType(context, fieldType, value, (RubyModule) typeClass); - this.storage.set(RubyNumeric.num2int(index), value); + this.storage.set(arrIndex, value); return context.runtime.getNil(); } @@ -117,12 +118,15 @@ public class RubyRepeatedField extends RubyObject { * call-seq: * RepeatedField.[](index) => value * - * Accesses the element at the given index. Throws an exception on out-of-bounds - * errors. + * Accesses the element at the given index. Returns nil on out-of-bounds */ @JRubyMethod(name = "[]") public IRubyObject index(ThreadContext context, IRubyObject index) { - return this.storage.eltInternal(RubyNumeric.num2int(index)); + int arrIndex = normalizeArrayIndex(index); + if (arrIndex < 0 || arrIndex >= this.storage.size()) { + return context.runtime.getNil(); + } + return this.storage.eltInternal(arrIndex); } /* @@ -134,8 +138,7 @@ public class RubyRepeatedField extends RubyObject { @JRubyMethod(rest = true) public IRubyObject insert(ThreadContext context, IRubyObject[] args) { for (int i = 0; i < args.length; i++) { - Utils.checkType(context, fieldType, args[i], (RubyModule) typeClass); - this.storage.add(args[i]); + push(context, args[i]); } return context.runtime.getNil(); } @@ -385,6 +388,15 @@ public class RubyRepeatedField extends RubyObject { } } + private int normalizeArrayIndex(IRubyObject index) { + int arrIndex = RubyNumeric.num2int(index); + int arrSize = this.storage.size(); + if (arrIndex < 0 && arrSize > 0) { + arrIndex = arrSize + arrIndex; + } + return arrIndex; + } + private RubyArray storage; private Descriptors.FieldDescriptor.Type fieldType; private IRubyObject typeClass; diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 307374e3ae..141ce7c518 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -314,6 +314,17 @@ module BasicTest assert l4 == [0, 0, 0, 0, 0, 42, 100, 101, 102] end + def test_parent_rptfield + #make sure we set the RepeatedField and can add to it + m = TestMessage.new + assert m.repeated_string == [] + m.repeated_string << 'ok' + m.repeated_string.push('ok2') + assert m.repeated_string == ['ok', 'ok2'] + m.repeated_string += ['ok3'] + assert m.repeated_string == ['ok', 'ok2', 'ok3'] + end + def test_rptfield_msg l = Google::Protobuf::RepeatedField.new(:message, TestMessage) l.push TestMessage.new @@ -383,10 +394,31 @@ module BasicTest length_methods.each do |lm| assert l.send(lm) == 0 end + # out of bounds returns a nil + assert l[0] == nil + assert l[1] == nil + assert l[-1] == nil l.push 4 length_methods.each do |lm| - assert l.send(lm) == 1 + assert l.send(lm) == 1 + end + assert l[0] == 4 + assert l[1] == nil + assert l[-1] == 4 + assert l[-2] == nil + + l.push 2 + length_methods.each do |lm| + assert l.send(lm) == 2 end + assert l[0] == 4 + assert l[1] == 2 + assert l[2] == nil + assert l[-1] == 2 + assert l[-2] == 4 + assert l[-3] == nil + + #adding out of scope will backfill with empty objects end def test_map_basic @@ -724,9 +756,12 @@ module BasicTest m = TestMessage.new m.optional_string = "hello" m.optional_int32 = 42 - m.repeated_msg.push TestMessage2.new(:foo => 100) - m.repeated_msg.push TestMessage2.new(:foo => 200) - + tm1 = TestMessage2.new(:foo => 100) + tm2 = TestMessage2.new(:foo => 200) + m.repeated_msg.push tm1 + assert m.repeated_msg[-1] == tm1 + m.repeated_msg.push tm2 + assert m.repeated_msg[-1] == tm2 m2 = m.dup assert m == m2 m.optional_int32 += 1 From 64678265c5ae28998d031900c2de52419a8ed7e4 Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Sat, 2 May 2015 13:48:23 -0700 Subject: [PATCH 29/40] allow a message field to be unset --- ruby/ext/google/protobuf_c/storage.c | 4 +++- ruby/pom.xml | 2 +- .../com/google/protobuf/jruby/RubyMessage.java | 15 ++++++++++----- ruby/tests/basic.rb | 2 ++ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c index 5b1549d23a..2ad8bd74e9 100644 --- a/ruby/ext/google/protobuf_c/storage.c +++ b/ruby/ext/google/protobuf_c/storage.c @@ -155,7 +155,9 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class, break; } case UPB_TYPE_MESSAGE: { - if (CLASS_OF(value) != type_class) { + if (CLASS_OF(value) == CLASS_OF(Qnil)) { + value = Qnil; + } else if (CLASS_OF(value) != type_class) { rb_raise(rb_eTypeError, "Invalid type %s to assign to submessage field.", rb_class2name(CLASS_OF(value))); diff --git a/ruby/pom.xml b/ruby/pom.xml index 1630fe847d..01f0e16b6f 100644 --- a/ruby/pom.xml +++ b/ruby/pom.xml @@ -78,7 +78,7 @@ com.google.protobuf protobuf-java - 3.0.0-pre + 3.0.0-alpha-3-pre diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 04bc0b765d..9ddadfcdd3 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -659,14 +659,14 @@ public class RubyMessage extends RubyObject { } else { Descriptors.FieldDescriptor.Type fieldType = fieldDescriptor.getType(); IRubyObject typeClass = context.runtime.getObject(); + boolean addValue = true; if (fieldType == Descriptors.FieldDescriptor.Type.MESSAGE) { typeClass = ((RubyDescriptor) getDescriptorForField(context, fieldDescriptor)).msgclass(context); + if (value.isNil()){ + addValue = false; + } } else if (fieldType == Descriptors.FieldDescriptor.Type.ENUM) { typeClass = ((RubyEnumDescriptor) getDescriptorForField(context, fieldDescriptor)).enummodule(context); - } - Utils.checkType(context, fieldType, value, (RubyModule) typeClass); - // Convert integer enum to symbol - if (fieldType == Descriptors.FieldDescriptor.Type.ENUM) { Descriptors.EnumDescriptor enumDescriptor = fieldDescriptor.getEnumType(); if (Utils.isRubyNum(value)) { Descriptors.EnumValueDescriptor val = @@ -674,7 +674,12 @@ public class RubyMessage extends RubyObject { if (val.getIndex() != -1) value = context.runtime.newSymbol(val.getName()); } } - this.fields.put(fieldDescriptor, value); + if (addValue) { + Utils.checkType(context, fieldType, value, (RubyModule) typeClass); + this.fields.put(fieldDescriptor, value); + } else { + this.fields.remove(fieldDescriptor); + } } } return context.runtime.getNil(); diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 307374e3ae..0eb5cefc0d 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -154,6 +154,8 @@ module BasicTest assert m.optional_bytes == "world" m.optional_msg = TestMessage2.new(:foo => 42) assert m.optional_msg == TestMessage2.new(:foo => 42) + m.optional_msg = nil + assert m.optional_msg == nil end def test_ctor_args From 15bff99de644be9ebccb0c4cfbce9dceea273abe Mon Sep 17 00:00:00 2001 From: Andre Eisenbach Date: Mon, 4 May 2015 14:00:47 -0700 Subject: [PATCH 30/40] Handle 0-length byte buffers in micro and nano protobufs Change-Id: I845ee1ab1005d25c8d77a8c2ed801c0f7b7c847b --- .../java/com/google/protobuf/nano/CodedInputByteBufferNano.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/javanano/src/main/java/com/google/protobuf/nano/CodedInputByteBufferNano.java b/javanano/src/main/java/com/google/protobuf/nano/CodedInputByteBufferNano.java index 4b45c6d298..f399315511 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/CodedInputByteBufferNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/CodedInputByteBufferNano.java @@ -236,6 +236,8 @@ public final class CodedInputByteBufferNano { System.arraycopy(buffer, bufferPos, result, 0, size); bufferPos += size; return result; + } else if (size == 0) { + return WireFormatNano.EMPTY_BYTES; } else { // Slow path: Build a byte array first then copy it. return readRawBytes(size); From 30650d81d9baa446dbc8deb784ba53794cafda5b Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Fri, 1 May 2015 08:57:16 -0400 Subject: [PATCH 31/40] Alpha 1 drop of Google's Objective C plugin and runtime support for protobufs. --- .gitignore | 6 + Makefile.am | 124 +- objectivec/DevTools/check_version_stamps.sh | 56 + .../DevTools/generate_descriptors_proto.sh | 36 + objectivec/DevTools/pddm.py | 687 + objectivec/DevTools/pddm_tests.py | 515 + objectivec/GPBArray.h | 535 + objectivec/GPBArray.m | 2499 +++ objectivec/GPBArray_PackagePrivate.h | 130 + objectivec/GPBBootstrap.h | 84 + objectivec/GPBCodedInputStream.h | 81 + objectivec/GPBCodedInputStream.m | 801 + .../GPBCodedInputStream_PackagePrivate.h | 131 + objectivec/GPBCodedOutputStream.h | 340 + objectivec/GPBCodedOutputStream.m | 1229 ++ objectivec/GPBDescriptor.h | 143 + objectivec/GPBDescriptor.m | 888 ++ objectivec/GPBDescriptor_PackagePrivate.h | 293 + objectivec/GPBDictionary.h | 2233 +++ objectivec/GPBDictionary.m | 12627 ++++++++++++++++ objectivec/GPBDictionary_PackagePrivate.h | 577 + objectivec/GPBExtensionField.h | 51 + objectivec/GPBExtensionField.m | 525 + objectivec/GPBExtensionField_PackagePrivate.h | 51 + objectivec/GPBExtensionRegistry.h | 46 + objectivec/GPBExtensionRegistry.m | 98 + .../GPBExtensionRegistry_PackagePrivate.h | 40 + objectivec/GPBField.h | 56 + objectivec/GPBField.m | 328 + objectivec/GPBField_PackagePrivate.h | 49 + objectivec/GPBMessage.h | 151 + objectivec/GPBMessage.m | 4735 ++++++ objectivec/GPBMessage_PackagePrivate.h | 124 + objectivec/GPBProtocolBuffers.h | 45 + objectivec/GPBProtocolBuffers.m | 49 + .../GPBProtocolBuffers_RuntimeSupport.h | 41 + objectivec/GPBRootObject.h | 42 + objectivec/GPBRootObject.m | 177 + objectivec/GPBRootObject_PackagePrivate.h | 42 + objectivec/GPBTypes.h | 102 + objectivec/GPBUnknownFieldSet.h | 46 + objectivec/GPBUnknownFieldSet.m | 422 + .../GPBUnknownFieldSet_PackagePrivate.h | 61 + objectivec/GPBUtilities.h | 181 + objectivec/GPBUtilities.m | 1645 ++ objectivec/GPBUtilities_PackagePrivate.h | 426 + objectivec/GPBWellKnownTypes.h | 48 + objectivec/GPBWellKnownTypes.m | 117 + objectivec/GPBWireFormat.h | 68 + objectivec/GPBWireFormat.m | 78 + .../project.pbxproj | 919 ++ .../contents.xcworkspacedata | 7 + .../xcshareddata/WorkspaceSettings.xcsettings | 8 + .../xcschemes/PerformanceTests.xcscheme | 125 + .../xcschemes/ProtocolBuffers.xcscheme | 115 + .../project.pbxproj | 1093 ++ .../contents.xcworkspacedata | 7 + .../xcshareddata/WorkspaceSettings.xcsettings | 8 + ...FFE465CA-0E74-40E8-9F09-500B66B7DCB2.plist | 62 + .../Info.plist | 21 + .../xcschemes/PerformanceTests.xcscheme | 134 + .../xcschemes/ProtocolBuffers.xcscheme | 116 + objectivec/Tests/Filter1.txt | 40 + objectivec/Tests/Filter2.txt | 35 + objectivec/Tests/GPBARCUnittestProtos.m | 57 + objectivec/Tests/GPBArrayTests.m | 3365 ++++ objectivec/Tests/GPBCodedInputStreamTests.m | 290 + objectivec/Tests/GPBCodedOuputStreamTests.m | 321 + objectivec/Tests/GPBConcurrencyTests.m | 157 + objectivec/Tests/GPBDescriptorTests.m | 232 + objectivec/Tests/GPBDictionaryTests+Bool.m | 2421 +++ objectivec/Tests/GPBDictionaryTests+Int32.m | 3650 +++++ objectivec/Tests/GPBDictionaryTests+Int64.m | 3650 +++++ objectivec/Tests/GPBDictionaryTests+String.m | 3362 ++++ objectivec/Tests/GPBDictionaryTests+UInt32.m | 3650 +++++ objectivec/Tests/GPBDictionaryTests+UInt64.m | 3649 +++++ objectivec/Tests/GPBDictionaryTests.pddm | 1044 ++ objectivec/Tests/GPBFilteredMessageTests.m | 98 + objectivec/Tests/GPBMessageTests+Merge.m | 700 + objectivec/Tests/GPBMessageTests+Runtime.m | 1978 +++ .../Tests/GPBMessageTests+Serialization.m | 838 + objectivec/Tests/GPBMessageTests.m | 1728 +++ objectivec/Tests/GPBPerfTests.m | 306 + objectivec/Tests/GPBStringTests.m | 516 + objectivec/Tests/GPBSwiftTests.swift | 405 + objectivec/Tests/GPBTestUtilities.h | 87 + objectivec/Tests/GPBTestUtilities.m | 2350 +++ objectivec/Tests/GPBUnittestProtos.m | 56 + objectivec/Tests/GPBUnknownFieldSetTest.m | 255 + objectivec/Tests/GPBUtilitiesTests.m | 363 + objectivec/Tests/GPBWellKnownTypesTest.m | 102 + objectivec/Tests/GPBWireFormatTests.m | 246 + objectivec/Tests/UnitTests-Bridging-Header.h | 6 + objectivec/Tests/UnitTests-Info.plist | 20 + objectivec/Tests/golden_message | Bin 0 -> 493 bytes objectivec/Tests/golden_packed_fields_message | Bin 0 -> 493 bytes objectivec/Tests/iOSTestHarness/AppDelegate.m | 35 + .../AppIcon.appiconset/Contents.json | 116 + .../AppIcon.appiconset/iPad6.png | Bin 0 -> 8583 bytes .../AppIcon.appiconset/iPad6@2x.png | Bin 0 -> 17744 bytes .../AppIcon.appiconset/iPad7.png | Bin 0 -> 8969 bytes .../AppIcon.appiconset/iPad7@2x.png | Bin 0 -> 18788 bytes .../AppIcon.appiconset/iPhone6.png | Bin 0 -> 7021 bytes .../AppIcon.appiconset/iPhone6@2x.png | Bin 0 -> 13348 bytes .../AppIcon.appiconset/iPhone7@2x.png | Bin 0 -> 11128 bytes .../AppIcon.appiconset/iPhone7@3x.png | Bin 0 -> 21792 bytes .../LaunchImage.launchimage/Contents.json | 49 + objectivec/Tests/iOSTestHarness/Info.plist | 43 + .../Tests/iOSTestHarness/LaunchScreen.xib | 33 + .../iOSTestHarness/en.lproj/InfoPlist.strings | 2 + .../Tests/text_format_map_unittest_data.txt | 70 + .../Tests/text_format_unittest_data.txt | 116 + objectivec/Tests/unittest_cycle.proto | 58 + objectivec/Tests/unittest_filter.proto | 71 + objectivec/Tests/unittest_name_mangling.proto | 37 + objectivec/Tests/unittest_objc.proto | 389 + .../Tests/unittest_runtime_proto2.proto | 108 + .../Tests/unittest_runtime_proto3.proto | 101 + .../google/protobuf/Descriptor.pbobjc.h | 1046 ++ .../google/protobuf/Descriptor.pbobjc.m | 2205 +++ objectivec/google/protobuf/Duration.pbobjc.h | 83 + objectivec/google/protobuf/Duration.pbobjc.m | 85 + objectivec/google/protobuf/Timestamp.pbobjc.h | 94 + objectivec/google/protobuf/Timestamp.pbobjc.m | 85 + src/Makefile.am | 25 + src/google/protobuf/any.proto | 1 + src/google/protobuf/api.proto | 1 + src/google/protobuf/compiler/main.cc | 6 + .../compiler/objectivec/objectivec_enum.cc | 199 + .../compiler/objectivec/objectivec_enum.h | 73 + .../objectivec/objectivec_enum_field.cc | 129 + .../objectivec/objectivec_enum_field.h | 77 + .../objectivec/objectivec_extension.cc | 166 + .../objectivec/objectivec_extension.h | 73 + .../compiler/objectivec/objectivec_field.cc | 474 + .../compiler/objectivec/objectivec_field.h | 166 + .../compiler/objectivec/objectivec_file.cc | 392 + .../compiler/objectivec/objectivec_file.h | 94 + .../objectivec/objectivec_generator.cc | 95 + .../objectivec/objectivec_generator.h | 60 + .../compiler/objectivec/objectivec_helpers.cc | 1131 ++ .../compiler/objectivec/objectivec_helpers.h | 176 + .../objectivec/objectivec_helpers_unittest.cc | 242 + .../objectivec/objectivec_map_field.cc | 161 + .../objectivec/objectivec_map_field.h | 64 + .../compiler/objectivec/objectivec_message.cc | 642 + .../compiler/objectivec/objectivec_message.h | 103 + .../objectivec/objectivec_message_field.cc | 90 + .../objectivec/objectivec_message_field.h | 71 + .../compiler/objectivec/objectivec_oneof.cc | 139 + .../compiler/objectivec/objectivec_oneof.h | 77 + .../objectivec/objectivec_primitive_field.cc | 162 + .../objectivec/objectivec_primitive_field.h | 82 + src/google/protobuf/descriptor.pb.cc | 4 +- src/google/protobuf/descriptor.proto | 1 + src/google/protobuf/duration.proto | 1 + src/google/protobuf/empty.proto | 1 + src/google/protobuf/field_mask.proto | 1 + src/google/protobuf/source_context.proto | 1 + src/google/protobuf/struct.proto | 1 + src/google/protobuf/timestamp.proto | 1 + src/google/protobuf/type.proto | 1 + .../unittest_drop_unknown_fields.proto | 1 + .../unittest_preserve_unknown_enum.proto | 1 + src/google/protobuf/wrappers.proto | 1 + 165 files changed, 82460 insertions(+), 3 deletions(-) create mode 100755 objectivec/DevTools/check_version_stamps.sh create mode 100755 objectivec/DevTools/generate_descriptors_proto.sh create mode 100755 objectivec/DevTools/pddm.py create mode 100755 objectivec/DevTools/pddm_tests.py create mode 100644 objectivec/GPBArray.h create mode 100644 objectivec/GPBArray.m create mode 100644 objectivec/GPBArray_PackagePrivate.h create mode 100644 objectivec/GPBBootstrap.h create mode 100644 objectivec/GPBCodedInputStream.h create mode 100644 objectivec/GPBCodedInputStream.m create mode 100644 objectivec/GPBCodedInputStream_PackagePrivate.h create mode 100644 objectivec/GPBCodedOutputStream.h create mode 100644 objectivec/GPBCodedOutputStream.m create mode 100644 objectivec/GPBDescriptor.h create mode 100644 objectivec/GPBDescriptor.m create mode 100644 objectivec/GPBDescriptor_PackagePrivate.h create mode 100644 objectivec/GPBDictionary.h create mode 100644 objectivec/GPBDictionary.m create mode 100644 objectivec/GPBDictionary_PackagePrivate.h create mode 100644 objectivec/GPBExtensionField.h create mode 100644 objectivec/GPBExtensionField.m create mode 100644 objectivec/GPBExtensionField_PackagePrivate.h create mode 100644 objectivec/GPBExtensionRegistry.h create mode 100644 objectivec/GPBExtensionRegistry.m create mode 100644 objectivec/GPBExtensionRegistry_PackagePrivate.h create mode 100644 objectivec/GPBField.h create mode 100644 objectivec/GPBField.m create mode 100644 objectivec/GPBField_PackagePrivate.h create mode 100644 objectivec/GPBMessage.h create mode 100644 objectivec/GPBMessage.m create mode 100644 objectivec/GPBMessage_PackagePrivate.h create mode 100644 objectivec/GPBProtocolBuffers.h create mode 100644 objectivec/GPBProtocolBuffers.m create mode 100644 objectivec/GPBProtocolBuffers_RuntimeSupport.h create mode 100644 objectivec/GPBRootObject.h create mode 100644 objectivec/GPBRootObject.m create mode 100644 objectivec/GPBRootObject_PackagePrivate.h create mode 100644 objectivec/GPBTypes.h create mode 100644 objectivec/GPBUnknownFieldSet.h create mode 100644 objectivec/GPBUnknownFieldSet.m create mode 100644 objectivec/GPBUnknownFieldSet_PackagePrivate.h create mode 100644 objectivec/GPBUtilities.h create mode 100644 objectivec/GPBUtilities.m create mode 100644 objectivec/GPBUtilities_PackagePrivate.h create mode 100644 objectivec/GPBWellKnownTypes.h create mode 100644 objectivec/GPBWellKnownTypes.m create mode 100644 objectivec/GPBWireFormat.h create mode 100644 objectivec/GPBWireFormat.m create mode 100644 objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj create mode 100644 objectivec/ProtocolBuffers_OSX.xcodeproj/project.xcworkspace/contents.xcworkspacedata create mode 100644 objectivec/ProtocolBuffers_OSX.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings create mode 100644 objectivec/ProtocolBuffers_OSX.xcodeproj/xcshareddata/xcschemes/PerformanceTests.xcscheme create mode 100644 objectivec/ProtocolBuffers_OSX.xcodeproj/xcshareddata/xcschemes/ProtocolBuffers.xcscheme create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/project.pbxproj create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/project.xcworkspace/contents.xcworkspacedata create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcbaselines/8BBEA4A5147C727100C4ADB7.xcbaseline/FFE465CA-0E74-40E8-9F09-500B66B7DCB2.plist create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcbaselines/8BBEA4A5147C727100C4ADB7.xcbaseline/Info.plist create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcschemes/PerformanceTests.xcscheme create mode 100644 objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcschemes/ProtocolBuffers.xcscheme create mode 100644 objectivec/Tests/Filter1.txt create mode 100644 objectivec/Tests/Filter2.txt create mode 100644 objectivec/Tests/GPBARCUnittestProtos.m create mode 100644 objectivec/Tests/GPBArrayTests.m create mode 100644 objectivec/Tests/GPBCodedInputStreamTests.m create mode 100644 objectivec/Tests/GPBCodedOuputStreamTests.m create mode 100644 objectivec/Tests/GPBConcurrencyTests.m create mode 100644 objectivec/Tests/GPBDescriptorTests.m create mode 100644 objectivec/Tests/GPBDictionaryTests+Bool.m create mode 100644 objectivec/Tests/GPBDictionaryTests+Int32.m create mode 100644 objectivec/Tests/GPBDictionaryTests+Int64.m create mode 100644 objectivec/Tests/GPBDictionaryTests+String.m create mode 100644 objectivec/Tests/GPBDictionaryTests+UInt32.m create mode 100644 objectivec/Tests/GPBDictionaryTests+UInt64.m create mode 100644 objectivec/Tests/GPBDictionaryTests.pddm create mode 100644 objectivec/Tests/GPBFilteredMessageTests.m create mode 100644 objectivec/Tests/GPBMessageTests+Merge.m create mode 100644 objectivec/Tests/GPBMessageTests+Runtime.m create mode 100644 objectivec/Tests/GPBMessageTests+Serialization.m create mode 100644 objectivec/Tests/GPBMessageTests.m create mode 100644 objectivec/Tests/GPBPerfTests.m create mode 100644 objectivec/Tests/GPBStringTests.m create mode 100644 objectivec/Tests/GPBSwiftTests.swift create mode 100644 objectivec/Tests/GPBTestUtilities.h create mode 100644 objectivec/Tests/GPBTestUtilities.m create mode 100644 objectivec/Tests/GPBUnittestProtos.m create mode 100644 objectivec/Tests/GPBUnknownFieldSetTest.m create mode 100644 objectivec/Tests/GPBUtilitiesTests.m create mode 100644 objectivec/Tests/GPBWellKnownTypesTest.m create mode 100644 objectivec/Tests/GPBWireFormatTests.m create mode 100644 objectivec/Tests/UnitTests-Bridging-Header.h create mode 100644 objectivec/Tests/UnitTests-Info.plist create mode 100644 objectivec/Tests/golden_message create mode 100644 objectivec/Tests/golden_packed_fields_message create mode 100644 objectivec/Tests/iOSTestHarness/AppDelegate.m create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/Contents.json create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad6.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad6@2x.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad7.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad7@2x.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone6.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone6@2x.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone7@2x.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone7@3x.png create mode 100644 objectivec/Tests/iOSTestHarness/Images.xcassets/LaunchImage.launchimage/Contents.json create mode 100644 objectivec/Tests/iOSTestHarness/Info.plist create mode 100644 objectivec/Tests/iOSTestHarness/LaunchScreen.xib create mode 100644 objectivec/Tests/iOSTestHarness/en.lproj/InfoPlist.strings create mode 100644 objectivec/Tests/text_format_map_unittest_data.txt create mode 100644 objectivec/Tests/text_format_unittest_data.txt create mode 100644 objectivec/Tests/unittest_cycle.proto create mode 100644 objectivec/Tests/unittest_filter.proto create mode 100644 objectivec/Tests/unittest_name_mangling.proto create mode 100644 objectivec/Tests/unittest_objc.proto create mode 100644 objectivec/Tests/unittest_runtime_proto2.proto create mode 100644 objectivec/Tests/unittest_runtime_proto3.proto create mode 100644 objectivec/google/protobuf/Descriptor.pbobjc.h create mode 100644 objectivec/google/protobuf/Descriptor.pbobjc.m create mode 100644 objectivec/google/protobuf/Duration.pbobjc.h create mode 100644 objectivec/google/protobuf/Duration.pbobjc.m create mode 100644 objectivec/google/protobuf/Timestamp.pbobjc.h create mode 100644 objectivec/google/protobuf/Timestamp.pbobjc.m create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_enum.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_enum.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_enum_field.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_extension.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_extension.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_field.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_field.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_file.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_file.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_generator.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_generator.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_helpers.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_helpers.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_map_field.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_map_field.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_message.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_message.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_message_field.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_message_field.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_oneof.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_oneof.h create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_primitive_field.cc create mode 100644 src/google/protobuf/compiler/objectivec/objectivec_primitive_field.h diff --git a/.gitignore b/.gitignore index b8ba4892a4..9a5508def0 100644 --- a/.gitignore +++ b/.gitignore @@ -74,3 +74,9 @@ src/**/*.trs # JavaBuild output. java/target javanano/target + +# Directories created by opening the Objective C Xcode projects. +objectivec/ProtocolBuffers_OSX.xcodeproj/project.xcworkspace/xcuserdata/ +objectivec/ProtocolBuffers_OSX.xcodeproj/xcuserdata/ +objectivec/ProtocolBuffers_iOS.xcodeproj/project.xcworkspace/xcuserdata/ +objectivec/ProtocolBuffers_iOS.xcodeproj/xcuserdata/ diff --git a/Makefile.am b/Makefile.am index 4fd93f26b7..eecffa1f24 100644 --- a/Makefile.am +++ b/Makefile.am @@ -197,6 +197,128 @@ javanano_EXTRA_DIST= javanano/README.md \ javanano/pom.xml +objectivec_EXTRA_DIST= \ + objectivec/DevTools/generate_descriptors_proto.sh \ + objectivec/DevTools/pddm.py \ + objectivec/DevTools/pddm_tests.py \ + objectivec/google/protobuf/Descriptor.pbobjc.h \ + objectivec/google/protobuf/Descriptor.pbobjc.m \ + objectivec/google/protobuf/Duration.pbobjc.h \ + objectivec/google/protobuf/Duration.pbobjc.m \ + objectivec/google/protobuf/Timestamp.pbobjc.h \ + objectivec/google/protobuf/Timestamp.pbobjc.m \ + objectivec/GPBArray.h \ + objectivec/GPBArray.m \ + objectivec/GPBArray_PackagePrivate.h \ + objectivec/GPBBootstrap.h \ + objectivec/GPBCodedInputStream.h \ + objectivec/GPBCodedInputStream.m \ + objectivec/GPBCodedInputStream_PackagePrivate.h \ + objectivec/GPBCodedOutputStream.h \ + objectivec/GPBCodedOutputStream.m \ + objectivec/GPBDescriptor.h \ + objectivec/GPBDescriptor.m \ + objectivec/GPBDescriptor_PackagePrivate.h \ + objectivec/GPBDictionary.h \ + objectivec/GPBDictionary.m \ + objectivec/GPBDictionary_PackagePrivate.h \ + objectivec/GPBExtensionField.h \ + objectivec/GPBExtensionField.m \ + objectivec/GPBExtensionField_PackagePrivate.h \ + objectivec/GPBExtensionRegistry.h \ + objectivec/GPBExtensionRegistry.m \ + objectivec/GPBExtensionRegistry_PackagePrivate.h \ + objectivec/GPBField.h \ + objectivec/GPBField.m \ + objectivec/GPBField_PackagePrivate.h \ + objectivec/GPBMessage.h \ + objectivec/GPBMessage.m \ + objectivec/GPBMessage_PackagePrivate.h \ + objectivec/GPBProtocolBuffers.h \ + objectivec/GPBProtocolBuffers.m \ + objectivec/GPBProtocolBuffers_RuntimeSupport.h \ + objectivec/GPBRootObject.h \ + objectivec/GPBRootObject.m \ + objectivec/GPBRootObject_PackagePrivate.h \ + objectivec/GPBTypes.h \ + objectivec/GPBUnknownFieldSet.h \ + objectivec/GPBUnknownFieldSet.m \ + objectivec/GPBUnknownFieldSet_PackagePrivate.h \ + objectivec/GPBUtilities.h \ + objectivec/GPBUtilities.m \ + objectivec/GPBUtilities_PackagePrivate.h \ + objectivec/GPBWellKnownTypes.h \ + objectivec/GPBWellKnownTypes.m \ + objectivec/GPBWireFormat.h \ + objectivec/GPBWireFormat.m \ + objectivec/ProtocolBuffers_iOS.xcodeproj/project.pbxproj \ + objectivec/ProtocolBuffers_iOS.xcodeproj/project.xcworkspace/contents.xcworkspacedata \ + objectivec/ProtocolBuffers_iOS.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings \ + objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcbaselines/8BBEA4A5147C727100C4ADB7.xcbaseline/FFE465CA-0E74-40E8-9F09-500B66B7DCB2.plist \ + objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcbaselines/8BBEA4A5147C727100C4ADB7.xcbaseline/Info.plist \ + objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcschemes/PerformanceTests.xcscheme \ + objectivec/ProtocolBuffers_iOS.xcodeproj/xcshareddata/xcschemes/ProtocolBuffers.xcscheme \ + objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj \ + objectivec/ProtocolBuffers_OSX.xcodeproj/project.xcworkspace/contents.xcworkspacedata \ + objectivec/ProtocolBuffers_OSX.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings \ + objectivec/ProtocolBuffers_OSX.xcodeproj/xcshareddata/xcschemes/PerformanceTests.xcscheme \ + objectivec/ProtocolBuffers_OSX.xcodeproj/xcshareddata/xcschemes/ProtocolBuffers.xcscheme \ + objectivec/Tests/Filter1.txt \ + objectivec/Tests/Filter2.txt \ + objectivec/Tests/golden_message \ + objectivec/Tests/golden_packed_fields_message \ + objectivec/Tests/GPBARCUnittestProtos.m \ + objectivec/Tests/GPBArrayTests.m \ + objectivec/Tests/GPBCodedInputStreamTests.m \ + objectivec/Tests/GPBCodedOuputStreamTests.m \ + objectivec/Tests/GPBConcurrencyTests.m \ + objectivec/Tests/GPBDescriptorTests.m \ + objectivec/Tests/GPBDictionaryTests+Bool.m \ + objectivec/Tests/GPBDictionaryTests+Int32.m \ + objectivec/Tests/GPBDictionaryTests+Int64.m \ + objectivec/Tests/GPBDictionaryTests+String.m \ + objectivec/Tests/GPBDictionaryTests+UInt32.m \ + objectivec/Tests/GPBDictionaryTests+UInt64.m \ + objectivec/Tests/GPBDictionaryTests.pddm \ + objectivec/Tests/GPBFilteredMessageTests.m \ + objectivec/Tests/GPBMessageTests+Merge.m \ + objectivec/Tests/GPBMessageTests+Runtime.m \ + objectivec/Tests/GPBMessageTests+Serialization.m \ + objectivec/Tests/GPBMessageTests.m \ + objectivec/Tests/GPBPerfTests.m \ + objectivec/Tests/GPBStringTests.m \ + objectivec/Tests/GPBSwiftTests.swift \ + objectivec/Tests/GPBTestUtilities.h \ + objectivec/Tests/GPBTestUtilities.m \ + objectivec/Tests/GPBUnittestProtos.m \ + objectivec/Tests/GPBUnknownFieldSetTest.m \ + objectivec/Tests/GPBUtilitiesTests.m \ + objectivec/Tests/GPBWellKnownTypesTest.m \ + objectivec/Tests/GPBWireFormatTests.m \ + objectivec/Tests/iOSTestHarness/AppDelegate.m \ + objectivec/Tests/iOSTestHarness/en.lproj/InfoPlist.strings \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/Contents.json \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad6.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad6@2x.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad7.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPad7@2x.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone6.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone6@2x.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone7@2x.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/AppIcon.appiconset/iPhone7@3x.png \ + objectivec/Tests/iOSTestHarness/Images.xcassets/LaunchImage.launchimage/Contents.json \ + objectivec/Tests/iOSTestHarness/Info.plist \ + objectivec/Tests/iOSTestHarness/LaunchScreen.xib \ + objectivec/Tests/text_format_map_unittest_data.txt \ + objectivec/Tests/text_format_unittest_data.txt \ + objectivec/Tests/unittest_cycle.proto \ + objectivec/Tests/unittest_filter.proto \ + objectivec/Tests/unittest_name_mangling.proto \ + objectivec/Tests/unittest_objc.proto \ + objectivec/Tests/unittest_runtime_proto2.proto \ + objectivec/Tests/unittest_runtime_proto3.proto \ + objectivec/Tests/UnitTests-Bridging-Header.h \ + objectivec/Tests/UnitTests-Info.plist python_EXTRA_DIST= \ python/google/protobuf/internal/api_implementation.cc \ @@ -301,7 +423,7 @@ ruby_EXTRA_DIST= \ ruby/tests/generated_code.rb \ ruby/tests/generated_code_test.rb -all_EXTRA_DIST=$(java_EXTRA_DIST) $(javanano_EXTRA_DIST) $(python_EXTRA_DIST) $(ruby_EXTRA_DIST) +all_EXTRA_DIST=$(java_EXTRA_DIST) $(javanano_EXTRA_DIST) $(objectivec_EXTRA_DIST) $(python_EXTRA_DIST) $(ruby_EXTRA_DIST) EXTRA_DIST = $(@DIST_LANG@_EXTRA_DIST) \ autogen.sh \ diff --git a/objectivec/DevTools/check_version_stamps.sh b/objectivec/DevTools/check_version_stamps.sh new file mode 100755 index 0000000000..5de9ef146e --- /dev/null +++ b/objectivec/DevTools/check_version_stamps.sh @@ -0,0 +1,56 @@ +#!/bin/bash + +# This script checks that the runtime version number constant in the compiler +# source and in the runtime source is the same. +# +# A distro can be made of the protobuf sources with only a subset of the +# languages, so if the compiler depended on the Objective C runtime, those +# builds would break. At the same time, we don't want the runtime source +# depending on the compiler sources; so two copies of the constant are needed. + +set -eu + +readonly ScriptDir=$(dirname "$(echo $0 | sed -e "s,^\([^/]\),$(pwd)/\1,")") +readonly ProtoRootDir="${ScriptDir}/../.." + +die() { + echo "Error: $1" + exit 1 +} + +readonly ConstantName=GOOGLE_PROTOBUF_OBJC_GEN_VERSION + +# Collect version from plugin sources. + +readonly PluginSrc="${ProtoRootDir}/src/google/protobuf/compiler/objectivec/objectivec_file.cc" +readonly PluginVersion=$( \ + cat "${PluginSrc}" \ + | sed -n -e "s:const int32_t ${ConstantName} = \([0-9]*\);:\1:p" +) + +if [[ -z "${PluginVersion}" ]] ; then + die "Failed to fine ${ConstantName} in the plugin source (${PluginSrc})." +fi + +# Collect version from runtime sources. + +readonly RuntimeSrc="${ProtoRootDir}/objectivec/GPBBootstrap.h" +readonly RuntimeVersion=$( \ + cat "${RuntimeSrc}" \ + | sed -n -e "s:#define ${ConstantName} \([0-9]*\):\1:p" +) + +if [[ -z "${RuntimeVersion}" ]] ; then + die "Failed to fine ${ConstantName} in the runtime source (${RuntimeSrc})." +fi + +# Compare them. + +if [[ "${PluginVersion}" != "${RuntimeVersion}" ]] ; then + die "Versions don't match! + Plugin: ${PluginVersion} from ${PluginSrc} + Runtime: ${RuntimeVersion} from ${RuntimeSrc} +" +fi + +# Success diff --git a/objectivec/DevTools/generate_descriptors_proto.sh b/objectivec/DevTools/generate_descriptors_proto.sh new file mode 100755 index 0000000000..42502bfec5 --- /dev/null +++ b/objectivec/DevTools/generate_descriptors_proto.sh @@ -0,0 +1,36 @@ +#!/bin/bash + +# This script will generate the common descriptors needed by the Objective C +# runtime. + +# HINT: Flags passed to generate_descriptor_proto.sh will be passed directly +# to make when building protoc. This is particularly useful for passing +# -j4 to run 4 jobs simultaneously. + +set -eu + +readonly ScriptDir=$(dirname "$(echo $0 | sed -e "s,^\([^/]\),$(pwd)/\1,")") +readonly ProtoRootDir="${ScriptDir}/../.." +readonly ProtoC="${ProtoRootDir}/src/protoc" + +pushd "${ProtoRootDir}" > /dev/null + +# Compiler build fails if config.h hasn't been made yet (even if configure/etc. +# have been run, so get that made first). +make $@ config.h + +# Make sure the compiler is current. +cd src +make $@ protoc + +# These really should only be run when the inputs or compiler are newer than +# the outputs. + +# Needed by the runtime. +./protoc --objc_out="${ProtoRootDir}/objectivec" google/protobuf/descriptor.proto + +# Well known types that the library provides helpers for. +./protoc --objc_out="${ProtoRootDir}/objectivec" google/protobuf/timestamp.proto +./protoc --objc_out="${ProtoRootDir}/objectivec" google/protobuf/duration.proto + +popd > /dev/null diff --git a/objectivec/DevTools/pddm.py b/objectivec/DevTools/pddm.py new file mode 100755 index 0000000000..d1b53f5a75 --- /dev/null +++ b/objectivec/DevTools/pddm.py @@ -0,0 +1,687 @@ +#! /usr/bin/python +# +# Protocol Buffers - Google's data interchange format +# Copyright 2015 Google Inc. All rights reserved. +# https://developers.google.com/protocol-buffers/ +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""PDDM - Poor Developers' Debug-able Macros + +A simple markup that can be added in comments of source so they can then be +expanded out into code. Most of this could be done with CPP macros, but then +developers can't really step through them in the debugger, this way they are +expanded to the same code, but you can debug them. + +Any file can be processed, but the syntax is designed around a C based compiler. +Processed lines start with "//%". There are three types of sections you can +create: Text (left alone), Macro Definitions, and Macro Expansions. There is +no order required between definitions and expansions, all definitions are read +before any expansions are processed (thus, if desired, definitions can be put +at the end of the file to keep them out of the way of the code). + +Macro Definitions are started with "//%PDDM-DEFINE Name(args)" and all lines +afterwards that start with "//%" are included in the definition. Multiple +macros can be defined in one block by just using a another "//%PDDM-DEFINE" +line to start the next macro. Optionally, a macro can be ended with +"//%PDDM-DEFINE-END", this can be useful when you want to make it clear that +trailing blank lines are included in the macro. You can also end a definition +with an expansion. + +Macro Expansions are started by single lines containing +"//%PDDM-EXPAND Name(args)" and then with "//%PDDM-EXPAND-END" or another +expansions. All lines in-between are replaced by the result of the expansion. +The first line of the expansion is always a blank like just for readability. + +Expansion itself is pretty simple, one macro can invoke another macro, but +you cannot nest the invoke of a macro in another macro (i.e. - can't do +"foo(bar(a))", but you can define foo(a) and bar(b) where bar invokes foo() +within its expansion. + +When macros are expanded, the arg references can also add "$O" suffix to the +name (i.e. - "NAME$O") to specify an option to be applied. The options are: + + $S - Replace each character in the value with a space. + $l - Lowercase the first letter of the value. + $L - Lowercase the whole value. + $u - Uppercase the first letter of the value. + $U - Uppercase the whole value. + +Within a macro you can use ## to cause things to get joined together after +expansion (i.e. - "a##b" within a macro will become "ab"). + +Example: + + int foo(MyEnum x) { + switch (x) { + //%PDDM-EXPAND case(Enum_Left, 1) + //%PDDM-EXPAND case(Enum_Center, 2) + //%PDDM-EXPAND case(Enum_Right, 3) + //%PDDM-EXPAND-END + } + + //%PDDM-DEFINE case(_A, _B) + //% case _A: + //% return _B; + + A macro ends at the start of the next one, or an optional %PDDM-DEFINE-END + can be used to avoid adding extra blank lines/returns (or make it clear when + it is desired). + + One macro can invoke another by simply using its name NAME(ARGS). You cannot + nest an invoke inside another (i.e. - NAME1(NAME2(ARGS)) isn't supported). + + Within a macro you can use ## to cause things to get joined together after + processing (i.e. - "a##b" within a macro will become "ab"). + + +""" + +import optparse +import os +import re +import sys + + +# Regex for macro definition. +_MACRO_RE = re.compile(r'(?P\w+)\((?P.*?)\)') +# Regex for macro's argument definition. +_MACRO_ARG_NAME_RE = re.compile(r'^\w+$') + +# Line inserted after each EXPAND. +_GENERATED_CODE_LINE = ( + '// This block of code is generated, do not edit it directly.' +) + + +def _MacroRefRe(macro_names): + # Takes in a list of macro names and makes a regex that will match invokes + # of those macros. + return re.compile(r'\b(?P(?P(%s))\((?P.*?)\))' % + '|'.join(macro_names)) + +def _MacroArgRefRe(macro_arg_names): + # Takes in a list of macro arg names and makes a regex that will match + # uses of those args. + return re.compile(r'\b(?P(%s))(\$(?P