RFC: Educational notes for data-race safety errors

I've started to add educational notes for data-race safety errors in [Educational Notes] Start adding educational notes for data-race safety. by hborla · Pull Request #79509 · swiftlang/swift · GitHub

This initial PR contains educational notes for a handful of errors that were brought up in Feedback wanted: confusing concurrency diagnostic messages, including most of the region isolation errors. The educational note expands on what the problem is, and walks through common approaches to fixing the problem, similar to the Common Compiler Errors section of the migration guide.

I'd love some thoughts on these educational notes:

  1. Do they make sense? Is the writing style and terminology approachable? It's tricky to strike the right balance of casual language that contains enough technical information to understand the problem.
  2. Which other concurrency errors could use educational notes sooner rather than later? The end goal is, of course, for all error messages to have an educational note, but there are hundreds of concurrency error messages and thousands overall :slight_smile:

I look forward to your feedback!

-Holly

27 Likes

These notes are wonderful, and honestly I feel like I learned something just reading through them. And I think you strike the right balance in writing style. They feel approachable yet use the correct terminology, which will help the reader correlate to other documentation.

This set of notes feels like a really good start, hitting some of the most common concurrency errors. My one suggestion would be to cover errors around protocol conformances. It’s fairly common to take a MainActor isolated type and attempt to conform it to a nonisolated protocol. Especially when you don’t control the protocol.

One other minor suggestion: would it make sense to link to relevant swift evolution proposals notes? It might provide the reader the chance to dig deeper on an issue, but I suppose at the risk of overwhelming them.

3 Likes

Good idea! I'll add the protocol conformance errors next.

I'd rather not link to Swift Evolution proposals because they are typically much more dense than most programmers need, even to dig deeper into a particular language concept. However, I think these notes definitely can and should link to other resources like TSPL and the migration guide.

3 Likes

First of all, this is great! I've been wanting something like this for a long time! :fire:

What's the vision for educational notes? I believe no tool is rendering them at the moment, so what would be the expected format? Just slightly longer compiler messages? Or more like articles?

Personally I'd love to see richer media interleaved within the notes (diagrams, flows...), even if it's all complementary and the text-only version is still self-contained. I found the diagrams massively helped to explain the topic in the Swift Concurrency WWDC talks that used them. It seems to me like concurrency in particular is a topic that is hard to explain using words only. But I don't know whether that fits the concept of educational notes.

Quick mockup of what I'm thinking, taking some paragraphs from the sending-risks-data-race educational note as an example:

[...] This happens because the printNameConcurrently function runs off of the main actor, and the onMainActor function suspends while waiting for printNameConcurrently to complete. While suspended, the main actor can run other tasks that still have access to person , which can lead to a data race.

And this:

[...] This eliminates the risk of data-races because printNameConcurrently will continue to run on the main actor, so all access to person are serialized.

Of course, I'm not saying this should be done all the time, just that I think it'd be nice to leave that door open for some educational notes. Though I haven't seen many examples of images embedded in Swift's documentation, I wonder if that's by design (I couldn't find anything on the topic).


Yes! The style of the notes in the PR is spot on I'd say. Are the individual notes added meant to be complete? I haven't gone through all of them yet, but for the sending-risks-data-race.md educational note, given how many possible scenarios are there that would emit that educational note, I think there are other avenues (other than the new @execution(caller) attribute) that should be mentioned. Off the top of my head, some other generally useful approaches:

  • Making Person sendable should probably be the preferred way to do it if Person can trivially be made Sendable, so I think that should be one of the options in the note.
  • Annotating Person with @MainActor is another reasonable approach as well.
  • Actually sending the value to a different isolation region (probably not relevant to an instance method, but it could be useful for other use cases that run into the same error / educational note where it is possible to send the value with minor modifications).

I can think of similar feedback for other notes but I'm not sure if that's out of the scope of the feedback wanted at this point. If feedback should focus on the style only, I think it's really nice, no other comments from me on that front.


I'd love to help with this if possible, once the first concurrency-related educational notes are merged :eyes: What's the best way to let others know you're working on adding an educational note to an error, as to avoid working on the same thing as someone else? Just opening a issue on GitHub? Is there any other way this is coordinated?


I agree with this. Plus AFAIK the Swift Evolution proposals are not updated once they're implemented, so the linked SE proposals may contain code that no longer works in current Swift or mention limitations that are no longer there.

2 Likes

They make sense, and I can parse them pretty easily, but the terminology used to describe what's happening and how its related it pretty diverse across the notes, and gets a bit jargon-y in several places. The migration guide dialed in on talking about "isolation domains", but in most of these notes they're detailed as "actor-isolated" or variations on the theme, which I think could led someone not familiar with the errors or background to get a bit confused on the terms and how to refer to them.

