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.

Sign up for GitHub

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

Address a number of warnings on Windows #184

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Address a number of warnings on Windows #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
96b4c6b
SWBCore: re-iterate `@unchecked Sendable` conformance
compnerd Feb 17, 2025
2c5ca0c
Tests: remove unnecessary `#equire`s
compnerd Feb 17, 2025
7371d7a
SWBUtil: use `FileManager` for file removal
compnerd Feb 17, 2025
80dedf0
SWBUtil: adjust `strerror` usage for Windows
compnerd Feb 17, 2025
a8666d5
SWBUtilPerfTests: remove extraneous `#expect`
compnerd Feb 17, 2025
ca5a0e1
SWBSpecificationsCompiler: silence unused value warning
compnerd Feb 17, 2025
7fb020e
SWBCore: add a `strdup` wrapper
compnerd Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Sources/SWBCore/LibclangVendored/CStringArray.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@

import Foundation

#if os(Windows)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This brings back memories :)

private func strdup(_ s: UnsafePointer<CChar>) -> UnsafeMutablePointer<CChar> {
return _strdup(s)
}
#endif

/// `CStringArray` represents a C null-terminated array of pointers to C strings.
///
/// The lifetime of the C strings will correspond to the lifetime of the `CStringArray`
Expand Down
2 changes: 1 addition & 1 deletion Sources/SWBCore/ProjectModel/Reference.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ public final class VariantGroup: GroupTreeReference, BuildFileRepresentable, @un


/// A ProductReference represents the product of a StandardTarget object. It acts as a placeholder so that product can be represented in other targets, but it contains no meaningful information itself; rather, it vends information about itself by querying its target for that information. A ProductReference object is not part of a product's group tree and has no parent property; rather, it is owned by and has an unowned backpointer to its target.
public final class ProductReference: Reference, BuildFileRepresentable
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't safe yet, I need to refactor how it initializes the public unowned var target: Target? backreference before it's safe to apply @unchecked Sendable

public final class ProductReference: Reference, BuildFileRepresentable, @unchecked Sendable
{
/// The name of this reference - primarily for debugging purposes.
public let name: String
Expand Down
2 changes: 1 addition & 1 deletion Sources/SWBSpecificationsCompiler/Compiler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct Main {
}
var inputData = try Data(contentsOf: URL(fileURLWithPath: inputFile))
unifdef(&inputData)
try PropertyListSerialization.propertyList(from: inputData, options: [], format: nil)
_ = try PropertyListSerialization.propertyList(from: inputData, options: [], format: nil)
inputData.removeAll(where: { $0 == Character("\r").asciiValue }) // normalize newlines for Windows
try inputData.write(to: URL(fileURLWithPath: outputFile), options: .atomic)
}
Expand Down
4 changes: 4 additions & 0 deletions Sources/SWBUtil/FSProxy.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,9 +562,13 @@ class LocalFS: FSProxy, @unchecked Sendable {
}

func remove(_ path: Path) throws {
#if os(Windows)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know that there's any value in this being platform-conditional. I think this is only using unlink in the first place out of some early attempt to avoid Foundation for whatever reason at the time which is long since irrelevant. And we're using FileManager in the implementation of most of these FSProxy these functions anyways, so we may as well get to benefit from Foundation's work.

try fileManager.removeItem(atPath: path.str)
#else
guard unlink(path.str) == 0 else {
throw POSIXError(errno, context: "unlink", path.str)
}
#endif
}

func removeDirectory(_ path: Path) throws {
Expand Down
31 changes: 20 additions & 11 deletions Sources/SWBUtil/POSIX.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,22 @@ import SWBLibc
public import protocol Foundation.LocalizedError

#if os(Windows)
#if canImport(System)
import System
#else
import SystemPackage
#endif
public import Foundation

private func strerror(_ code: CInt) -> String {
withUnsafeTemporaryAllocation(of: CChar.self, capacity: 95) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

95?

Copy link
Member Author

Choose a reason for hiding this comment

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

94 + 1; the strerror message is limited to 94 characters.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, could you encode that in a comment (with a link to docs?) so it doesn't look like an arbitrary magic to future readers?

guard strerror_s($0.baseAddress, $0.count, code) == 0 else {
return "unknown error"
}
return String(cString: $0.baseAddress!)
}
}
#endif

