[pigeon] [swift] Fix `PigeonError` sendability conformance in Swift 6 #8302

bc-lee · 2024-12-15T07:03:53Z

Ensure PigeonError conforms to Sendable for Swift 6.0 compatibility by updating the details property type from Any? to Sendable?.

Fixes flutter/flutter#160313

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

hellohuanlin · 2024-12-15T21:30:45Z

This will break old sdk where Sendable is undefined. You may have to put it behind a flag.

bc-lee · 2024-12-15T21:46:34Z

This will break old sdk where Sendable is undefined. You may have to put it behind a flag.

Are there any minimum Xcode requirements for Flutter projects? Sendable was added in Xcode 13.0 with Swift 5.5.

bc-lee · 2024-12-15T21:59:18Z

I found the answer.

Flutter 3.12 or later requires Xcode 14.0 or newer.
Source
Flutter 3.0 or later requires Xcode 13.0 or newer.
Source

So, I think it is safe to use this in Pigeon, as Pigeon requires Dart 3.3 and Flutter 3.19.

tarrinneal · 2024-12-19T19:03:00Z

packages/pigeon/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## NEXT


This should be updated to 22.7.1 along with the pubspec version and the generator_tools file.

I guess this is technically a breaking change, so 23.0.0 might be more correct.

tarrinneal · 2024-12-19T19:03:51Z

packages/pigeon/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## NEXT

 * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
+* [swift] Make `PigeonError` Class Conform to `Sendable`


[swift] Changes `PigeonError` class to conform to `Sendable`.

tarrinneal · 2024-12-19T19:13:01Z

@stuartmorgan shouldn't this pr be failing the test exemption check?

hellohuanlin

lgtm after addressing the comments.

stuartmorgan-g

@stuartmorgan shouldn't this pr be failing the test exemption check?

I think there's a false negative due to the changes to generated files in platform_tests; we should probably tune the Cocoon rules to ignore .gen. files in there.

Regardless, this does need testing. A Dart unit test should be fine for this for now (although long term we probably want to adjust the Swift integration tests to actually test a flow that ensures that everything is Sendable)

