Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

drewmccormack
Copy link

Was not compiling with recent Swift compilers. Fixed an unwrapping issue, and updated Swift-Kuery dependency.

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.
@CLAassistant
Copy link

CLAassistant commented Dec 8, 2021

CLA assistant check
All committers have signed the CLA.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 8, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@drewmccormack
Copy link
Author

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) }) }
Copy link
Contributor

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?

Copy link
Author

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.

@dannys42
Copy link
Contributor

@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

@drewmccormack
Copy link
Author

No problem. Thanks for taking it up.

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