Hello,
This revision of the proposal is looking good. Here are a couple of suggestions:
ConcurrentValue conformances
A conformance to ConcurrentValue should be restricted to the same source file as the type definition. This ensures that every stored property or associated value is visible (even the private ones). From outside that file or module, one can only declare conformance to UnsafeConcurrentValue. This ensures that retroactive conformances acknowledge that they are fundamentally unsafe.
I think we should allow a class to conform to ConcurrentValue, with these checks:
- Each stored property must be immutable (introduced with
let) and conform to ConcurrentValue,
- If the class has a superclass, that superclass must conform to
ConcurrentValue (or be NSObject; the same carve-out we have for actors), and
- The conformance must be in the same source file as the class definition.
Note that ConcurrentValue is a protocol, so conformance to ConcurrentValue is inherited by subclasses. Therefore, if you create an ConcurrentValue class:
class Point : ConcurrentValue { // okay: all stored properties are immutable and conform to ConcurrentValue
let x, y: Double
}
The subclasses also conform to ConcurrentValue and get the same checking:
class ColoredPoint: Point {
let color: Color
}
class Point3D: Point {
var z: Double // error: mutable stored property in `ConcurrentValue`-conforming class `Point3D`
}
Again, if you can't meet those requirements, use UnsafeConcurrentValue and take matters into your own hands. You could even do that for Point3D.
This really only picks up one case, but it's a useful one: immutable classes. Immutable classes can be safely shared so long as their stored properties conform to ConcurrentValue. Having the compiler check that makes this safe use of classes supported.
It's a small thing, but should we infer ConcurrentValue conformances on imported C enums? You can't make them unsafe in C.
Standard library conformances
While perhaps we don't want to list every single standard library type that should conform to ConcurrentValue, I think the proposal shouldn't try to push the work on a separate library-focused proposal.
I did go through all of the types in the standard library, and my suggestion is that nearly every type conform to ConcurrentValue, using conditional conformances on all generic parameters when those types have generic parameters. The exceptions are:
AnyKeyPath: key paths can become ConcurrentValue at PartialKeyPath, using a conditional conformance extension PartialKeyPath: ConcurrentValue where Root: ConcurrentValue.- Several
Codable-related types: CodingKey is a protocol that cannot add ConcurrentValue conformance retroactively, so EncodingError / DecodingError cannot conform, nor can Keyed(Encoding|Decoding)Container.
ManagedBuffer: it's a class that's meant to be shared. There's no way for it to be safe.Unsafe(Mutable)?(Raw?)Pointer: these should unconditionally conform to UnsafeConcurrentValue. They've already fully acknowledged that they are unsafe in every way; this will reduce friction for those that have taken matters into their own hands.- Lazy algorithm adapter types. When you do a
something.lazy.map { ... }, you get back some kind of lazy sequence adapter type. That type embeds a function type that is not @concurrent, so the adapter types should not conform to ConcurrentValue.
@concurrent functions
I'm confused by this rule:
Functions have @concurrent function type if their parameter and result types all conform to ConcurrentValue, they capture no values (e.g. global functions and closures / nested functions with no captures), and have no inout arguments.
I think this means that an existing function
func doSomething(i: Int) -> String { ... }
would have its type changed from (Int) -> String to @concurrent (Int) -> String by this proposal. That's going to break both source code and ABIs. I also don't feel that it's correct, because existing global functions may very well access global state.
I think the rule here should be that @concurrent is opt-in for functions, and the first/third bullets in should be compressed into something like:
- A function can be marked
@concurrent. Its parameter and result types must conform to ConcurrentValue, it may not have any inout parameters, and any captures must also conform to ConcurrentValue.
There's a place where you mention that @concurrent is "exactly like @escaping", which is a little misleading. @escaping is essentially the default for closures, for cases where you have no context:
let fn = { $0 + $1 } // fn will have escaping function type
@concurrent is not the default for closures. It's more like "noescape" in that sense, because it can only be inferred through context:
let concurrentFn: @concurrent (Int, Int) -> Int = { $0 + $1 } // concurrent, escaping function type
or via the proposed { @concurrent in $0 + $1 }.
I think we should tweak the rule about captures in @concurrent functions and closures. I mentioned it before (although the idea is @Joe_Groff 's), and @michelf ended up providing a good example:
The by-value vs. by-capture is invisible here, which is the subtlety @michelf is pointing out. Joe suggested we introduce a flow-sensitive rule banning mutation of the original variable after the point of capture. @concurrent closures, as already in the proposal, cannot ever mutate their captures. Joe's rule says that once you hit the point of capturing a variable, nobody can mutate it. In the example above, the compiler would produce an error at the line state += 1 if Printer took its closure as @concurrent:
error: mutation of variable `state` after it was captured by a concurrent closure
This checking eliminates the surprise, by making sure that the value can never change (for anyone) after it's been captured in a concurrent closure.
There's one extra check that I've found useful with local functions, which is to require them to be marked @concurrent if they are referenced from other @concurrent code. It's best to start with an example:
func globalFunc() {
var k = 17
acceptConcurrentClosure {
local() // note: access in concurrently-executed code here
}
func local() { // error: concurrently-executed local function 'local()' must be marked as '@concurrent'
k = 25 // error: mutation of captured var 'k' in concurrently-executing code
}
}
local is being used in code that executes concurrently with the body of globalFunc, so it needs to be marked @concurrent. That makes the requirements on captured local variables kick in. The "may execute concurrently with" predicate described in Preventing Data Races in the Swift Concurrency Model covers the rule here. (That proposal is mostly subsumed by this proposal; it just a reference now).
Interaction of Actor self and @concurrent closures
Most of this section is also covered in the recent revision of the actors proposal, which introduces cross-actor references and ties in to ConcurrentValue. I recommend cutting this section way down and referring over there, so the actor rules can be in one place.
That may sound like a lot, but it's fine tuning. I feel like we're getting close.
Doug