Skip to content

Commit 1a1f79c

Browse files
committed
Introduce safety checkin for ConcurrentValue conformance.
Introduce checking of ConcurrentValue conformances: - For structs, check that each stored property conforms to ConcurrentValue - For enums, check that each associated value conforms to ConcurrentValue - For classes, check that each stored property is immutable and conforms to ConcurrentValue Because all of the stored properties / associated values need to be visible for this check to work, limit ConcurrentValue conformances to be in the same source file as the type definition. This checking can be disabled by conforming to a new marker protocol, UnsafeConcurrentValue, that refines ConcurrentValue. UnsafeConcurrentValue otherwise his no specific meaning. This allows both "I know what I'm doing" for types that manage concurrent access themselves as well as enabling retroactive conformance, both of which are fundamentally unsafe but also quite necessary. The bulk of this change ended up being to the standard library, because all conformances of standard library types to the ConcurrentValue protocol needed to be sunk down into the standard library so they would benefit from the checking above. There were numerous little mistakes in the initial pass through the stsandard library types that have now been corrected.
1 parent 9b166c1 commit 1a1f79c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+344
-167
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4319,6 +4319,17 @@ WARNING(non_concurrent_function_type,none,
43194319
"`@concurrent` %select{function type|closure}0 has "
43204320
"non-concurrent-value %select{parameter|result}1 type %2",
43214321
(bool, bool, Type))
4322+
ERROR(non_concurrent_type_member,none,
4323+
"%select{stored property %1|associated value %1}0 of "
4324+
"'ConcurrentValue'-conforming %2 %3 has non-concurrent-value type %4",
4325+
(bool, DeclName, DescriptiveDeclKind, DeclName, Type))
4326+
ERROR(concurrent_value_class_mutable_property,none,
4327+
"stored property %0 of 'ConcurrentValue'-conforming %1 %2 is mutable",
4328+
(DeclName, DescriptiveDeclKind, DeclName))
4329+
ERROR(concurrent_value_outside_source_file,none,
4330+
"conformance 'ConcurrentValue' must occur in the same source file as "
4331+
"%0 %1; use 'UnsafeConcurrentValue' for retroactive conformance",
4332+
(DescriptiveDeclKind, DeclName))
43224333

43234334
ERROR(actorindependent_let,none,
43244335
"'@actorIndependent' is meaningless on 'let' declarations because "

include/swift/AST/KnownProtocols.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ PROTOCOL(Encodable)
8181
PROTOCOL(Decodable)
8282

8383
PROTOCOL(ConcurrentValue)
84+
PROTOCOL(UnsafeConcurrentValue)
8485

8586
PROTOCOL_(ObjectiveCBridgeable)
8687
PROTOCOL_(DestructorSafeContainer)

lib/IRGen/GenMeta.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5139,6 +5139,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
51395139
case KnownProtocolKind::FloatingPoint:
51405140
case KnownProtocolKind::Actor:
51415141
case KnownProtocolKind::ConcurrentValue:
5142+
case KnownProtocolKind::UnsafeConcurrentValue:
51425143
return SpecialProtocol::None;
51435144
}
51445145

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2429,3 +2429,68 @@ static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
24292429

24302430
return false;
24312431
}
2432+
2433+
void swift::checkConcurrentValueConformance(ProtocolConformance *conformance) {
2434+
auto conformanceDC = conformance->getDeclContext();
2435+
auto nominal = conformance->getType()->getAnyNominal();
2436+
if (!nominal)
2437+
return;
2438+
2439+
// Actors implicitly conform to ConcurrentValue and protect their state.
2440+
auto classDecl = dyn_cast<ClassDecl>(nominal);
2441+
if (classDecl && classDecl->isActor())
2442+
return;
2443+
2444+
// ConcurrentValue can only be used in the same source file.
2445+
auto conformanceDecl = conformanceDC->getAsDecl();
2446+
if (!conformanceDC->getParentSourceFile() ||
2447+
conformanceDC->getParentSourceFile() != nominal->getParentSourceFile()) {
2448+
conformanceDecl->diagnose(
2449+
diag::concurrent_value_outside_source_file,
2450+
nominal->getDescriptiveKind(), nominal->getName());
2451+
return;
2452+
}
2453+
2454+
// Stored properties of structs and classes must have
2455+
// ConcurrentValue-conforming types.
2456+
if (isa<StructDecl>(nominal) || classDecl) {
2457+
for (auto property : nominal->getStoredProperties()) {
2458+
if (classDecl && property->supportsMutation()) {
2459+
property->diagnose(diag::concurrent_value_class_mutable_property, property->getName(), nominal->getDescriptiveKind(),
2460+
nominal->getName());
2461+
continue;
2462+
}
2463+
2464+
auto propertyType =
2465+
conformanceDC->mapTypeIntoContext(property->getInterfaceType());
2466+
if (!isConcurrentValueType(conformanceDC, propertyType)) {
2467+
property->diagnose(
2468+
diag::non_concurrent_type_member, false, property->getName(),
2469+
nominal->getDescriptiveKind(), nominal->getName(), propertyType);
2470+
continue;
2471+
}
2472+
}
2473+
2474+
return;
2475+
}
2476+
2477+
// Associated values of enum cases must have ConcurrentValue-conforming
2478+
// types.
2479+
if (auto enumDecl = dyn_cast<EnumDecl>(nominal)) {
2480+
for (auto caseDecl : enumDecl->getAllCases()) {
2481+
for (auto element : caseDecl->getElements()) {
2482+
if (!element->hasAssociatedValues())
2483+
continue;
2484+
2485+
auto elementType = conformanceDC->mapTypeIntoContext(
2486+
element->getArgumentInterfaceType());
2487+
if (!isConcurrentValueType(conformanceDC, elementType)) {
2488+
element->diagnose(
2489+
diag::non_concurrent_type_member, true, element->getName(),
2490+
nominal->getDescriptiveKind(), nominal->getName(), elementType);
2491+
continue;
2492+
}
2493+
}
2494+
}
2495+
}
2496+
}

