-
Notifications
You must be signed in to change notification settings - Fork 195
Fix a few extension field bugs #1062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b450740 to
8335677
Compare
osa1
commented
Oct 15, 2025
|
|
||
| * Fix `GeneratedMessage.getExtension` returning differently typed lists when the | ||
| message extension field set is initialized and frozen and initialized but not | ||
| frozen. ([#1062]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: this is not fixing a bug caused by the previous entry, it exists in the master branch independently of the other bugs. However the repro is different in the master branch vs. this branch.
sigurdm
reviewed
Oct 20, 2025
sigurdm
approved these changes
Oct 20, 2025
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this pull request
Oct 21, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`. protobuf (https://github.com/dart-lang/protobuf/compare/14bbd0b..78cf743): 78cf743 2025-10-20 Ömer Sinan Ağacan Fix a few extension field bugs (google/protobuf.dart#1062) tools (https://github.com/dart-lang/tools/compare/f5920a2..d0941a3): d0941a35 2025-10-17 Morgan :) Unify watcher tests (dart-lang/tools#2211) fa978cd2 2025-10-17 Morgan :) Remove backup expectations (dart-lang/tools#2209) web (https://github.com/dart-lang/web/compare/816abcc..5a7d0be): 5a7d0be 2025-10-16 Srujan Gaddam Initial commit of package:js_interop 0.1.0-beta (dart-lang/web#476) webdev (https://github.com/dart-lang/webdev/compare/2517aa9..82b3855): 82b38557 2025-10-20 Jessy Yameogo [DWDS] Fixes hot reload/restart crashes after closing browser tab on web-server device (dart-lang/webdev#2699) Change-Id: I98c840892d243f3dc34f9409956d8726ff84238a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/456040 Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Auto-Submit: Devon Carew <devoncarew@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When an extension field set isn't initialized before freezing the message:
addExtensionallows adding and modifying extensions.getExtensionreturns non-frozen defaults that can be modified.Fix this by initializing the extension field set as frozen when the parent message is frozen.
Secondly, extension field set returns differently typed lists when it's frozen and not frozen. Fix this by always creating the list via the field's
FieldInfo.Also refactor the code a little bit:
GeneratedMessageby-passes a layer of abstraction and accesses unknown field sets directly. Instead make it callFieldSetalways.New
FieldSetmethods are added as public (without an underscore in the name) to make it clear that they're a public interface (can be called fromGeneratedMessage).(Ideally
FieldSetshould be a separate library, but that requires a lot of refactoring to split the parts into libraries.)Also remove duplicate "is repeated" and "ensure writable" checks when modifying extension fields.
cl/819146715