Skip to content

Commit 4f199e9

Browse files
ktosoktoso
committed
[Concurrency] Warn about "old" Executor.enqueue impl and prefer new
move new enqueue method to be named enqueue and not enqueueOwned
1 parent 8c897e9 commit 4f199e9

20 files changed

+327
-55
lines changed

‎include/swift/AST/Decl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3852,6 +3852,9 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
38523852
/// Find the 'RemoteCallArgument(label:name:value:)' initializer function.
38533853
ConstructorDecl *getDistributedRemoteCallArgumentInitFunction() const;
38543854

3855+
/// Get the move-only `enqueue(Job)` protocol requirement function on the `Executor` protocol.
3856+
AbstractFunctionDecl *getExecutorOwnedEnqueueFunction() const;
3857+
38553858
/// Collect the set of protocols to which this type should implicitly
38563859
/// conform, such as AnyObject (for classes).
38573860
void getImplicitProtocols(SmallVectorImpl<ProtocolDecl *> &protocols);

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6393,6 +6393,11 @@ WARNING(hashvalue_implementation,Deprecation,
63936393
"conform type %0 to 'Hashable' by implementing 'hash(into:)' instead",
63946394
(Type))
63956395

6396+
WARNING(executor_enqueue_unowned_implementation,Deprecation,
6397+
"'Executor.enqueue(UnownedJob)' is deprecated as a protocol requirement; "
6398+
"conform type %0 to 'Executor' by implementing 'func enqueue(Job)' instead",
6399+
(Type))
6400+
63966401
//------------------------------------------------------------------------------
63976402
// MARK: property wrapper diagnostics
63986403
//------------------------------------------------------------------------------

‎include/swift/AST/KnownProtocols.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ PROTOCOL(SIMDScalar)
7676
PROTOCOL(BinaryInteger)
7777
PROTOCOL(FixedWidthInteger)
7878
PROTOCOL(RangeReplaceableCollection)
79+
PROTOCOL(Executor)
7980
PROTOCOL(SerialExecutor)
8081
PROTOCOL(GlobalActor)
8182

‎include/swift/AST/KnownSDKTypes.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ KNOWN_SDK_TYPE_DECL(ObjectiveC, ObjCBool, StructDecl, 0)
3939
// standardized
4040
KNOWN_SDK_TYPE_DECL(Concurrency, UnsafeContinuation, NominalTypeDecl, 2)
4141
KNOWN_SDK_TYPE_DECL(Concurrency, MainActor, NominalTypeDecl, 0)
42+
KNOWN_SDK_TYPE_DECL(Concurrency, Job, StructDecl, 0)
43+
KNOWN_SDK_TYPE_DECL(Concurrency, UnownedJob, StructDecl, 0)
44+
KNOWN_SDK_TYPE_DECL(Concurrency, Executor, NominalTypeDecl, 0)
45+
KNOWN_SDK_TYPE_DECL(Concurrency, SerialExecutor, NominalTypeDecl, 0)
4246
KNOWN_SDK_TYPE_DECL(Concurrency, UnownedSerialExecutor, NominalTypeDecl, 0)
4347

4448
KNOWN_SDK_TYPE_DECL(Concurrency, TaskLocal, ClassDecl, 1)

‎lib/AST/ASTContext.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,7 @@ ProtocolDecl *ASTContext::getProtocol(KnownProtocolKind kind) const {
11201120
case KnownProtocolKind::GlobalActor:
11211121
case KnownProtocolKind::AsyncSequence:
11221122
case KnownProtocolKind::AsyncIteratorProtocol:
1123+
case KnownProtocolKind::Executor:
11231124
case KnownProtocolKind::SerialExecutor:
11241125
M = getLoadedModule(Id_Concurrency);
11251126
break;

‎lib/AST/Decl.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5193,6 +5193,42 @@ VarDecl *NominalTypeDecl::getGlobalActorInstance() const {
51935193
nullptr);
51945194
}
51955195

