Commit 1bce70dd authored by Feng Xiao's avatar Feng Xiao

Fix compatiblity issues.

Currently some public API methods are defined in GenreatedMessage.java
and they have a generric return type:
  class GeneratedMessage {
    class Builder<BuilderType extends Builder<BuilderType>> {
      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<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
parent 0b682551
......@@ -1396,7 +1396,7 @@ public abstract class GeneratedMessage extends AbstractMessage
return setExtension((ExtensionLite<MessageType, Type>) extension, value);
}
/** Set the value of an extension. */
public final <Type> BuilderType setExtension(
public <Type> BuilderType setExtension(
final GeneratedExtension<MessageType, Type> extension, final Type value) {
return setExtension((ExtensionLite<MessageType, Type>) extension, value);
}
......@@ -1407,7 +1407,7 @@ public abstract class GeneratedMessage extends AbstractMessage
return setExtension((ExtensionLite<MessageType, List<Type>>) extension, index, value);
}
/** Set the value of one element of a repeated extension. */
public final <Type> BuilderType setExtension(
public <Type> BuilderType setExtension(
final GeneratedExtension<MessageType, List<Type>> extension,
final int index, final Type value) {
return setExtension((ExtensionLite<MessageType, List<Type>>) extension, index, value);
......@@ -1418,7 +1418,7 @@ public abstract class GeneratedMessage extends AbstractMessage
return addExtension((ExtensionLite<MessageType, List<Type>>) extension, value);
}
/** Append a value to a repeated extension. */
public final <Type> BuilderType addExtension(
public <Type> BuilderType addExtension(
final GeneratedExtension<MessageType, List<Type>> extension, final Type value) {
return addExtension((ExtensionLite<MessageType, List<Type>>) extension, value);
}
......@@ -1428,7 +1428,7 @@ public abstract class GeneratedMessage extends AbstractMessage
return clearExtension((ExtensionLite<MessageType, ?>) extension);
}
/** Clear an extension. */
public final <Type> BuilderType clearExtension(
public <Type> BuilderType clearExtension(
final GeneratedExtension<MessageType, ?> extension) {
return clearExtension((ExtensionLite<MessageType, ?>) extension);
}
......
......@@ -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 <Type> 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 <Type> Builder setExtension(\n"
" com.google.protobuf.GeneratedMessage.GeneratedExtension<\n"
" $classname$, java.util.List<Type>> extension,\n"
" int index, Type value) {\n"
" return (Builder) super.setExtension(extension, index, value);\n"
"}\n"
"public <Type> Builder addExtension(\n"
" com.google.protobuf.GeneratedMessage.GeneratedExtension<\n"
" $classname$, java.util.List<Type>> extension,\n"
" Type value) {\n"
" return (Builder) super.addExtension(extension, value);\n"
"}\n"
"public <Type> 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_)) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment