Pitch #4: ConcurrentValue and @concurrent closures Evolution Pitches

[John_McCall]

I don't really see any acceptable option except that Error implies ConcurrentValue. Anything else is going to create a massive static/dynamic problem for people whenever they need to move a computation result between threads.

[michelf (Michel Fortin)]

Perhaps in Swift 5 we could make Error implicitly conform to ConcurrentValue, but emit a warning instead of an error if one of the members is not itself a ConcurrentValue. You could silence the warning by conforming it to UnsafeConcurrentValue (or by fixing the members).

I guess that also means NSError would permanently need to be an UnsafeConcurrentValue due to its userInfo member of type [String: Any].

[Douglas_Gregor]

@Chris_Lattner3

@rjmcall I see your post downthread, does this make sense to you?

Edit : After wrestling with this, I agree with you both. I will changed this in the proposal, thank you for clarifying this!

(This regards @concurrent closures not needing to check that their parameter and result types are ConcurrentValue)

I agree, and thanks to Jordan and John for explaining it thoroughly.

@Chris_Lattner3

@Douglas_Gregor

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'm ok to lock this down to be conservative in the first revision (like we did with the first version of DynamicMemberLookup ), but I think this is the wrong way to go for several reasons:

I think DynamicMemberLookup was a different case; we did it because we were worried about abuse, but it didn't really have other benefits. Your first two points are interesting, because I think we're starting from the same examples and coming to different conclusions. Let's take two structs:

class MyClass { }

public struct A {
  public var x: Int
  private var y: Int
}

public struct B {
  public var x: Int
  private var y: MyClass
}

There are a couple of things that should be facts we can agree on:

• If we make A and B conform to ConcurrentValue within the same source file they are declared, A: ConcurrentValue will be well-formed and B: ConcurrentValue will be an error because MyClass does not conform to ConcurrentValue.
• If we make A and B conform to ConcurrentValue outside of the source file, we cannot see the private stored properties named y so the structural rule that the stored properties of a ConcurrentValue struct are themselves ConcurrentValue cannot be fully checked. If we do the check, it would have to conclude that both A and B conform to ConcurrentValue correctly.
• With resilience, A can evolve to have the stored properties of B but the conformance to ConcurrentValue will be ill-formed if it was in the source file.
• With resilience, B can evolve to make its property y public. That would make an out-of-source-file conformance B: ConcurrentValue ill-formed.

My take on this is that allowing a ConcurrentValue conformance outside of the source file means we're admitting a hole into the basic model (because B: ConcurrentValue is accepted despite the stored property of type MyClass) without a corresponding "unsafe" tag on it.

@Chris_Lattner3

@Douglas_Gregor

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.

I'm ok with this if you feel strongly about this, but I think it would be conservatively fine to just not allow classes to conform to ConcurrentValue in the first proposal (requiring them to use UnsafeConcurrentValue). Such a conformance has strange interactions with resiliences - for example, you can't add this to open class since you don't know if all the subclasses will be ConcurrentValues and therefore adding it may break code. It isn't unworkable, just non-obvious.

I suppose that means I do feel a bit strongly about this.

@Chris_Lattner3

We need a way to deal with imported C and C++ APIs, and I don't see how marking all conformances as "unsafe" is necessary. Perhaps we can make the importer "do the right thing" in all cases that this isn't a concern though?

This is a great point. We can do this because C doesn't have resilience, inferring ConcurrentValue conformance for all C enums and C structs whose stored properties are ConcurrentValue.

@Chris_Lattner3

@Douglas_Gregor

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.

Yeah this is cool, are you ok with further limiting this to the case where the class is final? If so, that seems obvious to me.

That said, I'm ok both ways, so if you feel strongly for this, let's do it! Just let me know.

I'd be okay with limiting ConcurrentValue to final classes. We can open it up later if it matters.

@Chris_Lattner3

I agree, I think this should just be handled in the patch, but I don't think the proposal should nail it all down. I agree completely with your list, lemme highlight these two:

@Douglas_Gregor

The exceptions are:

...

• 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.

Again, I agree with you. Do you think it would be good to call these out in the proposal, or do you want to include a full list?

The full list would be tedious. I think taking the list roughly as I wrote it in my post is the right level of detail.

@Chris_Lattner3

@Douglas_Gregor

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.

This makes me uncomfortable for a variety of reasons, including unpredictability and "spooky action at a distance". What do you think about just requiring explicit capture list for the first round of proposal? We can loosen this up in the future with more usage experience.

It can certainly be loosened in the future, so yeah, I'm fine with requiring an explicit capture list for mutable state.

@Chris_Lattner3

@Douglas_Gregor

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.

