-
Notifications
You must be signed in to change notification settings - Fork 6k
Ensure fields of Rect and OffsetBase classes are optimized as non-null. #16465
Conversation
lib/ui/geometry.dart
Outdated
| : _dx = dx ?? 0.0 | ||
| , _dy = dy ?? 0.0 | ||
| , assert(dx != null) | ||
| , assert(dy != null); |
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.
nit: can this be made consistent with the rest of the file? I think commas go at the end of the lines rather than the start.
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.
Is it enough to just assert these are not null rather than coalescing? Here and below.
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.
I would rather a user know they tried to pass null where they shouldn't than find out they passed null and got 0.
It's already an error to pass null based on the rest of the code.
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.
Unfortunately we can't just add an assert, because the compiler needs to be able to prove that the field will never be null in release mode, where the asserts aren't triggered.
If the constructor wasn't const, we would throw an exception unconditionally instead. Is it not sufficient to prevent those kinds of errors with an assert in debug mode?
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.
Ahh that makes sense.
I think this is fine then. Thanks!
dnfield
left a comment
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.
Lgtm
|
I bet applying this to the vector_math package would help too |
* c932214 Ensure fields of Rect and OffsetBase classes are optimized as non-null. (flutter/engine#16465) * 5c70356 Simplify task queues locking mechanism (flutter/engine#16477) * d589dde Fix text range logic for a11y (flutter/engine#16496) * 1a4f4e3 Fix unused import in Android embedder (flutter/engine#16494) * 964ae10 Disable ShellTest.WaitForFirstFrameTimeout on Fuchsia (flutter/engine#16495) * 6158f03 Roll src/third_party/skia 87e3bef6f82f..9f3eef796f63 (7 commits) (flutter/engine#16493)
…l. (flutter#16465) * Ensure fields of Rect and OffsetBase classes are optimized as non-null. * Update web_ui and formatting
… non-null. (flutter#16465)" This reverts commit da4baed.
By using the
field ?? 0.0pattern, we can ensure that the compiler knows the field will never be null, even when asserts are not enabled. This optimization improves Flutter Gallery code size:ARM64: -1.06%
ARM: -1.02%
/cc @mkustermann @alexmarkov @mraleph