Skip to content

Commit

Permalink
adopt sendable
Browse files Browse the repository at this point in the history
motivation: adjust to swift 5.6

changes:
* define sendable shims for protocols and structs that may be used in async context
* adjust tests
* add a test to make sure no warning are emittted
  • Loading branch information
tomerd committed Mar 15, 2022
1 parent dc6dd5c commit c889e29
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Sources/Logging/LogHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
/// Please note that the above `LogHandler` will still pass the 'log level is a value' test above it iff the global log
/// level has not been overridden. And most importantly it passes the requirement listed above: A change to the log
/// level on one `Logger` should not affect the log level of another `Logger` variable.
public protocol LogHandler {
public protocol LogHandler: _SwiftLogSendableLogHandler {
/// This method is called when a `LogHandler` must emit a log message. There is no need for the `LogHandler` to
/// check if the `level` is above or below the configured `logLevel` as `Logger` already performed this check and
/// determined that a message should be logged.
Expand Down
31 changes: 24 additions & 7 deletions Sources/Logging/Logging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@
//
//===----------------------------------------------------------------------===//

#if compiler(>=5.6)
#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
@preconcurrency import Darwin
#elseif os(Windows)
@preconcurrency import CRT
#elseif canImport(Glibc)
@preconcurrency import Glibc
#elseif canImport(WASILibc)
@preconcurrency import WASILibc
#else
#error("Unsupported runtime")
#endif
#else
#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
import Darwin
#elseif os(Windows)
Expand All @@ -23,6 +36,7 @@ import WASILibc
#else
#error("Unsupported runtime")
#endif
#endif

/// A `Logger` is the central type in `SwiftLog`. Its central function is to emit log messages using one of the methods
/// corresponding to a log level.
Expand All @@ -35,7 +49,7 @@ import WASILibc
///
/// logger.info("Hello World!")
///
public struct Logger {
public struct Logger: _SwiftLogSendable {
@usableFromInline
var handler: LogHandler

Expand Down Expand Up @@ -707,16 +721,19 @@ extension Logger {
/// over `..., metadata: ["colors": .array([.string("\(user.topColor)"), .string("\(user.secondColor)")])`
/// - prefer `logger.info("nested info", metadata: ["nested": ["fave-numbers": ["\(1)", "\(2)", "\(3)"], "foo": "bar"]])`
/// over `..., metadata: ["nested": .dictionary(["fave-numbers": ...])])`
public enum MetadataValue {
public enum MetadataValue: _SwiftLogSendable {
/// A metadata value which is a `String`.
///
/// Because `MetadataValue` implements `ExpressibleByStringInterpolation`, and `ExpressibleByStringLiteral`,
/// you don't need to type `.string(someType.description)` you can use the string interpolation `"\(someType)"`.
case string(String)

/// A metadata value which is some `CustomStringConvertible`.
#if compiler(>=5.6)
case stringConvertible(CustomStringConvertible & Sendable)
#else
case stringConvertible(CustomStringConvertible)

#endif
/// A metadata value which is a dictionary from `String` to `Logger.MetadataValue`.
///
/// Because `MetadataValue` implements `ExpressibleByDictionaryLiteral`, you don't need to type
Expand All @@ -734,7 +751,7 @@ extension Logger {
///
/// Log levels are ordered by their severity, with `.trace` being the least severe and
/// `.critical` being the most severe.
public enum Level: String, Codable, CaseIterable {
public enum Level: String, Codable, CaseIterable, _SwiftLogSendable {
/// Appropriate for messages that contain information normally of use only when
/// tracing the execution of a program.
case trace
Expand Down Expand Up @@ -857,7 +874,7 @@ extension Logger {
///
/// logger.info("Hello \(world)")
///
public struct Message: ExpressibleByStringLiteral, Equatable, CustomStringConvertible, ExpressibleByStringInterpolation {
public struct Message: ExpressibleByStringLiteral, Equatable, CustomStringConvertible, ExpressibleByStringInterpolation, _SwiftLogSendable {
public typealias StringLiteralType = String

private var value: String
Expand Down Expand Up @@ -1084,7 +1101,7 @@ public struct StreamLogHandler: LogHandler {
return StreamLogHandler(label: label, stream: StdioOutputStream.stderr)
}

private let stream: TextOutputStream
private let stream: TextOutputStream & _SwiftLogSendable
private let label: String

public var logLevel: Logger.Level = .info
Expand All @@ -1106,7 +1123,7 @@ public struct StreamLogHandler: LogHandler {
}

// internal for testing only
internal init(label: String, stream: TextOutputStream) {
internal init(label: String, stream: TextOutputStream & _SwiftLogSendable) {
self.label = label
self.stream = stream
}
Expand Down
27 changes: 27 additions & 0 deletions Sources/Logging/Sendable.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Logging API open source project
//
// Copyright (c) 2022 Apple Inc. and the Swift Logging API project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of Swift Logging API project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

// Sendable support helpers

#if compiler(>=5.6)
@preconcurrency public protocol _SwiftLogSendableLogHandler: Sendable {}
#else
public protocol _SwiftLogSendableLogHandler {}
#endif

#if compiler(>=5.6)
public typealias _SwiftLogSendable = Sendable
#else
public typealias _SwiftLogSendable = Any
#endif
46 changes: 45 additions & 1 deletion Tests/LoggingTests/LoggingTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,9 @@ class LoggingTest: XCTestCase {

// Example of custom "box" which may be used to implement "render at most once" semantics
// Not thread-safe, thus should not be shared across threads.
internal class LazyMetadataBox: CustomStringConvertible {
// @unchecked Sendable since not thread-safe by definition
#if compiler(>=5.6)
internal final class LazyMetadataBox: CustomStringConvertible, @unchecked Sendable {
private var makeValue: (() -> String)?
private var _value: String?

Expand All @@ -287,6 +289,33 @@ class LoggingTest: XCTestCase {
}
}

#else
internal final class LazyMetadataBox: CustomStringConvertible {
private var makeValue: (() -> String)?
private var _value: String?

public init(_ makeValue: @escaping () -> String) {
self.makeValue = makeValue
}

/// This allows caching a value in case it is accessed via an by name subscript,
// rather than as part of rendering all metadata that a LoggingContext was carrying
public var value: String {
if let f = self.makeValue {
self._value = f()
self.makeValue = nil
}

assert(self._value != nil, "_value MUST NOT be nil once `lazyValue` has run.")
return self._value!
}

public var description: String {
return "\(self.value)"
}
}
#endif

func testStringConvertibleMetadata() {
let testLogging = TestLogging()
LoggingSystem.bootstrapInternal(testLogging.make)
Expand Down Expand Up @@ -672,6 +701,20 @@ class LoggingTest: XCTestCase {
XCTAssertLessThan(Logger.Level.error, Logger.Level.critical)
}

#if compiler(>=5.6)
// @unchecked Sendable since non-thread safe by design
final class InterceptStream: TextOutputStream, @unchecked Sendable {
var interceptedText: String?
var strings = [String]()

func write(_ string: String) {
// This is a test implementation, a real implementation would include locking
self.strings.append(string)
self.interceptedText = (self.interceptedText ?? "") + string
}
}

#else
final class InterceptStream: TextOutputStream {
var interceptedText: String?
var strings = [String]()
Expand All @@ -682,6 +725,7 @@ class LoggingTest: XCTestCase {
self.interceptedText = (self.interceptedText ?? "") + string
}
}
#endif

func testStreamLogHandlerWritesToAStream() {
let interceptStream = InterceptStream()
Expand Down
43 changes: 43 additions & 0 deletions Tests/LoggingTests/TestSendable.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Logging API open source project
//
// Copyright (c) 2018-2019 Apple Inc. and the Swift Logging API project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of Swift Logging API project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import Dispatch
@testable import Logging
import XCTest

class SendableTest: XCTestCase {
#if compiler(>=5.6)
func testSendableLogger() async {
let testLogging = TestLogging()
LoggingSystem.bootstrapInternal(testLogging.make)

let logger = Logger(label: "test")
let message1 = Logger.Message(stringLiteral: "critical 1")
let message2 = Logger.Message(stringLiteral: "critical 2")
let metadata: Logger.Metadata = ["key": "value"]

let task = Task.detached {
logger.info("info")
logger.critical(message1)
logger.critical(message2)
logger.warning(.init(stringLiteral: "warning"), metadata: metadata)
}

await task.value
testLogging.history.assertExist(level: .info, message: "info", metadata: [:])
testLogging.history.assertExist(level: .critical, message: "critical 1", metadata: [:])
testLogging.history.assertExist(level: .critical, message: "critical 2", metadata: [:])
testLogging.history.assertExist(level: .warning, message: "warning", metadata: metadata)
}
#endif
}
2 changes: 1 addition & 1 deletion scripts/soundness.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

function replace_acceptable_years() {
# this needs to replace all acceptable forms with 'YEARS'
sed -e 's/20[12][78901]-20[12][8901]/YEARS/' -e 's/20[12][8901]/YEARS/'
sed -e 's/20[12][789012]-20[12][89012]/YEARS/' -e 's/20[12][89012]/YEARS/'
}

printf "=> Checking linux tests... "
Expand Down

0 comments on commit c889e29

Please sign in to comment.