iOS: Eliminate fml::scoped_nsobject pointer use #56295
Conversation
cbracken
requested review from
chinmaygarde,
stuartmorgan,
jmagman and
jonahwilliams
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
cbracken
force-pushed
the
no-scoped-nsobject
branch
2 times, most recently
from
520e7d5 to
12e83bf
Compare
|
This will be followed by a separate patch that deletes scoped_nsobject, its tests, and cleans up the build file. |
shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm
Outdated
Show resolved
Hide resolved
|
test-exempt: code refactor with no semantic change |
cbracken
force-pushed
the
no-scoped-nsobject
branch
from
9497671 to
cc1361c
Compare
shell/platform/darwin/ios/framework/Source/platform_views_controller.mm
Outdated
Show resolved
Hide resolved
Eliminates use of `fml::scoped_nsobject` now that the codebase has been migrated to ARC. Issue: flutter/flutter#137801
cbracken
force-pushed
the
no-scoped-nsobject
branch
from
cc1361c to
d1cacfd
Compare
cbracken
added
the
autosubmit
| NSMutableDictionary<NSNumber*, SemanticsObject*>* objects_; | ||
| FlutterBasicMessageChannel* accessibility_channel_; |
There was a problem hiding this comment.
I guess even if they are fully Objective-C isa objects we don't use Objective-C naming (objects and accessibilityChannel)?
There was a problem hiding this comment.
In C++ contexts (C++ class fields) the style guide says we have to use C++ naming, which really hits the code with an ugly stick :(
Ref: https://google.github.io/styleguide/objcguide.html#style-matches-the-language
There was a problem hiding this comment.
If the naming of ivars and variables depended on the thing being pointed to rather than the context, things would be much, much uglier.
I guarantee nobody would be happier if individual lines of Obj-C++ code had mixes of different variable naming styles depending on the underlying types of the things the variables are referring to.
There was a problem hiding this comment.
Agreed, but it's still unfortunate. I'd much prefer we reduced the number of C++ classes with Obj-C++ implementations and used straight Obj-C[++] classes where possible. I realise that's not always possible -- e.g. cases where embedders have some translation unit containing subclasses of a common C++ superclass etc.
…158132) flutter/engine@25c7e47...f880b56 2024-11-04 [email protected] Made it so angle builds on linux (flutter/engine#56328) 2024-11-04 [email protected] [Impeller] combine translate* scale mat mul when computing shader transform. (flutter/engine#56352) 2024-11-04 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.2.1 to 4.2.2 (flutter/engine#56191) 2024-11-04 [email protected] iOS: Eliminate fml::scoped_nsobject pointer use (flutter/engine#56295) 2024-11-04 [email protected] Roll Skia from e2ad60ea8039 to b2bb3af36da3 (1 revision) (flutter/engine#56355) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] 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
Eliminates use of `fml::scoped_nsobject` now that the codebase has been migrated to ARC. Issue: flutter#137801 No changes to tests since this patch makes no semantic changes. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Eliminates use of
fml::scoped_nsobjectnow that the codebase has been migrated to ARC.Issue: flutter/flutter#137801
No changes to tests since this patch makes no semantic changes.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.