…dable?`

…pigeon-error-sendable

bc-lee · 2024-12-27T16:51:02Z

Regardless, this does need testing. A Dart unit test should be fine for this for now (although long term we probably want to adjust the Swift integration tests to actually test a flow that ensures that everything is Sendable)

I have added the Dart test introduced by this PR. I also agree that the long-term solution would involve updating the Swift integration tests—not just to check for @Sendable, but to cover all aspects related to Swift's strict concurrency model. However, I believe this would require additional setup.

…pigeon-error-sendable

tarrinneal

Thanks for putting this together, looks good to merge to me.

auto-submit · 2025-01-14T22:01:18Z

auto label is removed for flutter/packages/8302, due to This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan}, please make the needed changes and resubmit this PR.
The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

tarrinneal · 2025-01-14T22:16:40Z

@stuartmorgan you'll need to approve this one to land it.

stuartmorgan-g · 2025-01-21T20:08:53Z

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 23.0.0
+
+* [swift] Changes `PigeonError` class to conform to `Sendable`.


Breaking changes need to follow the CHANGELOG style guide for breaking changes.

https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version-and-changelog-updates

I don't think change log is accurate, because PigeonError already conforms to Error before this PR (and Errors are by design Sendable). This PR does not change this conformance.

How about "Fixes a compile error about PigeonError's sendability conformance in Swift 6"?

If it was already Sendable, why would we need to change Any? to Sendable? I'm confused.

The PigeonError type itself was Sendable, but its details field was not Sendable, hence the compile error in Swift 6 (or compile warning in Swift 5's strict mode).

That being said, this PR is still a breaking change tho, because the init signature is changed.

Sorry for the delay—I was on a national holiday.

Yes, the above discussion is correct. PigeonError already conforms to Error, which also makes it Sendable. However, the type of its details field prevents it from conforming under Strict Concurrency, leading to a compilation error in Swift 6.

Fixes made, enough reviews to land

hellohuanlin

Sorry for the block (I removed autosubmit tag), just saw the discussion on CHANGELOG and left a comment there: #8302 (comment)

tarrinneal · 2025-01-31T22:07:16Z

Sorry for the block (I removed autosubmit tag), just saw the discussion on CHANGELOG and left a comment there: #8302 (comment)

I fixed that, can you approve so we can land?

packages/pigeon/CHANGELOG.md

Co-authored-by: hellohuanlin <[email protected]>

auto-submit · 2025-02-03T18:09:56Z

autosubmit label was removed for flutter/packages/8302, because This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan}, please make the needed changes and resubmit this PR.
The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

tarrinneal · 2025-02-03T18:11:31Z

autosubmit label was removed for flutter/packages/8302, because This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan}, please make the needed changes and resubmit this PR. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@stuartmorgan not sure what the deal is here. I did "invalidate" your review since it had been addressed. Might be related.

auto-submit · 2025-02-03T18:45:12Z

autosubmit label was removed for flutter/packages/8302, because This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan}, please make the needed changes and resubmit this PR.
The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

stuartmorgan-g

Weird, seems like a Cocoon bug one way or another.

…in Swift 6 (flutter/packages#8302)

flutter/packages@02c6fef...e6ce02c 2025-02-05 [email protected] [vector_graphics] Allow transition between placeholder and loaded image to have an animation (flutter/packages#8195) 2025-02-04 [email protected] [flutter_markdown] Make custom table column alignments work when text wraps (flutter/packages#8340) 2025-02-04 [email protected] [interactive_media_ads] Adds internal wrapper for iOS native `IMAAdPodInfo` (flutter/packages#8429) 2025-02-03 [email protected] [pigeon] reorg generator files (flutter/packages#8532) 2025-02-03 [email protected] [pigeon] [swift] Fix `PigeonError` sendability conformance in Swift 6 (flutter/packages#8302) 2025-02-03 [email protected] Roll Flutter from b007899 to 8e2a6fc (61 revisions) (flutter/packages#8556) 2025-02-03 [email protected] [google_maps_flutter] Support for Ground Overlay - platform interface (flutter/packages#8518) 2025-01-31 [email protected] [tool] Add --xcode-warnings-exceptions flag (flutter/packages#8524) 2025-01-31 [email protected] [tool] Ensure that injected dependency overrides are sorted (flutter/packages#8542) 2025-01-31 [email protected] [vector_graphics] Revert leak tracker change (flutter/packages#8544) 2025-01-31 [email protected] [shared_preferences_tool] Loosen vm_service constraint to allow for 15 (flutter/packages#8539) 2025-01-31 [email protected] [in_app_purchase] Activate leak testing for android (flutter/packages#8369) 2025-01-31 [email protected] [flutter_markdown] Allow tables to be scrollable with IntrinsicColumnWidth (flutter/packages#8526) 2025-01-30 [email protected] Update CODEOWNERS for pkg:animations (flutter/packages#8534) 2025-01-30 [email protected] Roll Flutter from c1ffaa9 to b007899 (43 revisions) (flutter/packages#8527) 2025-01-30 [email protected] [video_player_web] Adjust Web implementation to the new platform interface (flutter/packages#8528) 2025-01-30 [email protected] [shared_preferences] Exposed SharedPreferencesOptions. (flutter/packages#8530) 2025-01-29 [email protected] Re-land [shared_preferences] Add shared preferences devtool (flutter/packages#8531) 2025-01-29 [email protected] [in_app_purchase_storekit] Add Swift Package Manager compatibility (flutter/packages#8469) 2025-01-29 [email protected] Revert "Re-land [shared_preferences] Add shared preferences devtool" (flutter/packages#8529) 2025-01-29 [email protected] [go_router_builder] Fixes trailing `?` by comparing iterables (flutter/packages#8521) 2025-01-29 [email protected] [tool] Refactor args of strings or YAML file lists (flutter/packages#8513) 2025-01-28 [email protected] [go_router] Add missing await keyword to onTap callback in the code example in `navigation.md` (flutter/packages#8343) 2025-01-28 [email protected] Re-land [shared_preferences] Add shared preferences devtool (flutter/packages#8519) 2025-01-28 [email protected] [vector_graphics] Fix memory leaks and activate leak testing [prod-leak-fix] (flutter/packages#8373) 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 [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

[pigeon][swift] Make PigeonError Class Conform to Sendable

8490ca5

github-actions bot added p: pigeon platform-ios platform-macos labels Dec 15, 2024

bc-lee marked this pull request as ready for review December 15, 2024 19:46

bc-lee requested review from hellohuanlin, LouiseHsu and tarrinneal as code owners December 15, 2024 19:46

tarrinneal requested changes Dec 19, 2024

View reviewed changes

hellohuanlin approved these changes Dec 19, 2024

View reviewed changes

bc-lee changed the title ~~[pigeon][swift] Make PigeonError Class Conform to Sendable~~ [pigeon] [swift] Changes PigeonError class to conform to Sendable. Dec 19, 2024

[pigeon] Update version and change name.

ff3c1fd

bc-lee requested a review from tarrinneal December 19, 2024 22:56

stuartmorgan-g requested changes Dec 26, 2024

View reviewed changes

bc-lee added 2 commits December 28, 2024 01:12

[pigeon][swift] Add test for PigeonError and ensure details is `Sen…

