Skip to content

Resolve warnings in the Concurrency runtime #70948

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.

Sign up for GitHub

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 3 commits into from
Jan 17, 2024
Merged

Resolve warnings in the Concurrency runtime #70948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00da82d
NFC: Remove a superfluous if #available from Executor.swift.
tshortli Jan 16, 2024
f18b196
NFC: Suppress a -Wunreachable-code warning in GlobalExecutor.cpp.
tshortli Jan 16, 2024
8343e7a
NFC: Suppress -Wreturn-type-c-linkage warnings in GlobalExecutor.cpp.
tshortli Jan 17, 2024
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
6 changes: 1 addition & 5 deletions stdlib/public/Concurrency/Executor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,7 @@ where E: SerialExecutor {
@_silgen_name("_swift_task_enqueueOnTaskExecutor")
internal func _enqueueOnTaskExecutor<E>(job unownedJob: UnownedJob, executor: E) where E: _TaskExecutor {
#if !SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY
if #available(SwiftStdlib 9999, *) {
executor.enqueue(ExecutorJob(context: unownedJob._context))
} else {
executor.enqueue(unownedJob)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, that's safe -- thank you

executor.enqueue(ExecutorJob(context: unownedJob._context))
#else // SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY
executor.enqueue(unownedJob)
#endif // !SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY
Expand Down
6 changes: 6 additions & 0 deletions stdlib/public/Concurrency/GlobalExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,24 @@ void swift::swift_task_enqueueGlobalWithDeadline(
// Implemented in Swift because we need to obtain the user-defined flags on the executor ref.
//
// We could inline this with effort, though.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
extern "C" SWIFT_CC(swift)
SerialExecutorRef _task_serialExecutor_getExecutorRef(
HeapObject *executor, const Metadata *selfType,
const SerialExecutorWitnessTable *wtable);
#pragma clang diagnostic pop

// Implemented in Swift because we need to obtain the user-defined flags on the executor ref.
//
// We could inline this with effort, though.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
extern "C" SWIFT_CC(swift)
TaskExecutorRef _task_executor_getTaskExecutorRef(
HeapObject *executor, const Metadata *selfType,
const SerialExecutorWitnessTable *wtable);
#pragma clang diagnostic pop

SWIFT_CC(swift)
static bool swift_task_isOnExecutorImpl(HeapObject *executor,
Expand Down
5 changes: 5 additions & 0 deletions stdlib/public/Concurrency/TaskGroup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,12 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context)
_swift_taskGroup_detachChild(asAbstract(this), completedTask);
return unlock();
}
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunreachable-code"
// This _should_ be statically unreachable, but we leave it in as a
// safeguard in case the control flow above changes.
swift_unreachable("expected to early return from when handling offer of last task in group");
#pragma clang diagnostic pop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you -- yeah it's better to keep this comitted in case the control flow would change and we wrongly allowed this to be entered

}

assert(!hadErrorResult && "only successfully completed tasks can reach here");
Expand Down