Skip to content

FoundationEssentials,FoundationInternationalization: adjust for aliasing #1340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Jun 8, 2025

When enabling symbol renaming in ICU, we import the functions as an alias. This causes the implicit nullability translation with C to disappear. Make these conversions explicit so that we can enable symbol renaming on non-Darwin platforms.

When enabling symbol renaming in ICU, we import the functions as an
alias. This causes the implicit nullability translation with C to
disappear. Make these conversions explicit so that we can enable symbol
renaming on non-Darwin platforms.
@@ -117,6 +117,7 @@ import Darwin
#elseif canImport(Android)
@preconcurrency import Android
import posix_filesystem.dirent
internal import _FoundationCShims
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this file uses ICU - is this change required / relevant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was needed to resolve some of the aliases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the functions that would break without this? While I can believe this change is needed for us to build, I'm surprised that it's because of the ICU change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S:/SourceCache/swift-project/swift-foundation/Sources/FoundationEssentials/FileManager/FileOperations+Enumeration.swift:390:37: error: property 'd_type' is not available due to missing import of defining module '_FoundationCShims' [#MemberImportVisibility]
118 | @preconcurrency import Android
119 | import posix_filesystem.dirent
120 | // internal import _FoundationCShims
    | `- note: add import of module '_FoundationCShims'
121 | #elseif canImport(Glibc)
122 | @preconcurrency import Glibc
    :
388 |                 if appendSlash {
389 |                     var isDirectory = false
390 |                     if dent.pointee.d_type == DT_DIR {
    |                                     `- error: property 'd_type' is not available due to missing import of defining module '_FoundationCShims' [#MemberImportVisibility]
391 |                         isDirectory = true
392 |                     } else if dent.pointee.d_type == DT_UNKNOWN {

Copy link
Contributor

@itingliu itingliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ICU related parts look good to me

@@ -117,6 +117,7 @@ import Darwin
#elseif canImport(Android)
@preconcurrency import Android
import posix_filesystem.dirent
internal import _FoundationCShims
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the functions that would break without this? While I can believe this change is needed for us to build, I'm surprised that it's because of the ICU change.

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd compnerd requested review from itingliu and jmschonfeld June 23, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants