-
Notifications
You must be signed in to change notification settings - Fork 34
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
Fix dependency and a compile failure. #105
base: master
Are you sure you want to change the base?
Conversation
Without this, Swift-Kuery fails to build with recent Swift compilers due to an ambiguity bug.
…ed optionals. The cleanup code in withUnsafeBufferPointers would not build, because the values array is an array of optionals, and the free function was expecting a non-optional.
Kudos, SonarCloud Quality Gate passed! |
Not sure if I need to do something to get this looked at. Don't seem to be able to request a review. Probably permissions. Can someone take a look? The framework doesn't currently compile, and these changes fix that. (/cc @mbarnach) |
@@ -23,7 +23,7 @@ internal struct PostgreSQLParameterSet { | |||
/// - Throws: QueryError.unsupported if parameter cannot be converted to data | |||
internal func withUnsafeBufferPointers(_ body: @escaping (UnsafePointers) -> Void) throws { | |||
let (values, lengths, formats) = try parameterData() | |||
defer { values.forEach({ free($0) }) } | |||
defer { values.compactMap({ $0 }).forEach({ free($0) }) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a problem that the compactMap is solving?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. values
is an array of optionals, and the free
function expects a non-optional (at least in the latest compiler/library versions). This code would not compile with my Swift 5.5 toolchain without replacing forEach with compactMap, to remove the optionality.
@drewmccormack Thanks for the PR. There's a few other changes I wanted to include (ensuring CI builds are running correctly and updating links). If it's okay with you, I've group them into a single PR: #106 |
No problem. Thanks for taking it up. |
Was not compiling with recent Swift compilers. Fixed an unwrapping issue, and updated Swift-Kuery dependency.