Change GeneratedMessage.toBuilder() to only make a shallow copy
#145
Conversation
|
cc @nichite |
|
cc @rakudrama you might be interested. |
| _values.setRange(0, original._values.length, original._values); | ||
| for (int index = 0; index < _meta.byIndex.length; index++) { | ||
| FieldInfo fieldInfo = _meta.byIndex[index]; | ||
| if (fieldInfo.isRepeated) { |
There was a problem hiding this comment.
Do something similiar for map fields?
protoc_plugin/CHANGELOG.md
Outdated
| @@ -1,3 +1,9 @@ | |||
| ## 0.13.0 | |||
|
|
|||
| * Breaking change: generated message classes now have a new instance method `createNewInstance` that | |||
There was a problem hiding this comment.
Please try to avoid adding instance methods.
I would like to do a compiler optimization where FooMessage is replaced with GeneratedMessage unless there is a test like 'is FooMessage'.
This will remove many of the thousands of generated classes, but it is only possible if the generated message classes have instance methods that are always inlined (the field get/set/has methods are resolved to one target and inlined, leaving the class 'empty').
createNewInstance is defined in the base class and overridden so is not inlined.
This can be fixed by storing FooMessage.create in the BuilderInfo, and using it from there.
There was a problem hiding this comment.
Sad that I got to release a version before seeing your comment. :(
We have the same problem with clone() no?
Could we store the FooMessage.create() method in BuilderInfo as you suggest, and then make this method non-abstract and use that?
protoc_plugin/pubspec.yaml
Outdated
| @@ -23,3 +23,7 @@ dev_dependencies: | |||
|
|
|||
| executables: | |||
| protoc-gen-dart: protoc_plugin | |||
|
|
|||
| dependency_overrides: | |||
| import 'package:test/test.dart'; | ||
|
|
||
| main() { | ||
| test('frozen behavior', () { |
# Conflicts: # protobuf/CHANGELOG.md # protobuf/pubspec.yaml # protoc_plugin/CHANGELOG.md
extend tests.
protobuf/CHANGELOG.md
Outdated
| `GeneratedMessage` has a new abstract method: `createEmptyInstance()` that subclasses must | ||
| implement. | ||
|
|
||
| Proto files must be rebuilt using protoc_plugin 0.14.0 or newer. |
There was a problem hiding this comment.
update protoc version dependency
protobuf/CHANGELOG.md
Outdated
| @@ -8,6 +17,7 @@ | |||
| Map fields are now represented as Dart maps and are accessed through a getter with the same name as the map field. | |||
| To use the map support, use Dart protoc_plugin version 11.0 or newer. | |||
|
|
|||
|
|
|||
| // for sub-messages, instead of cloning everything here. | ||
| GeneratedMessage toBuilder() => clone(); | ||
| /// | ||
| /// The lists for repeated fields are also copied. |
protoc_plugin/CHANGELOG.md
Outdated
| @@ -1,3 +1,9 @@ | |||
| ## 0.13.0 | |||
|
|
|||
| * Breaking change: generated message classes now have a new instance method `createNewInstance` that | |||
# Conflicts: # protobuf/CHANGELOG.md # protobuf/pubspec.yaml # protoc_plugin/CHANGELOG.md # protoc_plugin/pubspec.yaml
No description provided.