That makes sense to me, but doesn't this fall out of the capture rules? In your example, you're capturing local, so of course it needs to be a ConcurrentValue . Maybe the compiler isn't modeling it this way?

You're right, it does fall out of the capture rules. Yay simplicity.

@Chris_Lattner3

@Douglas_Gregor

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.

Yes, I agree that this makes more sense to live in the actors proposal. I moved it to future work but can drop it entirely if you prefer.

Future work is fine by me.

@Chris_Lattner3

@John_McCall

This is a problem independent of the root/value type issue, and I see two ways to solve it:

• Introduce the concept of both concurrent and non-concurrent key paths.
• Forbid capturing non-concurrent values in key paths.

Since capturing values in key paths is relatively uncommon in the first place, and it seems extremely likely that vanishingly few captured values are of non-concurrent types rather than types like Int and String , I strongly favor the second option.

I agree, the second approach seems best to me. Doug, let me know how you want to revise the stdlib types in general.

Yes, forbidding capture of non-concurrent values in key paths seems like the best approach here.

@John_McCall

I don't really see any acceptable option except that Error implies ConcurrentValue . Anything else is going to create a massive static/dynamic problem for people whenever they need to move a computation result between threads.

I think this is the only sound thing to do, but for the pesky problem that we'll be breaking source code to do it. We could perhaps stage it in via warning: if a type gets its ConcurrentValue conformance through Error, we warn about stored properties or associated values that aren't ConcurrentValue rather than error. Or perhaps we do the check only when there is a need for that particular conformance, so one only sees the diagnostic if you're writing concurrent code, which leaves a hole but won't be noisy in existing code.

Either way, we'd turn it into an error in Swift 6 to tighten things up.

@michelf

I guess that also means NSError would permanently need to be an UnsafeConcurrentValue due to its userInfo member of type [String: Any] .

Yeah, there's nothing we can do to make NSError non-unsafe. I think that's acceptable.

Doug

[michelf (Michel Fortin)]

@Chris_Lattner3

In the case of @concurrent closures, I think there is nothing to do here: thrown errors are equivalent to returning a Result, and result values don't need to conform to ConcurrentValue per the logic above.

This is a serious issue with actor memory safety though!

Giving more thought to this, I think this is also a problem for async let, runDetached, and task groups from structured concurrency. An error thrown crosses the bridge between two executors in those cases. Of course, forcing Error to conform to ConcurrentValue would work there too.

And the return type should be constrained to ConcurrentValue too for the cases above. runDetached could for instance add a generic constraint to ensure the returned value conforms to ConcurrentValue. (That wasn't necessary when @concurrent implied ConcurrentValue for parameter and return types, but now it will be.)

[John_McCall]

@Douglas_Gregor

I think this is the only sound thing to do, but for the pesky problem that we'll be breaking source code to do it. We could perhaps stage it in via warning: if a type gets its ConcurrentValue conformance through Error , we warn about stored properties or associated values that aren't ConcurrentValue rather than error.

Yes, that seems reasonable. Really, this should be the rule for any standard-library protocol that gains a ConcurrentValue conformance; Error may be the first, but it might not be the only one.

@Douglas_Gregor

Or perhaps we do the check only when there is a need for that particular conformance, so one only sees the diagnostic if you're writing concurrent code, which leaves a hole but won't be noisy in existing code.

This is unfortunately not really possible, since we'll use the fact that Error implies ConcurrentValue pervasively, and the Error conformance of the specific type will be used if it's ever thrown, which I think we can safely assume will happen somewhere.

[Douglas_Gregor]

@John_McCall

@Douglas_Gregor

I think this is the only sound thing to do, but for the pesky problem that we'll be breaking source code to do it. We could perhaps stage it in via warning: if a type gets its ConcurrentValue conformance through Error , we warn about stored properties or associated values that aren't ConcurrentValue rather than error.

Yes, that seems reasonable. Really, this should be the rule for any standard-library protocol that gains a ConcurrentValue conformance; Error may be the first, but it might not be the only one.

Right, we can do this for CodingKey as well, which lets a bunch of Codable-related stuff conform to ConcurrentValue.

@John_McCall

@Douglas_Gregor

Or perhaps we do the check only when there is a need for that particular conformance, so one only sees the diagnostic if you're writing concurrent code, which leaves a hole but won't be noisy in existing code.

This is unfortunately not really possible, since we'll use the fact that Error implies ConcurrentValue pervasively, and the Error conformance of the specific type will be used if it's ever thrown, which I think we can safely assume will happen somewhere .

It would never work well, no. This is something where we'll need a source break in "Swift 6" to tighten up the model.

Doug