Related to that, is there an option to include links out to external documentation "for more information"? In many cases, this short synopsis detail is perfect for someone who's already got it, but for someone just hitting it for the first time, a pointer to relevant detail in TSPL or the Swift 6 Migration guide would be a huge benefit.

As an example, "@MainActor" and "main actor" are used a interchangably, and I spotted a few places where "concurrency domain" and "isolation domain" are used, but I think are meant to indicate the same thing. I went ahead and made some comments and suggestions for wording changes in the PR, hopefully which help.

(I'm basing this on the idea that the person seeing this note is an active developer getting it as a response from the compiler, so they're in code and getting warnings or errors with additional detail explaining what's happening.)

That's correct, nothing is currently consuming educational notes; there's a discussion on that topic here: Surfacing educational notes for compiler errors

I envision these to be more like mini articles that provide more context about the error message and show various strategies for resolving the error. I think the most important quality is that they are actionable - programmers should understand their options for resolving an error and be able to determine which approach might be applicable in the context of their code.

I'd love to get started with text and code examples, but I'm definitely open to exploring different visualizations for concurrency errors. I also think visualizations would be valuable to incorporate into the migration guide articles.

They are definitely not complete. To start, I included what I think are the most common resolutions to specific error messages; you're right that there are more ways to encounter and fix region isolation errors. I was worried about making the note too long, but I think making the note a little longer might be alright if the note has more structure so that it's easy to scan for the information you're looking for.

Thank you for offering to help! I would love to collaborate :slight_smile: We can file a top-level GitHub issue for expanding educational notes coverage (I think one exists already that we can use), and file subtasks for specific subsets of diagnostics that we're each working on to track who is working on what. I'm also happy to organize an ad-hoc "workgroup" for tackling educational notes coverage if anybody else is interested in contributing!

3 Likes

Yeah, I can see having a good coverage is more important right now.

Oh, hadn't relized the migration guide was open to contributions :eyes:

IMO the notes that deal with many reasonable alternatives (like the Unsafe mutable global and static variables note) would benefit from structuring the different options in separate sections, even if it ends up being a bit longer. Something like:

Rough example based on Unsafe mutable global and static variables

Unsafe mutable global and static variables

Concurrency checking prohibits mutable global and static variables that are nonisolated because they can be accessed from arbitrary concurrency domains at once and lead to data races.

For example, building this code with complete concurrency checking will point out the unsafe static variable.

struct Constants {
    static var value = 10 // ❌ error: static property 'value' is not concurrency-safe because it is nonisolated global shared mutable state
}

Change var to let

If the type of the variable conforms to Sendable and the value is never changed, the most common fix is to change the var to a let to make the state immutable. Immutable state is safe to access concurrently!

struct Constants {
    static let value = 10 // ✅
}

Use @MainActor to isolate accesses to the main actor

If mutable state is needed, another option is to isolate the enclosing type to the main actor, which will protect access to all mutable state:

@MainActor struct Constants {
    static var value = 10 // ✅
}

Alternatively, if you don't want to isolate the entire type to the main actor, it's also possible to use @MainActor to isolate individual properties.

Use nonisolated(unsafe) if access has been made safe through other mechanisms

If you carefully access the global variable in a way that cannot cause data races, such as by wrapping all accesses in an external synchronization mechanism like a lock or a dispatch queue, you can apply nonisolated(unsafe) to opt out of concurrency checking:

struct Constants {
    nonisolated(unsafe) var value = 10 // ✅ Careful, though!
}

IMO it's easier to visually parse (despite being longer) than a shorter version where the options aren't as clearly delimited. One can do a quick scan of the titles and quickly jump to the case that is relevant to them. Just a thought :man_bowing: would love to know what other people find easier to read/parse.


On the topic of notes becoming too long, a couple ideas:

  • Maybe some notes can be split? I was mildly surprised that the Unsafe mutable global and static variables note touches the problem of global let with non-Sendable type, given there's a separate compiler error for that. I would have expected a separate note as well (I see how the two issues are related, but maybe a single comment about let + non-Sendable in the first option is enough if there is a fallback compiler message + educational note about that specific problem?).
  • Maybe there could be links between notes? On top of the previous point, that note also mentions using @MainActor at the type level vs @MainActor at the property level, which is a topic all by itself (and judging by the threads in this forum, a common pain point when gotten wrong). Maybe linking to another note on that topic could be a solution?

Hmm I could only find this issue about educational notes coverage.

Yeah, that's the issue I was thinking of. Instead of re-purposing that issue, I closed it and filed ☂️ Expand educational notes coverage · Issue #79638 · swiftlang/swift · GitHub along with a few initial subtasks. I will add other subtasks for specific subsets of diagnostics using this issue so that other folks can explore all available subtasks and follow progress of the effort in one place.

2 Likes