Skip to content

Commit a62c87f

Browse files
LucianoPAlmeidalorentey
authored andcommitted
[stdlib] Addressing Fixme comment atomic stdlib unit test flags (swiftlang#23198)
* Addressing FIXME comment making StdlibUnittest flags atomic * Commenting store * Fix wrong change * Add comment * Addressing the PR comment suggestions. * Remove public wrong place. * Reverting comment iff * Update stdlib/private/StdlibUnittest/StdlibUnittest.swift Co-Authored-By: LucianoPAlmeida <[email protected]> * File private vars
1 parent d909dd1 commit a62c87f

File tree

1 file changed

+36
-20
lines changed

1 file changed

+36
-20
lines changed

stdlib/private/StdlibUnittest/StdlibUnittest.swift

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,22 @@ public struct SourceLocStack {
103103
}
104104
}
105105

106+
fileprivate struct AtomicBool {
107+
108+
private var _value: _stdlib_AtomicInt
109+
110+
init(_ b: Bool) { self._value = _stdlib_AtomicInt(b ? 1 : 0) }
111+
112+
func store(_ b: Bool) { _value.store(b ? 1 : 0) }
113+
114+
func load() -> Bool { return _value.load() != 0 }
115+
116+
@discardableResult
117+
func orAndFetch(_ b: Bool) -> Bool {
118+
return _value.orAndFetch(b ? 1 : 0) != 0
119+
}
120+
}
121+
106122
func _printStackTrace(_ stackTrace: SourceLocStack?) {
107123
guard let s = stackTrace, !s.locs.isEmpty else { return }
108124
print("stacktrace:")
@@ -112,10 +128,8 @@ func _printStackTrace(_ stackTrace: SourceLocStack?) {
112128
}
113129
}
114130

115-
// FIXME: these variables should be atomic, since multiple threads can call
116-
// `expect*()` functions.
117-
var _anyExpectFailed = false
118-
var _seenExpectCrash = false
131+
fileprivate var _anyExpectFailed = AtomicBool(false)
132+
fileprivate var _seenExpectCrash = AtomicBool(false)
119133

120134
/// Run `body` and expect a failure to happen.
121135
///
@@ -125,16 +139,16 @@ public func expectFailure(
125139
stackTrace: SourceLocStack = SourceLocStack(),
126140
showFrame: Bool = true,
127141
file: String = #file, line: UInt = #line, invoking body: () -> Void) {
128-
let startAnyExpectFailed = _anyExpectFailed
129-
_anyExpectFailed = false
142+
let startAnyExpectFailed = _anyExpectFailed.load()
143+
_anyExpectFailed.store(false)
130144
body()
131-
let endAnyExpectFailed = _anyExpectFailed
132-
_anyExpectFailed = false
145+
let endAnyExpectFailed = _anyExpectFailed.load()
146+
_anyExpectFailed.store(false)
133147
expectTrue(
134148
endAnyExpectFailed, "running `body` should produce an expected failure",
135149
stackTrace: stackTrace.pushIf(showFrame, file: file, line: line)
136150
)
137-
_anyExpectFailed = _anyExpectFailed || startAnyExpectFailed
151+
_anyExpectFailed.orAndFetch(startAnyExpectFailed)
138152
}
139153

140154
public func identity(_ element: OpaqueValue<Int>) -> OpaqueValue<Int> {
@@ -257,7 +271,7 @@ public func expectationFailure(
257271
_ reason: String,
258272
trace message: String,
259273
stackTrace: SourceLocStack) {
260-
_anyExpectFailed = true
274+
_anyExpectFailed.store(true)
261275
stackTrace.print()
262276
print(reason, terminator: reason == "" ? "" : "\n")
263277
print(message, terminator: message == "" ? "" : "\n")
@@ -681,12 +695,12 @@ public func expectNotNil<T>(_ value: T?,
681695
}
682696

683697
public func expectCrashLater(withMessage message: String = "") {
684-
print("\(_stdlibUnittestStreamPrefix);expectCrash;\(_anyExpectFailed)")
698+
print("\(_stdlibUnittestStreamPrefix);expectCrash;\(_anyExpectFailed.load())")
685699

686700
var stderr = _Stderr()
687701
print("\(_stdlibUnittestStreamPrefix);expectCrash;\(message)", to: &stderr)
688702

689-
_seenExpectCrash = true
703+
_seenExpectCrash.store(true)
690704
}
691705

692706
public func expectCrash(withMessage message: String = "", executing: () -> Void) -> Never {
@@ -831,10 +845,10 @@ func _childProcess() {
831845
}
832846

833847
let testSuite = _allTestSuites[_testSuiteNameToIndex[testSuiteName]!]
834-
_anyExpectFailed = false
848+
_anyExpectFailed.store(false)
835849
testSuite._runTest(name: testName, parameter: testParameter)
836850

837-
print("\(_stdlibUnittestStreamPrefix);end;\(_anyExpectFailed)")
851+
print("\(_stdlibUnittestStreamPrefix);end;\(_anyExpectFailed.load())")
838852

839853
var stderr = _Stderr()
840854
print("\(_stdlibUnittestStreamPrefix);end", to: &stderr)
@@ -1212,25 +1226,27 @@ class _ParentProcess {
12121226
if _runTestsInProcess {
12131227
if t.stdinText != nil {
12141228
print("The test \(fullTestName) requires stdin input and can't be run in-process, marking as failed")
1215-
_anyExpectFailed = true
1229+
_anyExpectFailed.store(true)
12161230
} else if t.requiresOwnProcess {
12171231
print("The test \(fullTestName) requires running in a child process and can't be run in-process, marking as failed.")
1218-
_anyExpectFailed = true
1232+
_anyExpectFailed.store(true)
12191233
} else {
1220-
_anyExpectFailed = false
1234+
_anyExpectFailed.store(false)
12211235
testSuite._runTest(name: t.name, parameter: testParameter)
12221236
}
12231237
} else {
1224-
(_anyExpectFailed, expectCrash, childTerminationStatus, crashStdout,
1238+
var anyExpectFailed = false
1239+
(anyExpectFailed, expectCrash, childTerminationStatus, crashStdout,
12251240
crashStderr) =
12261241
_runTestInChild(testSuite, t.name, parameter: testParameter)
1242+
_anyExpectFailed.store(anyExpectFailed)
12271243
}
12281244

12291245
// Determine if the test passed, not taking XFAILs into account.
12301246
var testPassed = false
12311247
switch (childTerminationStatus, expectCrash) {
12321248
case (.none, false):
1233-
testPassed = !_anyExpectFailed
1249+
testPassed = !_anyExpectFailed.load()
12341250

12351251
case (.none, true):
12361252
testPassed = false
@@ -1241,7 +1257,7 @@ class _ParentProcess {
12411257
print("the test crashed unexpectedly")
12421258

12431259
case (.some, true):
1244-
testPassed = !_anyExpectFailed
1260+
testPassed = !_anyExpectFailed.load()
12451261
}
12461262
if testPassed && t.crashOutputMatches.count > 0 {
12471263
// If we still think that the test passed, check if the crash

0 commit comments

Comments
 (0)