5196+
AbstractFunctionDecl *
5197+
NominalTypeDecl::getExecutorOwnedEnqueueFunction() const {
5198+
auto &C = getASTContext();
5199+
5200+
auto proto = dyn_cast<ProtocolDecl>(this);
5201+
if (!proto)
5202+
return nullptr;
5203+
5204+
llvm::SmallVector<ValueDecl *, 2> results;
5205+
lookupQualified(getSelfNominalTypeDecl(),
5206+
DeclNameRef(C.Id_enqueue),
5207+
NL_ProtocolMembers,
5208+
results);
5209+
5210+
for (auto candidate: results) {
5211+
// we're specifically looking for the Executor protocol requirement
5212+
if (!isa<ProtocolDecl>(candidate->getDeclContext()))
5213+
continue;
5214+
5215+
fprintf(stderr, "[%s:%d](%s) candidate\n", __FILE_NAME__, __LINE__, __FUNCTION__);
5216+
candidate->dump();
5217+
5218+
if (auto *funcDecl = dyn_cast<AbstractFunctionDecl>(candidate)) {
5219+
if (funcDecl->getParameters()->size() != 1)
5220+
continue;
5221+
5222+
auto params = funcDecl->getParameters();
5223+
if (params->get(0)->getSpecifier() == ParamSpecifier::Owned) {
5224+
return funcDecl;
5225+
}
5226+
}
5227+
}
5228+
5229+
return nullptr;
5230+
}
5231+
51965232
ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
51975233
ArrayRef<InheritedEntry> Inherited,
51985234
GenericParamList *GenericParams, DeclContext *Parent,

‎lib/IRGen/GenMeta.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6000,6 +6000,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
60006000
case KnownProtocolKind::CxxSequence:
60016001
case KnownProtocolKind::UnsafeCxxInputIterator:
60026002
case KnownProtocolKind::UnsafeCxxRandomAccessIterator:
6003+
case KnownProtocolKind::Executor:
60036004
case KnownProtocolKind::SerialExecutor:
60046005
case KnownProtocolKind::Sendable:
60056006
case KnownProtocolKind::UnsafeSendable:

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,79 @@ void swift::diagnoseMissingExplicitSendable(NominalTypeDecl *nominal) {
12251225
}
12261226
}
12271227

