Fix unnecessary toList/fromList calls during encode/decode process #6426
Conversation
tarrinneal
requested review from
stuartmorgan
and removed request for
hellohuanlin
Per offline discussion I think we should adjust the PR description here, as well as the changelog. It's not that things were double-encoding custom classes, it's that we were making the encoding logic needlessly complex by calling ToList/FromList directly instead of letting the encoder do the encoding internally. (This PR will actually make the byte encoding slightly less efficient; before a custom class was encoded as |
| @@ -186,15 +186,15 @@ ArrayList<Object> toList() { | |||
| return toListResult; | |||
| } | |||
|
|
|||
| static @NonNull MessageData fromList(@NonNull ArrayList<Object> list) { | |||
| static @NonNull MessageData fromList(@NonNull ArrayList<Object> __pigeon_list) { | |||
There was a problem hiding this comment.
Doesn't need to be in this PR, but we should consider just inlining all of the value expressions into the setter calls so we don't need variables, and thus don't have a collision concern.
| @@ -3,6 +3,7 @@ | |||
| // found in the LICENSE file. | |||
| // Autogenerated from Pigeon, do not edit directly. | |||
| // See also: https://pub.dev/packages/pigeon | |||
| @file:Suppress("UNCHECKED_CAST", "LocalVariableName", "ArrayInDataClass") | |||
There was a problem hiding this comment.
Why the change? Don't we want to make the suppressions for this as targeted as possible?
There was a problem hiding this comment.
most of them are full file regardless, it gives "redundant suppression" errors.
|
Now I remember why this error looked familiar: it's a bug in VS 2019, which was fixed in VS 2022. We actually shouldn't still be using VS 2019 in CI, but it's going to be tricky to update that right now so I'll dig up the workaround. IIRC it will just require turning on exceptions for the test app. |
Seems like it may be something else still... |
Or it is that same problem, but I put the workaround on the example app binary when the failure is coming from a unit test file that's not compiled in that binary, and I need to change the build settings of the thing that was actually failing to compile 🤦🏻 |
tarrinneal
changed the title
tarrinneal
added
the
autosubmit
…lutter#6426) Fix unnecessary toList/fromList calls during encode/decode process. also makes some kotlin code more idiomatic. removes the ability to have collisions with the name `list`. fixes flutter/flutter#119351
flutter/packages@f4719ca...2dfe645 2024-05-06 PROGrand@users.noreply.github.com [camera] MediaSettings parameter for createCameraWithSettings (flutter/packages#3586) 2024-05-06 tarrinneal@gmail.com Fix unnecessary toList/fromList calls during encode/decode process (flutter/packages#6426) 2024-05-03 engine-flutter-autoroll@skia.org Roll Flutter from bf7191f to f1037a0 (21 revisions) (flutter/packages#6641) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fix unnecessary toList/fromList calls during encode/decode process.
also makes some kotlin code more idiomatic.
removes the ability to have collisions with the name
list.fixes flutter/flutter#119351
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.