Repository navigation
Conversation
ktoso
left a comment
ktoso
left a comment
There was a problem hiding this comment.
Pretty good work! Almost there 👍
Please add runtime tests by adding functions returning such buffers to Samples/SwiftJavaExtractJNISampleApp/Sources/MySwiftLibrary and then adding tests in Java using them in Samples/SwiftJavaExtractJNISampleApp/src/test/java
|
Also same tests in FFM since we claim to support it now |
| public func bufferCount(data: UnsafeRawBufferPointer) -> Int64 { | ||
| Int64(data.count) | ||
| } | ||
| // snippet.end |
There was a problem hiding this comment.
This would break existing snippets
ba02d62 to
eb8a4cf
Compare
00a8cc5 to
a720acb
Compare
|
Sorry about the pr run fails. I currently have a windows, and I'm working on setting up my wsl so I can run github actions on my machine |
|
No problem, happy to rerun. You may have more success trying a docker container hmmm |
… FFM and JNI generators fixed test for UnsafeBufferPointer
663e446 to
290356a
Compare
| expectedChunks: [ | ||
| """ | ||
| do { | ||
| let baseAddressBits$ = Int64(Int(bitPattern: SwiftModule.globalReturnUnsafeBufferPointer().baseAddress)) |
There was a problem hiding this comment.
We call SwiftModule.globalReturnUnsafeBufferPointer here and
| do { | ||
| let baseAddressBits$ = Int64(Int(bitPattern: SwiftModule.globalReturnUnsafeBufferPointer().baseAddress)) | ||
| environment.interface.SetLongField(environment, resultOut, _JNIMethodIDCache.SwiftUnsafeBufferPointer.baseAddress, baseAddressBits$.getJNIValue(in: environment)) | ||
| let countBits$ = Int64(SwiftModule.globalReturnUnsafeBufferPointer().count) |
There was a problem hiding this comment.
here again SwiftModule.globalReturnUnsafeBufferPointer
this is not ok, the global function may have side effects so we call it twice here potentially causing some effect twice when it should be once
| printer.print( | ||
| """ | ||
| let baseAddressBits$ = Int64(Int(bitPattern: \(inner).baseAddress)) | ||
| environment.interface.SetLongField(environment, \(outArgumentName), _JNIMethodIDCache.SwiftUnsafeBufferPointer.baseAddress, baseAddressBits$.getJNIValue(in: environment)) |
There was a problem hiding this comment.
These are reusing the same ID but they may be mutable or not, we must use SwiftUnsafeMutableBufferPointer if it was mutable
| let baseAddressBits$ = Int64(Int(bitPattern: \(inner).baseAddress)) | ||
| environment.interface.SetLongField(environment, \(outArgumentName), _JNIMethodIDCache.SwiftUnsafeBufferPointer.baseAddress, baseAddressBits$.getJNIValue(in: environment)) | ||
| let countBits$ = Int64(\(inner).count) | ||
| environment.interface.SetLongField(environment, \(outArgumentName), _JNIMethodIDCache.SwiftUnsafeBufferPointer.count, countBits$.getJNIValue(in: environment)) |
There was a problem hiding this comment.
same here, needs to account for Mutable
| } | ||
| return "" | ||
|
|
||
| case .bufferPointerIndirectReturn(let inner, let outArgumentName): |
There was a problem hiding this comment.
add an isMutable here and use it to determine the IDs below
| let javaBufferType: JavaType = isMutable ? .swiftUnsafeMutableBufferPointer : .swiftUnsafeBufferPointer | ||
| return NativeResult( | ||
| javaType: .void, | ||
| conversion: .bufferPointerIndirectReturn(.placeholder, outArgumentName: resultName + "Out"), |
There was a problem hiding this comment.
this must pass along the isMutable: isMutable
|
Found a few bits I'm concerned about, can you have a look please @Tatenda-k ? |
2 participants
Added support for
case .unsafePointer, .unsafeMutablePointer,case .unsafePointer, .unsafeMutablePointer, andcase .unsafeBufferPointer, .unsafeMutableBufferPointerfor FFM and JNI.#840