Skip to content

[JExtract] Import Foundation.Data if used #301

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

Merged
merged 2 commits into from
Jul 9, 2025

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Jul 7, 2025

  • Introduce KnownSwiftModules to represent Swift and Foundation modules.
  • SwiftSymbolTable.setup() is now a factory method to create SwiftSymbolTable. It scans the source file for module dependency. If import Foundation is found, add Foundation from the KnownSwiftModules (Just Data for now)
  • In Swift2JavaTranslator import Data APIs if any of the imported APIs use Data
  • Data has limited hardcoded APIs init(bytes:count:), getter:count, and withUnsafeBytes(_:).
  • Add import Foundation in Swift thunk if any of the the original source files import it

@rintaro rintaro marked this pull request as draft July 7, 2025 21:22
@rintaro rintaro force-pushed the jextract-jmm-data branch 2 times, most recently from a53dbd4 to 9f2c63e Compare July 8, 2025 19:19
@rintaro rintaro marked this pull request as ready for review July 8, 2025 23:05
@rintaro rintaro changed the title [WIP][JExtract] Import Foundation.Data if used [JExtract] Import Foundation.Data if used Jul 8, 2025
Copy link
Collaborator

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Very cool, lgtm -- some minor things inline

// FIXME: Make this conditional.
outputSwiftFiles += [
outputSwiftDirectory.appending(path: "Data+SwiftJava.swift")
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

That FIXME will be hard to resolve... we'd have to grep files manually for any foundation imports etc I guess before we run any of the commands :(

But yeah, good to keep that fixme

var str = retBytes.getString(0);
SwiftRuntime.trace("retStr=" + str);
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice!

btw I'm thinking we should start trying to move all these examples into tests, so we'll get a better report "what failed" when CI fails.

But this we can do later separately

@@ -65,6 +63,9 @@ package class FFMSwift2JavaGenerator: Swift2JavaGenerator {
return String(filePathPart.replacing(".swift", with: "+SwiftJava.swift"))
})
self.expectedOutputSwiftFiles.insert("\(translator.swiftModuleName)Module+SwiftJava.swift")

// FIXME: Can we avoid this?
self.expectedOutputSwiftFiles.insert("Data+SwiftJava.swift")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this is unavoidable in today's SwiftPM.

Can we call the file Foundation_Data+SwiftJava.swift maybe? just to lessen the chance of clashing with some user defined Data type?

FYI ongoing sadness with build plugins @bnbarham heh

if let dataDecl = self.symbolTable[.data] {
if self.isUsing(dataDecl) {
visitor.visit(nominalDecl: dataDecl.syntax!.asNominal!, in: nil)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Good enough for our current needs.

public init(cString: UnsafePointer<Int8>)
public func withCString(_ body: (UnsafePointer<Int8>) -> Void)
}
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense but I'm a bit worried about those "inline" pseudo-interface files... While we can't use the real files... maybe we could make this info at least real files and not just strings hardcoded like this, put them in SwiftTypes/KnownModules/Foundation.swiftinterface and then use them using the Bundle APIs? Seems that could work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will try making them resources in followups

// If we have already recorded a nominal type with the name in this module,
// it's an invalid redeclaration.
if let _ = symbolTable.lookupType(node.name.text, parent: parent) {
// TODO: Diagnose?
Copy link
Collaborator

Choose a reason for hiding this comment

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

might be good to at least log on debug here

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah, I forgot to hook up the Logger passed in to SwiftSymbolTable.setup()

Copy link
Collaborator

Choose a reason for hiding this comment

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

That’s be nice to do probably, thank you

if importedType.methods.contains(where: check) {
return true
}
if importedGlobalVariables.contains(where: check) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
if importedGlobalVariables.contains(where: check) {
if importedType.variables.contains(where: check) {

rintaro added 2 commits July 9, 2025 10:19
Instead of the pointer to the value witness table.
Witnesses expect type metadata.
* Introduce `KnownSwiftModules` to represent `Swift` and `Foundation`
  modules.
* `SwiftSymbolTable.setup()` is now a factory method to create
  `SwiftSymbolTable`. It scans the source file for module dependency.
  If `import Foundation` is found, add `Foundation` from the
  `KnownSwiftModules`  (Just `Data` for now)
* In `Swift2JavaTranslator` import `Data` APIs if any of the imported
  APIs use `Data`
* `Data` has limited hardcoded APIs `init(bytes:count:)`, `getter:count`,
  and `withUnsafeBytes(_:)`.
* Add `import Foundation` in Swift thunk if any of the the original
  source files import it
@rintaro rintaro force-pushed the jextract-jmm-data branch from 9d37c22 to c2f683e Compare July 9, 2025 17:21
@rintaro rintaro merged commit 8aa06d0 into swiftlang:main Jul 9, 2025
81 of 84 checks passed
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.

2 participants