1228+
void swift::tryDiagnoseExecutorConformance(ASTContext &C,
1229+
const NominalTypeDecl *nominal,
1230+
ProtocolDecl *proto) {
1231+
assert(proto->isSpecificProtocol(KnownProtocolKind::Executor) ||
1232+
proto->isSpecificProtocol(KnownProtocolKind::SerialExecutor));
1233+
1234+
auto &diags = C.Diags;
1235+
auto module = nominal->getParentModule();
1236+
Type nominalTy = nominal->getDeclaredInterfaceType();
1237+
1238+
// enqueue(_: UnownedJob)
1239+
auto enqueueDeclName = DeclName(C, DeclBaseName(C.Id_enqueue), { Identifier() });
1240+
1241+
FuncDecl *unownedEnqueueRequirement = nullptr;
1242+
FuncDecl *moveOnlyEnqueueRequirement = nullptr;
1243+
for (auto req: proto->getProtocolRequirements()) {
1244+
auto *funcDecl = dyn_cast<FuncDecl>(req);
1245+
if (!funcDecl)
1246+
continue;
1247+
1248+
if (funcDecl->getName() != enqueueDeclName)
1249+
continue;
1250+
1251+
1252+
// look for the first parameter being a Job or UnownedJob
1253+
if (funcDecl->getParameters()->size() != 1)
1254+
continue;
1255+
if (auto param = funcDecl->getParameters()->front()) {
1256+
if (param->getType()->isEqual(C.getJobDecl()->getDeclaredInterfaceType())) {
1257+
assert(moveOnlyEnqueueRequirement == nullptr);
1258+
moveOnlyEnqueueRequirement = funcDecl;
1259+
} else if (param->getType()->isEqual(C.getUnownedJobDecl()->getDeclaredInterfaceType())) {
1260+
assert(unownedEnqueueRequirement == nullptr);
1261+
unownedEnqueueRequirement = funcDecl;
1262+
}
1263+
}
1264+
1265+
// if we found both, we're done here and break out of the loop
1266+
if (unownedEnqueueRequirement && moveOnlyEnqueueRequirement)
1267+
break; // we're done looking for the requirements
1268+
}
1269+
1270+
1271+
auto conformance = module->lookupConformance(nominalTy, proto);
1272+
auto concreteConformance = conformance.getConcrete();
1273+
auto unownedEnqueueWitness = concreteConformance->getWitnessDeclRef(unownedEnqueueRequirement);
1274+
auto moveOnlyEnqueueWitness = concreteConformance->getWitnessDeclRef(moveOnlyEnqueueRequirement);
1275+
1276+
if (auto enqueueUnownedDecl = unownedEnqueueWitness.getDecl()) {
1277+
// Old UnownedJob based impl is present, warn about it suggesting the new protocol requirement.
1278+
if (enqueueUnownedDecl->getLoc().isValid()) {
1279+
diags.diagnose(enqueueUnownedDecl->getLoc(), diag::executor_enqueue_unowned_implementation, nominalTy);
1280+
}
1281+
}
1282+
1283+
if (auto unownedEnqueueDecl = unownedEnqueueWitness.getDecl()) {
1284+
if (auto moveOnlyEnqueueDecl = moveOnlyEnqueueWitness.getDecl()) {
1285+
if (unownedEnqueueDecl && unownedEnqueueDecl->getLoc().isInvalid() &&
1286+
moveOnlyEnqueueDecl && moveOnlyEnqueueDecl->getLoc().isInvalid()) {
1287+
// Neither old nor new implementation have been found, but we provide default impls for them
1288+
// that are mutually recursive, so we must error and suggest implementing the right requirement.
1289+
auto ownedRequirement = C.getExecutorDecl()->getExecutorOwnedEnqueueFunction();
1290+
nominal->diagnose(diag::type_does_not_conform, nominalTy, proto->getDeclaredInterfaceType());
1291+
ownedRequirement->diagnose(diag::no_witnesses,
1292+
getProtocolRequirementKind(ownedRequirement),
1293+
ownedRequirement->getName(),
1294+
proto->getDeclaredInterfaceType(),
1295+
/*AddFixIt=*/true);
1296+
}
1297+
}
1298+
}
1299+
}
1300+
12281301
/// Determine whether this is the main actor type.
12291302
/// FIXME: the diagnostics engine has a copy of this.
12301303
static bool isMainActor(Type type) {

‎lib/Sema/TypeCheckConcurrency.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,9 @@ void diagnoseMissingSendableConformance(
280280
/// state whether it conforms to Sendable, provide a diagnostic.
281281
void diagnoseMissingExplicitSendable(NominalTypeDecl *nominal);
282282

283+
/// Warn about deprecated `Executor.enqueue` implementations.
284+
void tryDiagnoseExecutorConformance(ASTContext &C, const NominalTypeDecl *nominal, ProtocolDecl *proto);
285+
283286
/// How the Sendable check should be performed.
284287
enum class SendableCheck {
285288
/// Sendable conformance was explicitly stated and should be

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6496,6 +6496,8 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
64966496
} else if (proto->isSpecificProtocol(
64976497
KnownProtocolKind::UnsafeSendable)) {
64986498
sendableConformanceIsUnchecked = true;
6499+
} else if (proto->isSpecificProtocol(KnownProtocolKind::Executor)) {
6500+
tryDiagnoseExecutorConformance(Context, nominal, proto);
64996501
}
65006502
}
65016503

‎stdlib/public/BackDeployConcurrency/Actor.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,14 @@ JobPriority swift::swift_task_getCurrentThreadPriority() {
285285
SWIFT_CC(swift)
286286
static bool swift_task_isCurrentExecutorImpl(ExecutorRef executor) {
287287
if (auto currentTracking = ExecutorTrackingInfo::current()) {
288+
fprintf(stderr, "[%s:%d](%s) current executor = %p, main:%s, default:%s\n", __FILE_NAME__, __LINE__, __FUNCTION__,
289+
currentTracking->getActiveExecutor().getIdentity(),
290+
currentTracking->getActiveExecutor().isMainExecutor() ? "y" : "n",
291+
currentTracking->getActiveExecutor().isDefaultActor() ? "y" : "n");
288292
return currentTracking->getActiveExecutor() == executor;
289293
}
290294

295+
fprintf(stderr, "[%s:%d](%s) no executor found in tracking\n", __FILE_NAME__, __LINE__, __FUNCTION__);
291296
return executor.isMainExecutor() && isExecutingOnMainThread();
292297
}
293298

‎stdlib/public/BackDeployConcurrency/Executor.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ internal final class DispatchQueueShim: @unchecked Sendable, SerialExecutor {
117117
_enqueueOnDispatchQueue(job, queue: self)
118118
}
119119

120+
func enqueue(_ job: __owned Job) {
121+
_enqueueOnDispatchQueue(job, queue: self)
122+
}
123+
120124
func asUnownedSerialExecutor() -> UnownedSerialExecutor {
121125
return UnownedSerialExecutor(ordinary: self)
122126
}

‎stdlib/public/Concurrency/Executor.swift

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,31 @@ import Swift
1515
/// A service that can execute jobs.
1616
@available(SwiftStdlib 5.1, *)
1717
public protocol Executor: AnyObject, Sendable {
18-
// This requirement is repeated here as a non-override so that we
19-
// get a redundant witness-table entry for it. This allows us to
20-
// avoid drilling down to the base conformance just for the basic
21-
// work-scheduling operation.
22-
@available(*, deprecated, message: "Implement enqueueJob instead")
18+
@available(*, deprecated, message: "Implement 'enqueue(_: __owned Job)' instead")
2319
func enqueue(_ job: UnownedJob)
2420

2521
@available(SwiftStdlib 5.9, *)
26-
func enqueueJob(_ job: __owned Job) // FIXME: figure out how to introduce in compatible way
22+
func enqueue(_ job: __owned Job)
2723
}
2824

2925
/// A service that executes jobs.
3026
@available(SwiftStdlib 5.1, *)
3127
public protocol SerialExecutor: Executor {
32-
28+
// This requirement is repeated here as a non-override so that we
29+
// get a redundant witness-table entry for it. This allows us to
30+
// avoid drilling down to the base conformance just for the basic
31+
// work-scheduling operation.
3332
@_nonoverride
34-
@available(*, deprecated, message: "Implement enqueueJob instead")
33+
@available(*, deprecated, message: "Implement 'enqueue_: __owned Job) instead")
3534
func enqueue(_ job: UnownedJob)
3635

36+
// This requirement is repeated here as a non-override so that we
37+
// get a redundant witness-table entry for it. This allows us to
38+
// avoid drilling down to the base conformance just for the basic
39+
// work-scheduling operation.
3740
@_nonoverride
3841
@available(SwiftStdlib 5.9, *)
39-
func enqueueJob(_ job: __owned Job) // FIXME: figure out how to introduce in compatible way
42+
func enqueue(_ job: __owned Job)
4043

4144
/// Convert this executor value to the optimized form of borrowed
4245
/// executor references.
@@ -45,14 +48,22 @@ public protocol SerialExecutor: Executor {
4548

4649
@available(SwiftStdlib 5.9, *)
4750
extension Executor {
48-
public func enqueue(_ job: UnownedJob) { // FIXME: this is bad; how could we deploy this nicely
51+
public func enqueue(_ job: UnownedJob) {
4952
fatalError("Please implement \(Self.self).enqueueJob(_:)")
5053
}
51-
public func enqueueJob(_ job: __owned Job) {
54+
55+
public func enqueue(_ job: __owned Job) {
5256
self.enqueue(UnownedJob(job))
5357
}
5458
}
5559

60+
@available(SwiftStdlib 5.9, *)
61+
extension SerialExecutor {
62+
public func asUnownedSerialExecutor() -> UnownedSerialExecutor {
63+
UnownedSerialExecutor(ordinary: self)
64+
}
65+
}
66+
5667
/// An unowned reference to a serial executor (a `SerialExecutor`
5768
/// value).
5869
///
@@ -117,7 +128,7 @@ internal func _getJobTaskId(_ job: UnownedJob) -> UInt64
117128
internal func _enqueueOnExecutor<E>(job unownedJob: UnownedJob, executor: E)
118129
where E: SerialExecutor {
119130
if #available(SwiftStdlib 5.9, *) {
120-
executor.enqueueJob(Job(context: unownedJob.context))
131+
executor.enqueue(Job(context: unownedJob.context))
121132
} else {
122133
executor.enqueue(unownedJob)
123134
}

0 commit comments

Comments
 (0)