9f2fce2

…dable?`

Merge commit '9dffa395f42f5954f9ff90f6a461d192abfbe0a9' into feature/…

b92c6c9

…pigeon-error-sendable

bc-lee requested a review from stuartmorgan-g December 27, 2024 16:51

Merge commit '2e313dbf89b823f80a452f1c1d6c9070dd9b5862' into feature/…

51449a8

…pigeon-error-sendable

tarrinneal approved these changes Jan 14, 2025

View reviewed changes

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 14, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 14, 2025

hellohuanlin mentioned this pull request Jan 16, 2025

[pigeon] Support @Sendable in Swift code flutter/flutter#140439

Open

stuartmorgan-g previously requested changes Jan 21, 2025

View reviewed changes

Merge branch 'main' into feature/pigeon-error-sendable

d7abcbe

tarrinneal requested a review from stuartmorgan-g January 31, 2025 18:31

tarrinneal mentioned this pull request Jan 31, 2025

[pigeon] reorg generator files #8532

Merged

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2025

hellohuanlin removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2025

hellohuanlin requested changes Jan 31, 2025

View reviewed changes

Update CHANGELOG.md

7925bfd

tarrinneal requested a review from hellohuanlin January 31, 2025 22:06

hellohuanlin approved these changes Jan 31, 2025

View reviewed changes

packages/pigeon/CHANGELOG.md Show resolved Hide resolved

Update packages/pigeon/CHANGELOG.md

3a764d5

Co-authored-by: hellohuanlin <[email protected]>

bc-lee changed the title ~~[pigeon] [swift] Changes PigeonError class to conform to Sendable.~~ [pigeon] [swift] Fix PigeonError sendability conformance in Swift 6 Jan 31, 2025

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 3, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 3, 2025

Update CHANGELOG.md style

7634ac8

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 3, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 3, 2025

stuartmorgan-g approved these changes Feb 3, 2025

View reviewed changes

stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 3, 2025

auto-submit bot merged commit 3960c96 into flutter:main Feb 3, 2025
77 checks passed

bc-lee deleted the feature/pigeon-error-sendable branch February 3, 2025 20:12

engine-flutter-autoroll mentioned this pull request Feb 5, 2025

Roll Packages from 02c6fef4ef63 to e6ce02c2a859 (25 revisions) flutter/flutter#162744

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 5, 2025

3960c96e3 [pigeon] [swift] Fix PigeonError sendability conformance …

bb3d850

…in Swift 6 (flutter/packages#8302)

engine-flutter-autoroll mentioned this pull request Feb 5, 2025

Roll Packages from 02c6fef4ef63 to e6ce02c2a859 (25 revisions) flutter/flutter#162760

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 5, 2025

3960c96e3 [pigeon] [swift] Fix PigeonError sendability conformance …

6dda5e1

…in Swift 6 (flutter/packages#8302)

		@@ -1,3 +1,7 @@
		## 23.0.0

		* [swift] Changes `PigeonError` class to conform to `Sendable`.

[pigeon] [swift] Fix PigeonError sendability conformance in Swift 6 #8302

[pigeon] [swift] Fix PigeonError sendability conformance in Swift 6 #8302

Uh oh!

Conversation

bc-lee commented Dec 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

hellohuanlin commented Dec 15, 2024

Uh oh!

bc-lee commented Dec 15, 2024

Uh oh!

bc-lee commented Dec 15, 2024

Uh oh!

tarrinneal Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

tarrinneal Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

tarrinneal commented Dec 19, 2024

Uh oh!

hellohuanlin left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

bc-lee commented Dec 27, 2024

Uh oh!

tarrinneal left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Jan 14, 2025

Uh oh!

tarrinneal commented Jan 14, 2025

Uh oh!

stuartmorgan-g Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

tarrinneal Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarrinneal Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

bc-lee Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

hellohuanlin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarrinneal commented Jan 31, 2025

Uh oh!

Uh oh!

auto-submit bot commented Feb 3, 2025

Uh oh!

tarrinneal commented Feb 3, 2025

Uh oh!

auto-submit bot commented Feb 3, 2025

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[pigeon] [swift] Fix `PigeonError` sendability conformance in Swift 6 #8302

[pigeon] [swift] Fix `PigeonError` sendability conformance in Swift 6 #8302

bc-lee commented Dec 15, 2024 •

edited

Loading

hellohuanlin Jan 31, 2025 •

edited

Loading

hellohuanlin left a comment •

edited

Loading