lib/Sema/TypeCheckConcurrency.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class Expr;
3636
class FuncDecl;
3737
class Initializer;
3838
class PatternBindingDecl;
39+
class ProtocolConformance;
3940
class TopLevelCodeDecl;
4041
class TypeBase;
4142
class ValueDecl;
@@ -242,6 +243,9 @@ bool diagnoseNonConcurrentTypesInFunctionType(
242243
const AnyFunctionType *fnType, const DeclContext *dc, SourceLoc loc,
243244
bool isClosure);
244245

246+
/// Check the correctness of the given ConcurrentValue conformance.
247+
void checkConcurrentValueConformance(ProtocolConformance *conformance);
248+
245249
} // end namespace swift
246250

247251
#endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5590,6 +5590,8 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
55905590
auto &Context = dc->getASTContext();
55915591
MultiConformanceChecker groupChecker(Context);
55925592

5593+
ProtocolConformance *concurrentValueConformance = nullptr;
5594+
ProtocolConformance *unsafeConcurrentValueConformance = nullptr;
55935595
bool anyInvalid = false;
55945596
for (auto conformance : conformances) {
55955597
// Check and record normal conformances.
@@ -5610,14 +5612,24 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
56105612
}
56115613
}
56125614

5613-
if (conformance->getProtocol()->
5614-
isSpecificProtocol(KnownProtocolKind::StringInterpolationProtocol)) {
5615+
auto proto = conformance->getProtocol();
5616+
if (proto->isSpecificProtocol(
5617+
KnownProtocolKind::StringInterpolationProtocol)) {
56155618
if (auto typeDecl = dc->getSelfNominalTypeDecl()) {
56165619
diagnoseMissingAppendInterpolationMethod(typeDecl);
56175620
}
5621+
} else if (proto->isSpecificProtocol(KnownProtocolKind::ConcurrentValue)) {
5622+
concurrentValueConformance = conformance;
5623+
} else if (proto->isSpecificProtocol(
5624+
KnownProtocolKind::UnsafeConcurrentValue)) {
5625+
unsafeConcurrentValueConformance = conformance;
56185626
}
56195627
}
56205628

5629+
// Check constraints of ConcurrentValue.
5630+
if (concurrentValueConformance && !unsafeConcurrentValueConformance)
5631+
checkConcurrentValueConformance(concurrentValueConformance);
5632+
56215633
// Check all conformances.
56225634
groupChecker.checkAllConformances();
56235635

stdlib/public/core/Array.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,3 +1984,5 @@ internal struct _ArrayAnyHashableBox<Element: Hashable>
19841984
return true
19851985
}
19861986
}
1987+
1988+
extension Array: ConcurrentValue where Element: ConcurrentValue { }

stdlib/public/core/ArrayBuffer.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,3 +673,6 @@ extension _ArrayBuffer {
673673
}
674674
}
675675
#endif
676+
677+
extension _ArrayBuffer: ConcurrentValue, UnsafeConcurrentValue
678+
where Element: ConcurrentValue { }

stdlib/public/core/ArraySlice.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,3 +1516,6 @@ extension ArraySlice {
15161516
shiftedToStartIndex: _startIndex))
15171517
}
15181518
}
1519+
1520+
extension ArraySlice: ConcurrentValue, UnsafeConcurrentValue
1521+
where Element: ConcurrentValue { }

stdlib/public/core/Bool.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
/// that functions, methods, and properties imported from C and Objective-C
6262
/// have a consistent type interface.
6363
@frozen
64-
public struct Bool {
64+
public struct Bool: ConcurrentValue {
6565
@usableFromInline
6666
internal var _value: Builtin.Int1
6767

0 commit comments

Comments
 (0)