public enum POSIX: Sendable {
public static func getenv(_ name: String) throws -> String? {
#if os(Windows)
try name.withCString(encodedAs: CInterop.PlatformUnicodeEncoding.self) { wName in
try name.withCString(encodedAs: UTF16.self) { wName in
let dwLength: DWORD = GetEnvironmentVariableW(wName, nil, 0)
if dwLength == 0 {
if GetLastError() == ERROR_ENVVAR_NOT_FOUND {
Expand All @@ -36,7 +41,7 @@ public enum POSIX: Sendable {
return try withUnsafeTemporaryAllocation(of: WCHAR.self, capacity: Int(dwLength)) {
switch GetEnvironmentVariableW(wName, $0.baseAddress!, DWORD($0.count)) {
case dwLength - 1:
return String(decodingCString: $0.baseAddress!, as: CInterop.PlatformUnicodeEncoding.self)
return String(decodingCString: $0.baseAddress!, as: UTF16.self)
case 0 where GetLastError() == ERROR_ENVVAR_NOT_FOUND:
return nil
default:
Expand All @@ -54,13 +59,13 @@ public enum POSIX: Sendable {
let valueString = String(cString: value)
#if os(Windows)
if overwrite == 0 {
if nameString.withCString(encodedAs: CInterop.PlatformUnicodeEncoding.self, { GetEnvironmentVariableW($0, nil, 0) }) == 0 && GetLastError() != ERROR_ENVVAR_NOT_FOUND {
if nameString.withCString(encodedAs: UTF16.self, { GetEnvironmentVariableW($0, nil, 0) }) == 0 && GetLastError() != ERROR_ENVVAR_NOT_FOUND {
throw POSIXError(errno, context: "GetEnvironmentVariableW", nameString)
}
return
}
guard nameString.withCString(encodedAs: CInterop.PlatformUnicodeEncoding.self, { nameWString in
valueString.withCString(encodedAs: CInterop.PlatformUnicodeEncoding.self, { valueWString in
guard nameString.withCString(encodedAs: UTF16.self, { nameWString in
valueString.withCString(encodedAs: UTF16.self, { valueWString in
SetEnvironmentVariableW(nameWString, valueWString)
})
}) else {
Expand All @@ -77,7 +82,7 @@ public enum POSIX: Sendable {
public static func unsetenv(_ name: UnsafePointer<CChar>) throws {
let nameString = String(cString: name)
#if os(Windows)
guard nameString.withCString(encodedAs: CInterop.PlatformUnicodeEncoding.self, { SetEnvironmentVariableW($0, nil) }) else {
guard nameString.withCString(encodedAs: UTF16.self, { SetEnvironmentVariableW($0, nil) }) else {
throw POSIXError(errno, context: "SetEnvironmentVariableW", nameString)
}
#else
Expand Down Expand Up @@ -105,7 +110,11 @@ public struct POSIXError: Error, LocalizedError, CustomStringConvertible, Equata
}

public var description: String {
#if os(Windows)
let end = "\(strerror(code)) (\(code))"
#else
let end = "\(String(cString: strerror(code))) (\(code))"
#endif
if let context {
return "\(context)(\(arguments.joined(separator: ", "))): \(end)"
}
Expand Down
26 changes: 13 additions & 13 deletions Tests/SWBCoreTests/PIFLoadingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,14 +683,14 @@ private final class ProjectModelItemClass: ProjectModelItem {
#expect(fileGroup.children.count == 2)

// Examine its children
if let fileRef = try? #require(fileGroup.children.first as? FileReference) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We keep having to flip these #requires back and forth due to compiler bugs, I don't know that there's a way to avoid the warnings on every platform

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it a platform thing or a compiler version thing? I'm building with a relatively recent compiler.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As I understand it it's a compiler version thing: swiftlang/swift#79202

if let fileRef = fileGroup.children.first as? FileReference {
#expect(fileRef.guid == "first-fileReference-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "ClassOne.m")
#expect(fileRef.fileTypeIdentifier == "sourcecode.c.objc")
#expect(fileRef.regionVariantName == nil)
}
if let fileRef = try? #require(fileGroup.children[1] as? FileReference) {
if let fileRef = fileGroup.children[1] as? FileReference {
#expect(fileRef.guid == "second-fileReference-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "ClassOne.h")
Expand Down Expand Up @@ -740,14 +740,14 @@ private final class ProjectModelItemClass: ProjectModelItem {
#expect(versionGroup.children.count == 2)

// Examine its children
if let fileRef = try? #require(versionGroup.children[0] as? FileReference) {
if let fileRef = versionGroup.children[0] as? FileReference {
#expect(fileRef.guid == "first-versionedFile-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "CoreData-1.xcdatamodel")
#expect(fileRef.fileTypeIdentifier == "wrapper.xcdatamodel")
#expect(fileRef.regionVariantName == nil)
}
if let fileRef = try? #require(versionGroup.children[1] as? FileReference) {
if let fileRef = versionGroup.children[1] as? FileReference {
#expect(fileRef.guid == "second-versionedFile-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "CoreData-2.xcdatamodel")
Expand Down Expand Up @@ -821,21 +821,21 @@ private final class ProjectModelItemClass: ProjectModelItem {
#expect(variantGroup.name == "Thingy.xib")

// Examine its children, the xib and its localized strings files
if let fileRef = try? #require(variantGroup.children[0] as? FileReference) {
if let fileRef = variantGroup.children[0] as? FileReference {
#expect(fileRef.guid == "xib-fileReference-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "Thingy.xib")
#expect(fileRef.fileTypeIdentifier == "file.xib")
#expect(fileRef.regionVariantName == nil)
}
if let fileRef = try? #require(variantGroup.children[1] as? FileReference) {
if let fileRef = variantGroup.children[1] as? FileReference {
#expect(fileRef.guid == "fr-strings-fileReference-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "Thingy.strings")
#expect(fileRef.fileTypeIdentifier == "text.plist.strings")
#expect(fileRef.regionVariantName == "fr")
}
if let fileRef = try? #require(variantGroup.children[2] as? FileReference) {
if let fileRef = variantGroup.children[2] as? FileReference {
#expect(fileRef.guid == "de-strings-fileReference-guid")
#expect(fileRef.sourceTree == SourceTree.groupRelative)
#expect(fileRef.path.stringRep == "Thingy.strings")
Expand Down Expand Up @@ -941,7 +941,7 @@ private final class ProjectModelItemClass: ProjectModelItem {
]

// Convert the test data into a property list, then read the build phase from it.
if let buildPhase = try? #require(BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? SourcesBuildPhase) {
if let buildPhase = try? BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? SourcesBuildPhase {
// Examine the build phase.
#expect(buildPhase.buildFiles.count == 1)
}
Expand All @@ -962,7 +962,7 @@ private final class ProjectModelItemClass: ProjectModelItem {
]

// Convert the test data into a property list, then read the build phase from it.
if let buildPhase = try? #require(BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? HeadersBuildPhase) {
if let buildPhase = try? BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? HeadersBuildPhase {
// Examine the build phase.
#expect(buildPhase.buildFiles.count == 1)
}
Expand All @@ -983,7 +983,7 @@ private final class ProjectModelItemClass: ProjectModelItem {
]

// Convert the test data into a property list, then read the build phase from it.
if let buildPhase = try? #require(BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? ResourcesBuildPhase) {
if let buildPhase = try? BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? ResourcesBuildPhase {
// Examine the build phase.
#expect(buildPhase.buildFiles.count == 1)
}
Expand All @@ -1007,7 +1007,7 @@ private final class ProjectModelItemClass: ProjectModelItem {
]

// Convert the test data into a property list, then read the build phase from it.
if let buildPhase = try? #require(BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? CopyFilesBuildPhase) {
if let buildPhase = try? BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? CopyFilesBuildPhase {
// Examine the build phase.
#expect(buildPhase.destinationSubfolder.stringRep == "Resources")
#expect(buildPhase.destinationSubpath.stringRep == "Subpath")
Expand Down Expand Up @@ -1036,7 +1036,7 @@ private final class ProjectModelItemClass: ProjectModelItem {
]

// Convert the test data into a property list, then read the build phase from it.
if let buildPhase = try? #require(BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? ShellScriptBuildPhase) {
if let buildPhase = try? BuildPhase.parsePIFDictAsBuildPhase(buildPhasePIF, pifLoader: pifLoader) as? ShellScriptBuildPhase {
// Examine the build phase.
#expect(buildPhase.guid == "some-shellScriptBuildPhase-guid")
#expect(buildPhase.name == "A Shell Script Phase")
Expand Down Expand Up @@ -1353,7 +1353,7 @@ private final class ProjectModelItemClass: ProjectModelItem {

// Because of the way reference resolution of a BuildFile.BuildableItem works, we don't have a context to resolve the build file's references to real references, so all we can do is check that the GUID is what we expect.
func checkBuildFileRef(of buildPhase: BuildPhaseWithBuildFiles, fileRef: FileReference) throws {
guard let buildFileRef = try? #require(buildPhase.buildFiles.first) else {
guard let buildFileRef = buildPhase.buildFiles.first else {
return
}
guard case let .reference(buildFileRefGuid) = buildFileRef.buildableItem else {
Expand Down
19 changes: 8 additions & 11 deletions Tests/SWBUtilPerfTests/MsgPackSerializationPerfTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ fileprivate struct MsgPackSerializationPerfTests: PerfTests {
let iterations = 100000

await measure {
for _ in 1...iterations
{
#expect(self.serializeScalarData() != nil)
for _ in 1...iterations {
_ = self.serializeScalarData()
}
}
}
Expand Down Expand Up @@ -91,7 +90,7 @@ fileprivate struct MsgPackSerializationPerfTests: PerfTests {

await measure {
for _ in 1...iterations {
#expect(self.serializeStringData() != nil)
_ = self.serializeStringData()
}
}
}
Expand Down Expand Up @@ -131,9 +130,8 @@ fileprivate struct MsgPackSerializationPerfTests: PerfTests {
let iterations = 100000

await measure {
for _ in 1...iterations
{
#expect(self.serializeArrayData() != nil)
for _ in 1...iterations {
_ = self.serializeArrayData()
}
}
}
Expand Down Expand Up @@ -171,7 +169,7 @@ fileprivate struct MsgPackSerializationPerfTests: PerfTests {

await measure {
for _ in 1...iterations {
#expect(self.serializeDictionaryData() != nil)
_ = self.serializeDictionaryData()
}
}
}
Expand Down Expand Up @@ -218,9 +216,8 @@ fileprivate struct MsgPackSerializationPerfTests: PerfTests {

await measure
{
for _ in 1...iterations
{
#expect(self.serializeCustomElementData(elements) != nil)
for _ in 1...iterations {
_ = self.serializeCustomElementData(elements)
}
}
}
Expand Down