diff --git a/protobuf/CHANGELOG.md b/protobuf/CHANGELOG.md index 207a68ad2..186f72675 100644 --- a/protobuf/CHANGELOG.md +++ b/protobuf/CHANGELOG.md @@ -19,8 +19,15 @@ `@useResult` and will generate a warning when its result is not used. ([#896]) +* **Breaking:** `PbMap.unmodifiable` now takes key and value field types as + arguments, instead of another `PbMap`. + + To migrate, use `PbMap.unmodifiable(map.keyFieldType, map.valueFieldType)` + instead of `PbMap.unmodifiable(map)`. ([#902]) + [#738]: https://github.com/google/protobuf.dart/issues/738 [#896]: https://github.com/google/protobuf.dart/issues/896 +[#902]: https://github.com/google/protobuf.dart/issues/902 ## 3.1.0 diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 0c8b064fa..af86d641a 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -395,8 +395,7 @@ class _FieldSet { assert(fi.isMapField); if (_isReadOnly) { - return PbMap.unmodifiable( - PbMap(fi.keyFieldType, fi.valueFieldType)); + return PbMap.unmodifiable(fi.keyFieldType, fi.valueFieldType); } final map = fi._createMapField(); diff --git a/protobuf/lib/src/protobuf/pb_list.dart b/protobuf/lib/src/protobuf/pb_list.dart index 045d5f855..76a73287a 100644 --- a/protobuf/lib/src/protobuf/pb_list.dart +++ b/protobuf/lib/src/protobuf/pb_list.dart @@ -11,7 +11,20 @@ typedef CheckFunc = void Function(E? x); /// A [ListBase] implementation used for protobuf `repeated` fields. class PbList extends ListBase { + /// The actual list storing the elements. + /// + /// Note: We want only one [List] implementation class to be stored here to + /// make sure the list operations are monomorphic and can be inlined. In + /// constructors make sure initializers for this field all return the same + /// implementation class. (e.g. `_GrowableList` on the VM) final List _wrappedList; + + /// A growable list, to be used in `unmodifiable` constructor to avoid + /// allocating a list every time. + /// + /// We can't use `const []` as it makes the `_wrappedList` field polymorphic. + static final _emptyList = []; + final CheckFunc _check; bool _isReadOnly = false; @@ -23,11 +36,11 @@ class PbList extends ListBase { _check = check; PbList.unmodifiable() - : _wrappedList = const [], + : _wrappedList = _emptyList, _check = _checkNotNull, _isReadOnly = true; - PbList.from(List from) + PbList.from(List from) : _wrappedList = List.from(from), _check = _checkNotNull; diff --git a/protobuf/lib/src/protobuf/pb_map.dart b/protobuf/lib/src/protobuf/pb_map.dart index 5d9bc6064..f123e5880 100644 --- a/protobuf/lib/src/protobuf/pb_map.dart +++ b/protobuf/lib/src/protobuf/pb_map.dart @@ -21,24 +21,33 @@ class PbMap extends MapBase { static const int _keyFieldNumber = 1; static const int _valueFieldNumber = 2; + /// The actual list storing the elements. + /// + /// Note: We want only one [Map] implementation class to be stored here to + /// make sure the map operations are monomorphic and can be inlined. In + /// constructors make sure initializers for this field all return the same + /// implementation class. final Map _wrappedMap; - bool _isReadonly = false; + /// The map instance to be used in `PbMap.unmodifiable`. + /// + /// We can't use `const {}` as it makes the `_wrappedMap` field polymorphic. + static final _emptyMap = {}; + + bool _isReadOnly = false; PbMap(this.keyFieldType, this.valueFieldType) : _wrappedMap = {}; - PbMap.unmodifiable(PbMap other) - : keyFieldType = other.keyFieldType, - valueFieldType = other.valueFieldType, - _wrappedMap = Map.unmodifiable(other._wrappedMap), - _isReadonly = true; + PbMap.unmodifiable(this.keyFieldType, this.valueFieldType) + : _wrappedMap = _emptyMap, + _isReadOnly = true; @override V? operator [](Object? key) => _wrappedMap[key]; @override void operator []=(K key, V value) { - if (_isReadonly) { + if (_isReadOnly) { throw UnsupportedError('Attempted to change a read-only map field'); } ArgumentError.checkNotNull(key, 'key'); @@ -77,7 +86,7 @@ class PbMap extends MapBase { @override void clear() { - if (_isReadonly) { + if (_isReadOnly) { throw UnsupportedError('Attempted to change a read-only map field'); } _wrappedMap.clear(); @@ -88,7 +97,7 @@ class PbMap extends MapBase { @override V? remove(Object? key) { - if (_isReadonly) { + if (_isReadOnly) { throw UnsupportedError('Attempted to change a read-only map field'); } return _wrappedMap.remove(key); @@ -111,7 +120,7 @@ class PbMap extends MapBase { } PbMap freeze() { - _isReadonly = true; + _isReadOnly = true; if (_isGroupOrMessage(valueFieldType)) { for (final subMessage in values as Iterable) { subMessage.freeze();