Skip to content

Revert "[Concurrency] isCanceled spelling to follow guidance" #35564

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.

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

Merged
merged 1 commit into from
Jan 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions include/swift/ABI/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ static_assert(alignof(Job) == 2 * alignof(void*),
/// The current state of a task's status records.
class ActiveTaskStatus {
enum : uintptr_t {
IsCanceled = 0x1,
IsCancelled = 0x1,
IsLocked = 0x2,
RecordMask = ~uintptr_t(IsCanceled | IsLocked)
RecordMask = ~uintptr_t(IsCancelled | IsLocked)
};

uintptr_t Value;
Expand All @@ -106,10 +106,10 @@ class ActiveTaskStatus {
bool cancelled, bool locked)
: Value(reinterpret_cast<uintptr_t>(innermostRecord)
+ (locked ? IsLocked : 0)
+ (cancelled ? IsCanceled : 0)) {}
+ (cancelled ? IsCancelled : 0)) {}

/// Is the task currently cancelled?
bool isCanceled() const { return Value & IsCanceled; }
bool isCancelled() const { return Value & IsCancelled; }

/// Is there an active lock on the cancellation information?
bool isLocked() const { return Value & IsLocked; }
Expand Down Expand Up @@ -178,8 +178,8 @@ class AsyncTask : public HeapObject, public Job {

/// Check whether this task has been cancelled.
/// Checking this is, of course, inherently race-prone on its own.
bool isCanceled() const {
return Status.load(std::memory_order_relaxed).isCanceled();
bool isCancelled() const {
return Status.load(std::memory_order_relaxed).isCancelled();
}

/// A fragment of an async task structure that happens to be a child task.
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Runtime/Concurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
size_t swift_task_getJobFlags(AsyncTask* task);

SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
bool swift_task_isCanceled(AsyncTask* task);
bool swift_task_isCancelled(AsyncTask* task);

/// This should have the same representation as an enum like this:
/// enum NearestTaskDeadline {
Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/Concurrency/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,8 @@ void swift::swift_continuation_throwingResumeWithError(/* +1 */ SwiftError *erro
resumeTaskAfterContinuation(task, context);
}

bool swift::swift_task_isCanceled(AsyncTask *task) {
return task->isCanceled();
bool swift::swift_task_isCancelled(AsyncTask *task) {
return task->isCancelled();
}

SWIFT_CC(swift)
Expand Down
6 changes: 3 additions & 3 deletions stdlib/public/Concurrency/Task.swift
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func getJobFlags(_ task: Builtin.NativeObject) -> Task.JobFlags
@usableFromInline
func _enqueueJobGlobal(_ task: Builtin.Job)

@_silgen_name("swift_task_isCanceled")
@_silgen_name("swift_task_isCancelled")
func isTaskCancelled(_ task: Builtin.NativeObject) -> Bool

@_silgen_name("swift_task_runAndBlockThread")
Expand Down Expand Up @@ -415,8 +415,8 @@ public func _runGroupChildTask<T>(
@_silgen_name("swift_task_cancel")
func _taskCancel(_ task: Builtin.NativeObject)

@_silgen_name("swift_task_isCanceled")
func _taskIsCanceled(_ task: Builtin.NativeObject) -> Bool
@_silgen_name("swift_task_isCancelled")
func _taskIsCancelled(_ task: Builtin.NativeObject) -> Bool

#if _runtime(_ObjC)

Expand Down
12 changes: 6 additions & 6 deletions stdlib/public/Concurrency/TaskCancellation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ extension Task {
///
/// - SeeAlso: `checkCancellation()`
/* @instantaneous */
public static func isCanceled() async -> Bool {
_taskIsCanceled(Builtin.getCurrentAsyncTask())
public static func isCancelled() async -> Bool {
_taskIsCancelled(Builtin.getCurrentAsyncTask())
}

/// Check if the task is cancelled and throw an `CancellationError` if it was.
Expand All @@ -41,10 +41,10 @@ extension Task {
/// ### Suspension
/// This function returns instantly and will never suspend.
///
/// - SeeAlso: `isCanceled()`
/// - SeeAlso: `isCancelled()`
/* @instantaneous */
public static func checkCancellation() async throws {
if await Task.isCanceled() {
if await Task.isCancelled() {
throw CancellationError()
}
}
Expand Down Expand Up @@ -108,7 +108,7 @@ extension Task {
/// regardless of the inner deadlines of the specific child tasks.
///
/// Cancellation is co-operative and must be checked for by the operation, e.g.
/// by invoking `Task.checkCancellation`, or `Task.isCanceled`.
/// by invoking `Task.checkCancellation`, or `Task.isCancelled`.
///
/// ### Suspension
/// This function returns instantly and will never suspend.
Expand All @@ -135,7 +135,7 @@ extension Task {
/// regardless of the inner deadlines of the specific child tasks.
///
/// Cancellation is co-operative and must be checked for by the operation, e.g.
/// by invoking `Task.checkCancellation` or `Task.isCanceled`.
/// by invoking `Task.checkCancellation` or `Task.isCancelled`.
///
/// ### Suspension
/// This function returns instantly and will never suspend.
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/Concurrency/TaskGroup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ extension Task {
///
/// - SeeAlso: `Task.addCancellationHandler`
/// - SeeAlso: `Task.checkCancelled`
/// - SeeAlso: `Task.isCanceled`
/// - SeeAlso: `Task.isCancelled`
public mutating func cancelAll() {
_taskCancel(self.task) // TODO: do we also have to go over all child tasks and cancel there?
}
Expand Down
26 changes: 13 additions & 13 deletions stdlib/public/Concurrency/TaskStatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ static void waitForStatusRecordUnlock(AsyncTask *task,
/// If `forCancellation` is true, the cancelled bit will be set in the
/// state, and the lock will not be acquired if the task is already
/// cancelled or can be cancelled without the lock. If this occurs,
/// `isCanceled()` will be true for the return value.
/// `isCancelled()` will be true for the return value.
static ActiveTaskStatus
acquireStatusRecordLock(AsyncTask *task,
Optional<StatusRecordLockRecord> &recordLockRecord,
Expand All @@ -204,7 +204,7 @@ acquireStatusRecordLock(AsyncTask *task,
// Cancellation should be idempotent: if the task has already
// been cancelled (or is being cancelled concurrently), there
// shouldn't be any need to do this work again.
if (oldStatus.isCanceled() && forCancellation)
if (oldStatus.isCancelled() && forCancellation)
return oldStatus;

// If the old info says we're locked, wait for the lock to clear.
Expand Down Expand Up @@ -238,9 +238,9 @@ acquireStatusRecordLock(AsyncTask *task,

// Install the lock record as the active cancellation info, or
// restart if that fails.
bool newIsCanceled = forCancellation || oldStatus.isCanceled();
bool newIsCancelled = forCancellation || oldStatus.isCancelled();
ActiveTaskStatus newStatus(&*recordLockRecord,
/*cancelled*/ newIsCanceled,
/*cancelled*/ newIsCancelled,
/*locked*/ true);
if (task->Status.compare_exchange_weak(oldStatus, newStatus,
/*success*/ std::memory_order_release,
Expand Down Expand Up @@ -288,12 +288,12 @@ bool swift::swift_task_addStatusRecord(AsyncTask *task,
// We have to use a release on success to make the initialization of
// the new record visible to the cancelling thread.
ActiveTaskStatus newStatus(newRecord,
oldStatus.isCanceled(),
oldStatus.isCancelled(),
/*locked*/ false);
if (task->Status.compare_exchange_weak(oldStatus, newStatus,
/*success*/ std::memory_order_release,
/*failure*/ std::memory_order_relaxed))
return !oldStatus.isCanceled();
return !oldStatus.isCancelled();
}
}

Expand All @@ -305,14 +305,14 @@ bool swift::swift_task_tryAddStatusRecord(AsyncTask *task,

while (true) {
// If the old info is already cancelled, do nothing.
if (oldStatus.isCanceled())
if (oldStatus.isCancelled())
return false;

// Wait for any active lock to be released.
if (oldStatus.isLocked()) {
waitForStatusRecordUnlock(task, oldStatus);

if (oldStatus.isCanceled())
if (oldStatus.isCancelled())
return false;
}

Expand Down Expand Up @@ -345,12 +345,12 @@ bool swift::swift_task_removeStatusRecord(AsyncTask *task,
// If the record is the innermost record, try to just pop it off.
if (oldStatus.getInnermostRecord() == record) {
ActiveTaskStatus newStatus(record->getParent(),
oldStatus.isCanceled(),
oldStatus.isCancelled(),
/*locked*/ false);
if (task->Status.compare_exchange_weak(oldStatus, newStatus,
/*success*/ std::memory_order_release,
/*failure*/ std::memory_order_relaxed))
return !oldStatus.isCanceled();
return !oldStatus.isCancelled();

// Otherwise, restart.
continue;
Expand Down Expand Up @@ -389,7 +389,7 @@ bool swift::swift_task_removeStatusRecord(AsyncTask *task,
// exactly what we want to restore.
releaseStatusRecordLock(task, oldStatus, recordLockRecord);

return !oldStatus.isCanceled();
return !oldStatus.isCancelled();
}

/**************************************************************************/
Expand Down Expand Up @@ -443,7 +443,7 @@ void swift::swift_task_cancel(AsyncTask *task) {

// If we were already cancelled or were able to cancel without acquiring
// the lock, there's nothing else to do.
if (oldStatus.isCanceled())
if (oldStatus.isCancelled())
return;

// Otherwise, we've installed the lock record and are now the
Expand Down Expand Up @@ -561,7 +561,7 @@ NearestTaskDeadline swift::swift_task_getNearestDeadline(AsyncTask *task) {
NearestTaskDeadline result;

// If it's already cancelled, we're done.
if (oldStatus.isCanceled()) {
if (oldStatus.isCancelled()) {
result.ValueKind = NearestTaskDeadline::AlreadyCancelled;
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func test_skipCallingNext_butInvokeCancelAll() async {
await group.add { () async -> Int in
sleep(1)
print(" inside group.add { \(n) }")
let cancelled = await Task.isCanceled()
let cancelled = await Task.isCancelled()
print(" inside group.add { \(n) } (canceled: \(cancelled))")
return n
}
Expand All @@ -24,7 +24,7 @@ func test_skipCallingNext_butInvokeCancelAll() async {
group.cancelAll()

// return immediately; the group should wait on the tasks anyway
print("return immediately 0 (canceled: \(await Task.isCanceled()))")
print("return immediately 0 (canceled: \(await Task.isCancelled()))")
return 0
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ func test_skipCallingNext() async {
print("group.add { \(n) }")
await group.add { () async -> Int in
sleep(1)
print(" inside group.add { \(n) } (canceled: \(await Task.isCanceled()))")
print(" inside group.add { \(n) } (canceled: \(await Task.isCancelled()))")
return n
}
}

// return immediately; the group should wait on the tasks anyway
print("return immediately 0 (canceled: \(await Task.isCanceled()))")
print("return immediately 0 (canceled: \(await Task.isCancelled()))")
return 0
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func test_taskGroup_throws() async {
print("error caught in group: \(error)")

await group.add { () async -> Int in
print("task 3 (cancelled: \(await Task.isCanceled()))")
print("task 3 (cancelled: \(await Task.isCancelled()))")
return 3
}

Expand Down
8 changes: 4 additions & 4 deletions test/Concurrency/async_cancellation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ func test_cancellation_checkCancellation() async throws {
try await Task.checkCancellation()
}

func test_cancellation_guard_isCanceled(_ any: Any) async -> PictureData {
guard await !Task.isCanceled() else {
func test_cancellation_guard_isCancelled(_ any: Any) async -> PictureData {
guard await !Task.isCancelled() else {
return PictureData.failedToLoadImagePlaceholder
}

Expand All @@ -29,7 +29,7 @@ func test_cancellation_withCancellationHandler(_ anything: Any) async -> Picture
return try await Task.withCancellationHandler(
handler: { file.close() },
operation: {
await test_cancellation_guard_isCanceled(file)
await test_cancellation_guard_isCancelled(file)
})
}

Expand All @@ -41,7 +41,7 @@ func test_cancellation_loop() async -> Int {

let tasks = [SampleTask(), SampleTask()]
var processed = 0
for t in tasks where await !Task.isCanceled() {
for t in tasks where await !Task.isCancelled() {
await t.process()
processed += 1
}
Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/async_task_groups.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ func asyncThrowsFunc() async throws -> Int { 42 }
func asyncThrowsOnCancel() async throws -> Int {
// terrible suspend-spin-loop -- do not do this
// only for purposes of demonstration
while await !Task.isCanceled() {
while await !Task.isCancelled() {
await Task.sleep(until: Task.Deadline.in(.seconds(1)))
}

Expand Down
12 changes: 6 additions & 6 deletions unittests/runtime/TaskStatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ TEST(TaskStatusTest, cancellation_simple) {
withSimpleTask(Storage{47},
[&](AsyncTask *task, ExecutorRef executor,
ValueContext<Storage> *context) {
EXPECT_FALSE(task->isCanceled());
EXPECT_FALSE(task->isCancelled());
swift_task_cancel(task);
EXPECT_TRUE(task->isCanceled());
EXPECT_TRUE(task->isCancelled());
swift_task_cancel(task);
EXPECT_TRUE(task->isCanceled());
EXPECT_TRUE(task->isCancelled());
}, [&](AsyncTask *task) {
task->run(createFakeExecutor(1234));
});
Expand All @@ -145,7 +145,7 @@ TEST(TaskStatusTest, deadline) {
withSimpleTask(Storage{47},
[&](AsyncTask *task, ExecutorRef executor,
ValueContext<Storage> *context) {
EXPECT_FALSE(task->isCanceled());
EXPECT_FALSE(task->isCancelled());

TaskDeadline deadlineOne = { 1234 };
TaskDeadline deadlineTwo = { 2345 };
Expand Down Expand Up @@ -252,7 +252,7 @@ TEST(TaskStatusTest, deadline) {

// Cancel.
swift_task_cancel(task);
EXPECT_TRUE(task->isCanceled());
EXPECT_TRUE(task->isCancelled());

// We should report already cancelled now.
nearest = swift_task_getNearestDeadline(task);
Expand All @@ -277,7 +277,7 @@ TEST(TaskStatusTest, deadline) {
nearest = swift_task_getNearestDeadline(task);
EXPECT_EQ(NearestTaskDeadline::AlreadyCancelled, nearest.ValueKind);

EXPECT_TRUE(task->isCanceled());
EXPECT_TRUE(task->isCancelled());
}, [&](AsyncTask *task) {
task->run(createFakeExecutor(1234));
});
Expand Down