Repository navigation
Make InternalFlutterSwift & InternalFlutterSwiftCommon internal for real - #190494
Conversation
github-actions
Bot
added
platform-ios
d0146c6 to
b5209f9
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request restricts the visibility of various Swift classes, structs, protocols, and members across the Darwin platform implementation by removing explicit public and internal modifiers, and updates corresponding tests to use @testable import. The review feedback suggests further optimizing access control in Tracing+TraceScope.swift by removing @inlinable and @usableFromInline attributes from internal members, and identifies a typo in a comment within BUILD.gn.
github-actions
Bot
added
a: text input
2ac7983 to
db47c9b
Compare
github-actions
Bot
removed
a: text input
| /// prevent timeline corruption. | ||
| @objc(FlutterTraceScope) | ||
| public final class TraceScope: NSObject { | ||
| final class TraceScope: NSObject { |
There was a problem hiding this comment.
Removed the inlinable as gemini suggested, because the they're exposing symbols. According to the documentation that's expected behavior.
These APIs are currently not used. Maybe we can turn this into a freestanding macro instead of relying on the compiler to inline withTrace.
There was a problem hiding this comment.
Yes! I have a branch that plays with this but getting macro support requires some build work in the toolchain stuff.
LongCatIsLooong
changed the title
InternalFlutterSwift internalInternalFlutterSwift & InternalFlutterSwiftCommon internal for real
hellohuanlin
left a comment
hellohuanlin
left a comment
There was a problem hiding this comment.
I'm curious what was the original issue that forced us to use public? Is adding the bridging header the fix for that issue?
From what I was seeing before making the bridging header change, the Objc header I was able to fix that using |
LongCatIsLooong
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
3 participants
Marking types and type members internal prevents the swift compiler from marking the corresponding symbols (in the case of
@objc, both the Swift and Objc symbols) visible.Unfortunately since the CI is still on Swift 6.2, so theAdding a bridging header fixed that.-emit-clang-header-min-accessflag is not available. As a result theengine/src/out/host_debug_unopt_arm64/gen/flutter/shell/platform/darwin/common/test_utils_swift/test_utils_swift.hfile swift compiler emits doesn't actually include the interface ofStringOutputWritersince it's internal. The only workaround I found was to manually add the interface.With these changes, when I build the iOS
Flutter.frameworkwithenable-testingset to false, the only visible swift symbols are from a class marked@usableFromInline(_FlutterTraceScope). And as gemini suggested I removed the@inlinablefrom that class. So no Swift symbols or internal objc symbols:Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.