Skip to content

Commit 482c82c

Browse files
ravikandhadairavikandhadai
authored
Merge pull request swiftlang#30060 from ravikandhadai/oslog-dynamic-precision
[os log][stdlib/private] Enable precision and alignment values to be dynamic.
2 parents 573e552 + 7c9ddca commit 482c82c

File tree

8 files changed

+326
-37
lines changed

8 files changed

+326
-37
lines changed

‎lib/SIL/SILConstants.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ using namespace swift;
2020

2121
namespace swift {
2222
llvm::cl::opt<unsigned>
23-
ConstExprLimit("constexpr-limit", llvm::cl::init(2048),
23+
ConstExprLimit("constexpr-limit", llvm::cl::init(3072),
2424
llvm::cl::desc("Number of instructions interpreted in a"
2525
" constexpr function"));
2626
}

‎stdlib/private/OSLog/OSLogIntegerFormatting.swift

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// integer-valued interpolations passed to the os log APIs.
1515

1616
@frozen
17-
public struct OSLogIntegerFormatting: Equatable {
17+
public struct OSLogIntegerFormatting {
1818
/// The base to use for the string representation. `radix` must be at least 2
1919
/// and at most 36. The default is 10.
2020
public var radix: Int
@@ -32,7 +32,7 @@ public struct OSLogIntegerFormatting: Equatable {
3232

3333
/// Minimum number of digits to display. Numbers having fewer digits than
3434
/// minDigits will be displayed with leading zeros.
35-
public var minDigits: Int
35+
public var minDigits: (() -> Int)?
3636

3737
/// - Parameters:
3838
/// - radix: The base to use for the string representation. `radix` must be
@@ -49,12 +49,12 @@ public struct OSLogIntegerFormatting: Equatable {
4949
@_semantics("constant_evaluable")
5050
@inlinable
5151
@_optimize(none)
52-
public init(
52+
internal init(
5353
radix: Int = 10,
5454
explicitPositiveSign: Bool = false,
5555
includePrefix: Bool = false,
5656
uppercase: Bool = false,
57-
minDigits: Int = 1
57+
minDigits: (() -> Int)?
5858
) {
5959
precondition(radix >= 2 && radix <= 36)
6060

@@ -75,14 +75,29 @@ public struct OSLogIntegerFormatting: Equatable {
7575
@_optimize(none)
7676
public static func decimal(
7777
explicitPositiveSign: Bool = false,
78-
minDigits: Int = 1
78+
minDigits: @escaping @autoclosure () -> Int
7979
) -> OSLogIntegerFormatting {
8080
return OSLogIntegerFormatting(
8181
radix: 10,
8282
explicitPositiveSign: explicitPositiveSign,
8383
minDigits: minDigits)
8484
}
8585

86+
/// - Parameters:
87+
/// - explicitPositiveSign: Pass `true` to add a + sign to non-negative
88+
/// numbers. Default is `false`.
89+
@_semantics("constant_evaluable")
90+
@inlinable
91+
@_optimize(none)
92+
public static func decimal(
93+
explicitPositiveSign: Bool = false
94+
) -> OSLogIntegerFormatting {
95+
return OSLogIntegerFormatting(
96+
radix: 10,
97+
explicitPositiveSign: explicitPositiveSign,
98+
minDigits: nil)
99+
}
100+
86101
/// Default decimal format.
87102
@_semantics("constant_evaluable")
88103
@inlinable
@@ -106,7 +121,7 @@ public struct OSLogIntegerFormatting: Equatable {
106121
explicitPositiveSign: Bool = false,
107122
includePrefix: Bool = false,
108123
uppercase: Bool = false,
109-
minDigits: Int = 1
124+
minDigits: @escaping @autoclosure () -> Int
110125
) -> OSLogIntegerFormatting {
111126
return OSLogIntegerFormatting(
112127
radix: 16,
@@ -116,6 +131,30 @@ public struct OSLogIntegerFormatting: Equatable {
116131
minDigits: minDigits)
117132
}
118133

134+
/// - Parameters:
135+
/// - explicitPositiveSign: Pass `true` to add a + sign to non-negative
136+
/// numbers. Default is `false`.
137+
/// - includePrefix: Pass `true` to add a prefix: 0b, 0o, 0x to corresponding
138+
/// radices. Default is `false`.
139+
/// - uppercase: Pass `true` to use uppercase letters to represent numerals
140+
/// greater than 9, or `false` to use lowercase letters. The default is
141+
/// `false`.
142+
@_semantics("constant_evaluable")
143+
@inlinable
144+
@_optimize(none)
145+
public static func hex(
146+
explicitPositiveSign: Bool = false,
147+
includePrefix: Bool = false,
148+
uppercase: Bool = false
149+
) -> OSLogIntegerFormatting {
150+
return OSLogIntegerFormatting(
151+
radix: 16,
152+
explicitPositiveSign: explicitPositiveSign,
153+
includePrefix: includePrefix,
154+
uppercase: uppercase,
155+
minDigits: nil)
156+
}
157+
119158
/// Default hexadecimal format.
120159
@_semantics("constant_evaluable")
121160
@inlinable
@@ -139,7 +178,7 @@ public struct OSLogIntegerFormatting: Equatable {
139178
explicitPositiveSign: Bool = false,
140179
includePrefix: Bool = false,
141180
uppercase: Bool = false,
142-
minDigits: Int = 1
181+
minDigits: @autoclosure @escaping () -> Int
143182
) -> OSLogIntegerFormatting {
144183
OSLogIntegerFormatting(
145184
radix: 8,
@@ -149,6 +188,30 @@ public struct OSLogIntegerFormatting: Equatable {
149188
minDigits: minDigits)
150189
}
151190

191+
/// - Parameters:
192+
/// - explicitPositiveSign: Pass `true` to add a + sign to non-negative
193+
/// numbers. Default is `false`.
194+
/// - includePrefix: Pass `true` to add a prefix: 0b, 0o, 0x to corresponding
195+
/// radices. Default is `false`.
196+
/// - uppercase: Pass `true` to use uppercase letters to represent numerals
197+
/// greater than 9, or `false` to use lowercase letters. The default is
198+
/// `false`.
199+
@_semantics("constant_evaluable")
200+
@inlinable
201+
@_optimize(none)
202+
public static func octal(
203+
explicitPositiveSign: Bool = false,
204+
includePrefix: Bool = false,
205+
uppercase: Bool = false
206+
) -> OSLogIntegerFormatting {
207+
OSLogIntegerFormatting(
208+
radix: 8,
209+
explicitPositiveSign: explicitPositiveSign,
210+
includePrefix: includePrefix,
211+
uppercase: uppercase,
212+
minDigits: nil)
213+
}
214+
152215
/// Default octal format.
153216
@_semantics("constant_evaluable")
154217
@inlinable
@@ -294,16 +357,19 @@ extension OSLogIntegerFormatting {
294357
// In our case, we're already handling prefix ourselves; we choose not to
295358
// support this functionality. In our case, alignment always pads spaces (
296359
// to the left or right) until the minimum field width is met.
297-
if align.minimumColumnWidth > 0 {
298-
specification += align.minimumColumnWidth.description
360+
if let _ = align.minimumColumnWidth {
361+
// The alignment could be a dynamic value. Therefore, use a star here and pass it
362+
// as an additional argument.
363+
specification += "*"
299364
}
300365

301366
// 3. Precision
302367

303368
// Default precision for integers is 1, otherwise use the requested precision.
304-
if minDigits != 1 {
305-
specification += "."
306-
specification += minDigits.description
369+
// The precision could be a dynamic value. Therefore, use a star here and pass it
370+
// as an additional argument.
371+
if let _ = minDigits {
372+
specification += ".*"
307373
}
308374

309375
// 4. Length modifier

‎stdlib/private/OSLog/OSLogIntegerTypes.swift

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,19 @@ extension OSLogInterpolation {
102102
guard argumentCount < maxOSLogArgumentCount else { return }
103103
formatString +=
104104
format.formatSpecifier(for: T.self, align: align, privacy: privacy)
105-
addIntHeaders(privacy, sizeForEncoding(T.self))
105+
// If minimum column width is specified, append this value first. Note that the
106+
// format specifier would use a '*' for width e.g. %*d.
107+
if let minColumns = align.minimumColumnWidth {
108+
appendPrecisionArgument(minColumns)
109+
}
110+
111+
// If minimum number of digits (precision) is specified, append the precision before
112+
// the argument. Note that the format specifier would use a '*' for precision: %.*d.
113+
if let minDigits = format.minDigits {
114+
appendPrecisionArgument(minDigits)
115+
}
106116

117+
addIntHeaders(privacy, sizeForEncoding(T.self))
107118
arguments.append(number)
108119
argumentCount += 1
109120
}
@@ -131,6 +142,27 @@ extension OSLogInterpolation {
131142

132143
preamble = getUpdatedPreamble(privacy: privacy, isScalar: true)
133144
}
145+
146+
// Append argument indicating precision or width of a format specifier to the buffer.
147+
// These specify the value of the '*' in a format specifier like: %*.*ld.
148+
@_semantics("constant_evaluable")
149+
@inlinable
150+
@_optimize(none)
151+
internal mutating func appendPrecisionArgument(_ count: @escaping () -> Int) {
152+
// Note that we don't have to update the preamble here.
153+
let argumentHeader = getArgumentHeader(privacy: .auto, type: .count)
154+
arguments.append(argumentHeader)
155+
// Append number of bytes needed to serialize the argument.
156+
let byteCount = sizeForEncoding(CInt.self)
157+
arguments.append(UInt8(byteCount))
158+
// Increment total byte size by the number of bytes needed for this
159+
// argument, which is the sum of the byte size of the argument and
160+
// two bytes needed for the headers.
161+
totalBytesForSerializingArguments += 2 + byteCount
162+
// The count is expected to be a CInt.
163+
arguments.append({ CInt(count()) })
164+
argumentCount += 1
165+
}
134166
}
135167

136168
extension OSLogArguments {

‎stdlib/private/OSLog/OSLogStringAlignment.swift

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ public enum OSLogCollectionBound {
2323
public struct OSLogStringAlignment {
2424
/// Minimum number of characters to be displayed. If the value to be printed
2525
/// is shorter than this number, the result is padded with spaces. The value
26-
/// is not truncated even if the result is larger.
27-
public var minimumColumnWidth: Int
26+
/// is not truncated even if the result is larger.This value need not be a
27+
/// compile-time constant, and is therefore an autoclosure.
28+
public var minimumColumnWidth: (() -> Int)?
2829
/// This captures right/left alignment.
2930
public var anchor: OSLogCollectionBound
3031

@@ -36,8 +37,8 @@ public struct OSLogStringAlignment {
3637
@_semantics("constant_evaluable")
3738
@inlinable
3839
@_optimize(none)
39-
public init(
40-
minimumColumnWidth: Int = 0,
40+
internal init(
41+
minimumColumnWidth: (() -> Int)? = nil,
4142
anchor: OSLogCollectionBound = .end
4243
) {
4344
self.minimumColumnWidth = minimumColumnWidth
@@ -70,15 +71,19 @@ public struct OSLogStringAlignment {
7071
@_semantics("constant_evaluable")
7172
@inlinable
7273
@_optimize(none)
73-
public static func right(columns: Int = 0) -> OSLogStringAlignment {
74+
public static func right(
75+
columns: @escaping @autoclosure () -> Int
76+
) -> OSLogStringAlignment {
7477
OSLogStringAlignment(minimumColumnWidth: columns, anchor: .end)
7578
}
7679

7780
/// Left align and display at least`columns` characters.
7881
@_semantics("constant_evaluable")
7982
@inlinable
8083
@_optimize(none)
81-
public static func left(columns: Int = 0) -> OSLogStringAlignment {
84+
public static func left(
85+
columns: @escaping @autoclosure () -> Int
86+
) -> OSLogStringAlignment {
8287
OSLogStringAlignment(minimumColumnWidth: columns, anchor: .start)
8388
}
8489
}

‎stdlib/private/OSLog/OSLogStringTypes.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,14 @@ extension OSLogInterpolation {
4343
guard argumentCount < maxOSLogArgumentCount else { return }
4444

4545
formatString += getStringFormatSpecifier(align, privacy)
46-
addStringHeaders(privacy)
4746

47+
// If minimum column width is specified, append this value first. Note that the
48+
// format specifier would use a '*' for width e.g. %*s.
49+
if let minColumns = align.minimumColumnWidth {
50+
appendPrecisionArgument(minColumns)
51+
}
52+
53+
addStringHeaders(privacy)
4854
arguments.append(argumentString)
4955
argumentCount += 1
5056
}
@@ -94,8 +100,8 @@ extension OSLogInterpolation {
94100
if case .start = align.anchor {
95101
specifier += "-"
96102
}
97-
if align.minimumColumnWidth > 0 {
98-
specifier += align.minimumColumnWidth.description
103+
if let _ = align.minimumColumnWidth {
104+
specifier += "*"
99105
}
100106
specifier += "s"
101107
return specifier

‎test/SILOptimizer/OSLogConstantEvaluableTest.swift

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-silgen -primary-file %s -o %t/OSLogConstantEvaluableTest_silgen.sil
2+
// RUN: %target-swift-frontend -swift-version 5 -emit-silgen -primary-file %s -o %t/OSLogConstantEvaluableTest_silgen.sil
33
//
44
// Run the (mandatory) passes on which constant evaluator depends, and run the
55
// constant evaluator on the SIL produced after the dependent passes are run.
66
//
7-
// RUN: %target-sil-opt -silgen-cleanup -raw-sil-inst-lowering -allocbox-to-stack -mandatory-inlining -constexpr-limit 2048 -test-constant-evaluable-subset %t/OSLogConstantEvaluableTest_silgen.sil > %t/OSLogConstantEvaluableTest.sil 2> %t/error-output
7+
// RUN: %target-sil-opt -silgen-cleanup -raw-sil-inst-lowering -allocbox-to-stack -mandatory-inlining -constexpr-limit 3072 -test-constant-evaluable-subset %t/OSLogConstantEvaluableTest_silgen.sil > %t/OSLogConstantEvaluableTest.sil 2> %t/error-output
88
//
99
// RUN: %FileCheck %s < %t/error-output
1010
//
@@ -47,3 +47,12 @@ func intValueInterpolationTest() -> OSLogMessage {
4747
func stringValueInterpolationTest() -> OSLogMessage {
4848
return "A string value \("xyz")"
4949
}
50+
51+
@_semantics("test_driver")
52+
func intValueWithPrecisionTest() -> OSLogMessage {
53+
return
54+
"""
55+
An integer value \
56+
\(10, format: .decimal(minDigits: 25), align: .right(columns: 10))
57+
"""
58+
}

‎test/SILOptimizer/OSLogPrototypeFullOptTest.swift

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,5 +211,72 @@ func testFloatInterpolation(h: Logger, doubleValue: Double) {
211211
// CHECK-NEXT: ret void
212212
}
213213

214+
// This test checks that the precision and alignment are optimally "stored" into the
215+
// byte buffer at the right positions.
216+
// CHECK-LABEL: define hidden swiftcc void @"${{.*}}testDynamicPrecisionAndAlignment
217+
@available(OSX 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *)
218+
func testDynamicPrecisionAndAlignment(h: Logger) {
219+
h.debug(
220+
"""
221+
Maximum Int64 value: \
222+
\(Int32.max, format: .decimal(minDigits: 10), align: .left(columns: 5))
223+
""")
224+
// CHECK: entry:
225+
// CHECK-NEXT: tail call swiftcc %TSo9OS_os_logC* @"$s14OSLogPrototype6LoggerV9logObjectSo06OS_os_D0Cvg"
226+
// CHECK-NEXT: [[LOGLEVEL:%.+]] = tail call swiftcc i8 @"$sSo13os_log_type_ta0A0E5debugABvgZ"()
227+
// CHECK-NEXT: [[LOGOBJ:%.+]] = bitcast %TSo9OS_os_logC*
228+
// CHECK-NEXT: tail call zeroext i1 @os_log_type_enabled
229+
// CHECK-NEXT: br i1 {{%.*}}, label %[[ENABLED:[0-9]+]], label %[[NOT_ENABLED:[0-9]+]]
230+
231+
// CHECK: [[ENABLED]]:
232+
//
233+
// Header bytes.
234+
//
235+
// CHECK-NEXT: [[BUFFER:%.+]] = tail call noalias i8* @swift_slowAlloc(i{{.*}} 20
236+
// CHECK-NEXT: store i8 0, i8* [[BUFFER]], align 1
237+
// CHECK-NEXT: [[OFFSET1:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 1
238+
// CHECK-NEXT: store i8 3, i8* [[OFFSET1]], align 1
239+
//
240+
// First argument bytes.
241+
//
242+
// CHECK-NEXT: [[OFFSET2:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 2
243+
// CHECK-NEXT: store i8 16, i8* [[OFFSET2]], align 1
244+
// CHECK-NEXT: [[OFFSET3:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 3
245+
// CHECK-NEXT: store i8 4, i8* [[OFFSET3]], align 1
246+
// CHECK-NEXT: [[OFFSET4:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 4
247+
// CHECK-NEXT: [[BITCASTED:%.+]] = bitcast i8* [[OFFSET4]] to i32*
248+
// CHECK-NEXT: store i32 5, i32* [[BITCASTED]], align 1
249+
//
250+
// Second argument
251+
//
252+
// CHECK-NEXT: [[OFFSET12:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 8
253+
// CHECK-NEXT: store i8 16, i8* [[OFFSET12]], align 1
254+
// CHECK-NEXT: [[OFFSET13:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 9
255+
// CHECK-NEXT: store i8 4, i8* [[OFFSET13]], align 1
256+
// CHECK-NEXT: [[OFFSET14:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 10
257+
// CHECK-NEXT: [[BITCASTED2:%.+]] = bitcast i8* [[OFFSET14]] to i32*
258+
// CHECK-NEXT: store i32 10, i32* [[BITCASTED2]], align 1
259+
//
260+
// Third argument
261+
//
262+
// CHECK-NEXT: [[OFFSET22:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 14
263+
// CHECK-NEXT: store i8 0, i8* [[OFFSET22]], align 1
264+
// CHECK-NEXT: [[OFFSET23:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 15
265+
// CHECK-NEXT: store i8 4, i8* [[OFFSET23]], align 1
266+
// CHECK-NEXT: [[OFFSET24:%.+]] = getelementptr inbounds i8, i8* [[BUFFER]], i{{.*}} 16
267+
// CHECK-NEXT: [[BITCASTED3:%.+]] = bitcast i8* [[OFFSET24]] to i32*
268+
// CHECK-NEXT: store i32 2147483647, i32* [[BITCASTED3]], align 1
269+
//
270+
// os_log_impl call.
271+
// CHECK-NEXT: tail call void @_os_log_impl({{.*}}, {{.*}} [[LOGOBJ]], i8 zeroext [[LOGLEVEL]], i8* getelementptr inbounds ([28 x i8], [28 x i8]* @{{.*}}, i{{.*}} 0, i{{.*}} 0), i8* {{(nonnull )?}}[[BUFFER]], i32 20)
272+
// CHECK-NEXT: tail call void @swift_slowDealloc(i8* {{(nonnull )?}}[[BUFFER]]
273+
// CHECK-NEXT: br label %[[NOT_ENABLED]]
274+
275+
// CHECK: [[NOT_ENABLED]]:
276+
// CHECK-NEXT: bitcast
277+
// CHECK-NEXT: tail call void @llvm.objc.release
278+
// CHECK-NEXT: ret void
279+
}
280+
214281
// TODO: add test for String. It is more complicated due to more complex logic
215282
// in string serialization.

0 commit comments

Comments
 (0)