From c889e29d3ee0b5d8319c4fe525f8efcaab05ac84 Mon Sep 17 00:00:00 2001 From: tom doron Date: Mon, 14 Mar 2022 18:57:34 -0700 Subject: [PATCH] adopt sendable 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 --- Sources/Logging/LogHandler.swift | 2 +- Sources/Logging/Logging.swift | 31 ++++++++++++++---- Sources/Logging/Sendable.swift | 27 ++++++++++++++++ Tests/LoggingTests/LoggingTest.swift | 46 ++++++++++++++++++++++++++- Tests/LoggingTests/TestSendable.swift | 43 +++++++++++++++++++++++++ scripts/soundness.sh | 2 +- 6 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 Sources/Logging/Sendable.swift create mode 100644 Tests/LoggingTests/TestSendable.swift diff --git a/Sources/Logging/LogHandler.swift b/Sources/Logging/LogHandler.swift index 6b52b120..eaafde28 100644 --- a/Sources/Logging/LogHandler.swift +++ b/Sources/Logging/LogHandler.swift @@ -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. diff --git a/Sources/Logging/Logging.swift b/Sources/Logging/Logging.swift index 12a0e3ef..60901f01 100644 --- a/Sources/Logging/Logging.swift +++ b/Sources/Logging/Logging.swift @@ -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) @@ -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. @@ -35,7 +49,7 @@ import WASILibc /// /// logger.info("Hello World!") /// -public struct Logger { +public struct Logger: _SwiftLogSendable { @usableFromInline var handler: LogHandler @@ -707,7 +721,7 @@ 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`, @@ -715,8 +729,11 @@ extension Logger { 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 @@ -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 @@ -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 @@ -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 @@ -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 } diff --git a/Sources/Logging/Sendable.swift b/Sources/Logging/Sendable.swift new file mode 100644 index 00000000..74c5e463 --- /dev/null +++ b/Sources/Logging/Sendable.swift @@ -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 diff --git a/Tests/LoggingTests/LoggingTest.swift b/Tests/LoggingTests/LoggingTest.swift index 3400c8ef..50c157f0 100644 --- a/Tests/LoggingTests/LoggingTest.swift +++ b/Tests/LoggingTests/LoggingTest.swift @@ -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? @@ -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) @@ -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]() @@ -682,6 +725,7 @@ class LoggingTest: XCTestCase { self.interceptedText = (self.interceptedText ?? "") + string } } + #endif func testStreamLogHandlerWritesToAStream() { let interceptStream = InterceptStream() diff --git a/Tests/LoggingTests/TestSendable.swift b/Tests/LoggingTests/TestSendable.swift new file mode 100644 index 00000000..4d92ee27 --- /dev/null +++ b/Tests/LoggingTests/TestSendable.swift @@ -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 +} diff --git a/scripts/soundness.sh b/scripts/soundness.sh index 64fb7d14..f2a00dc4 100755 --- a/scripts/soundness.sh +++ b/scripts/soundness.sh @@ -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... "