From 1bce70dddb57aa82c3e6a74b072f6a742f47c900 Mon Sep 17 00:00:00 2001 From: Feng Xiao Date: Thu, 14 Jul 2016 15:33:45 -0700 Subject: Fix compatiblity issues. Currently some public API methods are defined in GenreatedMessage.java and they have a generric return type: class GeneratedMessage { class Builder> { public BuilderType setField(...); public BuilderType setExtension(...); } } With these definitions, the compiled byte code of a callsite will have a direct reference to GeneratedMessage. For example: fooBuilder.setField(...); becomes: ##: invokevirtual // Method Builder.setField:(...)LGeneratedMessage.Builder ##: checkcast // class Builder This will prevent us from updating generated classes to subclass a different versioned GeneratedMessageV3 class in the future (we can't do it in a binary compatible way). This change addresses the problem by overriding these methods directly in the generated class: class Foo { class Builder extends GeneratedMessage.Builder { public Builder setField(...) { return super.setField(...); } } } After this, fooBuilder.setField(...) will be compiled to: ##: invokevirtual // Method Builder.setField:(...)LFoo.Builder The callsites will no longer reference GeneratedMessage directly and we can change Foo to subclass GeneratedMessageV3 without breaking binary compatiblity. The downside of this change is: 1. It increases generated code size (though it saves some instructions on the callsites). 2. We can never stop generating these overrides because doing that will break binary compatibility. Change-Id: I879afbbc1325a66324a51565e017143489b06e97 --- .../protobuf/compiler/java/java_message_builder.cc | 68 ++++++++++++++++++++++ 1 file changed, 68 insertions(+) (limited to 'src') diff --git a/src/google/protobuf/compiler/java/java_message_builder.cc b/src/google/protobuf/compiler/java/java_message_builder.cc index b3e9e986..81a70a1f 100644 --- a/src/google/protobuf/compiler/java/java_message_builder.cc +++ b/src/google/protobuf/compiler/java/java_message_builder.cc @@ -181,6 +181,18 @@ Generate(io::Printer* printer) { " return this;\n" "}\n" "\n"); + } else { + printer->Print( + "public final Builder setUnknownFields(\n" + " final com.google.protobuf.UnknownFieldSet unknownFields) {\n" + " return super.setUnknownFields(unknownFields);\n" + "}\n" + "\n" + "public final Builder mergeUnknownFields(\n" + " final com.google.protobuf.UnknownFieldSet unknownFields) {\n" + " return super.mergeUnknownFields(unknownFields);\n" + "}\n" + "\n"); } printer->Print( @@ -438,6 +450,62 @@ GenerateCommonBuilderMethods(io::Printer* printer) { "\n", "classname", name_resolver_->GetImmutableClassName(descriptor_)); + printer->Print( + "public Builder clone() {\n" + " return (Builder) super.clone();\n" + "}\n" + "public Builder setField(\n" + " com.google.protobuf.Descriptors.FieldDescriptor field,\n" + " Object value) {\n" + " return (Builder) super.setField(field, value);\n" + "}\n" + "public Builder clearField(\n" + " com.google.protobuf.Descriptors.FieldDescriptor field) {\n" + " return (Builder) super.clearField(field);\n" + "}\n" + "public Builder clearOneof(\n" + " com.google.protobuf.Descriptors.OneofDescriptor oneof) {\n" + " return (Builder) super.clearOneof(oneof);\n" + "}\n" + "public Builder setRepeatedField(\n" + " com.google.protobuf.Descriptors.FieldDescriptor field,\n" + " int index, Object value) {\n" + " return (Builder) super.setRepeatedField(field, index, value);\n" + "}\n" + "public Builder addRepeatedField(\n" + " com.google.protobuf.Descriptors.FieldDescriptor field,\n" + " Object value) {\n" + " return (Builder) super.addRepeatedField(field, value);\n" + "}\n"); + + if (descriptor_->extension_range_count() > 0) { + printer->Print( + "public Builder setExtension(\n" + " com.google.protobuf.GeneratedMessage.GeneratedExtension<\n" + " $classname$, Type> extension,\n" + " Type value) {\n" + " return (Builder) super.setExtension(extension, value);\n" + "}\n" + "public Builder setExtension(\n" + " com.google.protobuf.GeneratedMessage.GeneratedExtension<\n" + " $classname$, java.util.List> extension,\n" + " int index, Type value) {\n" + " return (Builder) super.setExtension(extension, index, value);\n" + "}\n" + "public Builder addExtension(\n" + " com.google.protobuf.GeneratedMessage.GeneratedExtension<\n" + " $classname$, java.util.List> extension,\n" + " Type value) {\n" + " return (Builder) super.addExtension(extension, value);\n" + "}\n" + "public Builder clearExtension(\n" + " com.google.protobuf.GeneratedMessage.GeneratedExtension<\n" + " $classname$, ?> extension) {\n" + " return (Builder) super.clearExtension(extension);\n" + "}\n", + "classname", name_resolver_->GetImmutableClassName(descriptor_)); + } + // ----------------------------------------------------------------- if (context_->HasGeneratedMethods(descriptor_)) { -- cgit v1.2.3