On improving communication between proposal authors and reviewers

Even as I've been following the majority of ongoing discussions, I can't figure out the context of the "issue" you're referring to and can't speak to what's going on, so I have to go by your comments in this thread.

Swift Evolution is specifically for user-facing changes to the language, allowing members of the community who are users to weigh in on how those changes would impact them. This provides instantly an answer to one of the most important questions when writing any text: Who is the target audience I'm writing for?

Authors of proposals should try to write in such a way that the greatest number of possible users of their proposed feature (not necessarily users of Swift generally) could give useful feedback about how any user-facing changes would affect them. Any text that is necessary to accomplish that goal should be included, and any text that's not necessary to accomplish that goal should be excluded.

The details of how a feature is to be implemented obviously dovetail with how the user-facing aspects are designed, but proposal authors don't have to explain their implementations beyond what's necessary to understand the user-facing design itself. Indeed, although an implementation is now required before a proposal proceeds to review, further iterations of the implementation occur after acceptance; in fact, there's no requirement that even a single line of code from the original implementation remain a part of the implementation that's ultimately merged.

Implementations of many features are going to be necessarily more complicated than the features themselves. Adding text, even links, about the implementation when such knowledge isn't necessary to understand the proposal is going to alienate users who, as @hborla describes, are fully competent Swift programmers and would likely have useful comments to make, but who may not be up on compiler implementation trivia and who don't have to be.

On the flip side, while educating others on how to implement a compiler feature is certainly useful and worthwhile, I think it's not appropriate to pile this on proposal authors. Implementers are expected to engage with code reviewers about what they want to merge; expecting implementers additionally to field informational questions from folks with varying familiarity with compiler internals is something that they haven't signed up for.

In general, better communication is about meeting each other in the middle, requiring an effort on both sides. Adding requirements produces more communication, but not necessarily better communication: the only thing for certain is that it makes the journey longer on both sides, making it harder and not easier to meet each other in the middle.

One addition to Swift proposals that I've advocated for previously is to bring in a section titled "How do we teach this?" (which I believe was added to Rust RFCs, or another "modern" programming language's analogous procedure). Obviously, such a section should spell out in sufficient detail how to use the proposed feature for a programmer otherwise generally familiar with Swift; part of the process of reviewing a feature would be explicitly evaluating whether it's sufficiently "teachable," and questions about understanding the feature would serve a double purpose during review of clarifying the proposal text and improving a feature's teachability.

15 Likes

I agree with this, the second quote in particular.

I'm looking at "Proposed Solution" and "Detailed Design" in the existing template at 0000-swift-template.md. Neither of those sections really covers the teachability issue based on their descriptions. "Proposed Solution" is close but the focus is slightly different.

It might be kind of costly to add a new section to the template though, especially if it duplicates in part an existing section. Does it make sense to adjust the guidance for "Proposed Solution" to try to focus it on teachability?

Actually writing the proposed solution in a way that is clear and teachable is probably more work than a lot of the other stuff we have talked about. In the discussion phase it's easy to write a lot of words and just to keep adding as people discuss.

To make it teachable you have to really shrink it down. You can compare how short the sections are in The Swift Programming Language. I assume they got that way by being rewritten over and over again, which took a lot of time and effort.

I'm not sure how you'd write guidance that said to rewrite the section over and over again aiming to make it concise though.

I feel like the "Proposed Solution" section should be structured and concise enough that it tells a Swift programmer what they need to know to use the feature. That might also be something to add to the guidance.

Then the guidance for "Detailed Design" could reflect things that a working Swift programmer
does not have to bring to the front of their mind while using the feature, even if they know about it.

Adding isPower(of:) to BinaryInteger is a pretty good example. The "Proposed Solution" section is how somebody would use this and what gets returned and when. The "Detailed Design" section talks about the fast and slow paths and details for different cases.

1 Like

I think this should be the goal of Detailed Design. This sentence in the template seems to place focus in the wrong area:

The detail in this section should be sufficient for someone who is not one of the authors to be able to reasonably implement the feature.

I mean, sure, other compiler engineers should be able to figure out how to implement the feature, but IMO that's not really the purpose of Detailed Design from the reviewers standpoint. In my opinion, the purpose of Detailed Design is to describe the syntax and semantics of the feature for users of the feature. For example, this is exactly what the Detailed Design of SE-0258 does. Of course, it talks about compiler-synthesized properties, but that's a fundamental part of the property wrapper mental model for users.

Maybe one of the questions in the review template should specifically ask something along the lines of "what questions do you have about the proposed feature?". I'd want to be careful about phrasing to avoid reviewers feeling self conscious about their own learning process. Like, if the question were "how easy was it for you to understand the feature", someone might feel really bad about saying it was hard for them to understand.

8 Likes

I think this would be helpful, and I'd prefer this over having two parallel threads per review. Questions are valuable feedback in that they highlight which areas are unclear or under-developed, so I'd be wary about relegating them to a separate thread.

3 Likes

I can agree with that only to a certain extent. Of course I wouldn't expect authors to discuss, dunno, the lines of code added to the typechecker, but as soon as code generation is involved, things change quite a bit. Perhaps that's how I personally learn or use the language, but I would be interested in at least the high-level details on how protocol witness tables work if I were to review a proposal that adds existential types to the language because this affects performance — and I view (the high-level aspect of) performance as part of user-facing design. I'm obviously not sure how a proposal would talk about that without authors needing to write a separate book, so I resorted to the least intense suggestion of "just link to something" if that something exists, even if it's a bit unapproachable.

But I recognize that this might be alienating and defeat the purpose. I would prefer such information to be included nevertheless, but the understanding of such should definitely worded as non-binding. Still, if I as a reviewer do happen to understand that in the end, that might contribute to me being able to produce much more targeted discussion, which was part of the "issue" I decided to only vaguely refer to for the sake of not disclosing any identities.

This passage from @hborla reiterates on my idea and how I would like to see it.

Putting aside that existential types already exist in the language, I disagree with this completely: Protocol witness tables are an implementation detail that shouldn't ever need to be mentioned in any Swift Evolution proposal for readers to understand a user-facing feature; otherwise, protocol witness tables would cease to be only an implementation detail.

And by the same token, authors of a proposal shouldn't ever feel obligated to write up anything about compiler internals. In fact, I would go so far as to say that if a feature can't be explained without surfacing a compiler-internal concept, then that should be a red flag to the author that the feature's design is leaking internal implementation details into the user-facing model that it shouldn't, and it should be rethought.

5 Likes

Right, I'm using existential types as an example of an already existing feature that I can already look into the implementation of to discuss it less theoretically, but that could be anything.

My perception of high-level languages is different though. The extremely user-facing observation that (an operation on) [Int] is much faster than [Intable] (for a theoretic Intable) still unavoidably leaks something about their implementation, regardless of how much we want it to be it an implementation detail merely. Users do not write code for abstract machines, they write code for very concrete CPUs that have cache lines of precise widths, and the way I regard high-level languages is as a tool that "just" helps me not write assembly, not as a tool which makes assembly non-existent.

That is, if I were to encounter a proposal which adds some runtime facilities, be they intended as implementation details or not, I would want to know about them. I, as a user, would want to be able to assess how much potentially this affects the performance of my code, what tradeoffs there are etc. — perhaps just on the level of Big-O — because that would help me make the decision whether I even want such modification to the language. An abstract talk about existential types wouldn't help me in that assessment; I would want to know what actually would be generated by the compiler and ultimately run on my CPU — again, in broad strokes, but still.

The Swift Evolution process does not govern all aspects of the Swift open source project, not even all aspects that would affect you as a user. Internal changes are made every day to the Swift compiler which affect the performance or even behavior of your code that aren't subject to Swift Evolution. For example, changing the encoding of native strings from UTF-16 to UTF-8 was a major overhaul simply announced after it was complete, while changing the sorting algorithm and the floating-point-to-string conversion algorithm were done without any fanfare.

By the same token, while users are perfectly justified in taking an interest in all aspects of a proposed feature, the review prompts specifically solicit your opinion of what's in the proposal. As a reviewer, you are not asked your opinion on details of implementation tradeoffs. That is not to say that others (such as the core team, or the code owners) won't consider those issues, just that it's not part of the reviewer's job in the Swift Evolution process.

4 Likes

Totally agree. That said, proposals do already link to the implementation PR, and anybody is welcome to give it a read, ask questions, and discuss implementation tradeoffs there. @wtedst I would encourage you to get more involved in code review given your interest in how features are implemented!

2 Likes

I'm starting to think that we are just pointing at two different things here. The one I was talking about is the issue of discoverability of the implementation as background information to help build a fuller mental model of the proposed feature, while you were worrying about the said implementation unnecessarily leaking into the discussion.

I feel that the latter is a non-issue: proposals already have mechanisms to specify what is purely contextual and what is really being proposed. I also feel that withholding supplementary details merely because they are subject to change only contributes to the chance of such gatekeeping to happen again, because this makes reviewers more reliant on ther experience and knowlegde of other languages with similar mechanisms to be able to "guess" how the proposed feature is going to work in Swift.

We generally can't assume how people acquire their mental model for a feature and what level of understanding they feel satisfied with, but we definitely want to facilitate building the highest possible level of understanding. High-level implementation details (or implications thereof) are still part of a feature's semantics (for instance, dynamic dispatch in protocol types; I'm going to continue using this analogy as it is technically dense enough to resemble the topic that originated this thread). There are only two ways that I see to positively affect a potential reviewer who doesn't yet have a solid understanding of the concept of dynamic dispatch:

  • Require proposal authors to explain what this is. This is a non-goal because of the sheer volume of work, even though this theoretically aligns with your sentiment that it's the authors' job to ensure proper understanding. There's no general prescription as to what to consider "necessary for understanding", so there'll anyways be some information left out. This is unavoidable, but doesn't need to be solved by writing a book on dispatch strategies alongside each proposal either.
  • Give them some means to learn on their own that it's something about hopping over a couple of pointers stored alongside the value to find the implementing function in the binary. The exact ABI doesn't matter, but the understanding of how it generally works greatly contributes to the reviewer's ability to, for example, reason about the implementability of changes that they might want to suggest during the review. This is the communication improvement I'm talking about.

Everything else would involve essentially telling them to "come back when you're educated", either directly in the discussion or by writing the proposal in such a way that makes it impossible to diagnose what they could read up on to catch up on the topic before the review ends.

I also feel that restricting access to the implementation is gatekeeping in and of itself: either it makes me feel that it was hidden from me because I'm too stupid to understand it, or because I'm too stupid to understand that it's ephemeral matter. I think that people need a way to be able to judge for themselves how they want to approach discovering the proposal's matter, be it digging through the compiler code or listening to a linked talk or whatever. Telling a reviewer that something is not a part of the proposal has much smaller cost than refusing to educate people when it's easy to provide them with means to do it essentially autonomously.

Interestingly enough, a thread was bumped recently, which is quite precisely how I envision what such section could be comprised of. I don't think that's asking for much.

The most common question that I have on proposals is "...but how do you actually make this work?", which is explicitly outside the scope of what a review thread is supposed to be for, and yet, understanding it is generally pretty central to my overall understanding of the feature in question. (Whatever the level of abstraction, I want to be able to know how it maps to what's directly below it in order to feel I have an informed opinion.)

I've previously tried to find references on coroutines in the context of both _modify accessors and async/await and unfortunately run into a lot of "well, there are a lot of ways to implement them" type vague answers, and somehow missed finding that John's talk existed.

So: (a) Thanks @wtedst for the link, that was very helpful, and (b) I, at least, would absolutely find pointers to applicable references to be incredibly useful. Maybe not in a "Prior Knowledge" way for reviews, but absolutely in a "If You Want To Know More" way.

2 Likes

Thanks for confirming that the accessibillity of such material is helpful; I would love to hear if other people benefitted from having something like that at hand when reviewing a proposal.

I think we, programmers, are just overly optimistic in trying to make a language look like pure syntactic sugar over a CPU. There's a distinction between an implementation detail that is truly a detail, versus something that makes a feature fundamentally possible. Both are not parts of a proposal itself, but it's hard to reason about a proposal without understanding the latter.

1 Like

I do agree that there are cases where knowing what the compiler does/what happens at runtime is necessary to understanding the semantics of the feature. I keep using code-generative features (e.g. property wrappers and result builders) as an example, because for such features it's important to understand what code the compiler is generating for you (and also because I love compile time :slightly_smiling_face:). For those features, it is important to have that description in Detailed Design.

I would like to point out again that deliberately withholding materials or details about the implementation is a nongoal.

Also, please keep in mind that proposals are not written perfectly the first time! When you're super deep into the technical details of a proposal, it's really easy to forget what is "common knowledge" to somebody who is not as deep into the details as you are. If details are missing from the proposal, it probably was not deliberate - proposal authors do rely on evolution participants to help identify missing details.

I mentioned participating in code review of the implementation, but I also think it's perfectly appropriate to start a separate thread here on the forums to discuss the implementation of a language feature if you're interested in better understanding how it works! The Compiler Development category would be the perfect place for such a thread. This would also be a great place to crowdsource finding resources for certain PL or compiler concepts. Again, I don't think it's a bad idea to include some in a proposal, e.g. in an appendix, but the proposal itself can only include so much information.

If somebody is telling you (or someone else) to stop participating because you're uneducated or incompetent, please involve one of the forum moderators.

7 Likes

Emphasizing this, I recently got feedback from my team that a proposal I was working on was lacking some important background information. I had actually intentionally excluded it in the interests of keeping the proposal brief and readable, but everyone is much happier now that I included it.

4 Likes

Thanks for keeping me reassured. I perhaps should have quoted the precise wording that made me believe that it is being suggested that supplying some knowledge on the implementation would only steer the discussion in a wrong direction, and I wanted to argue that it most likely won't.

I actually think these sorts of codegen features are a great example of how we generally distinguish implementation details from the language model during discussion. When we say that the following code:

@Wrapper
var x: Int

generates/desugars to:

var _x: Wrapper<Int>
var x: Int {
  get { _x.wrappedValue }
  set { _x.wrappedValue = newValue }
}

we don't actually mean that the compiler goes to all the work to generate the stream of bytes above and run it through the lexer and parser to produce an AST, we just mean that the language model of the feature is such that it is as if the compiler has done exactly that (even if the implementation skips some steps by generating the AST directly).

4 Likes

I believe that's a point I was trying to make initially:

Code generation is what affects the final product (the binary) and (I hope we agree that) it is important to understand in what ways. That comment of mine was met by the following responses:

— which to me reads like "it's cute that you are interested in the codegen, but you shouldn't worry about that and give unsolicited opinion on what the smarter people will figure out eventually". I'm very sorry if that wasn't the intended tone, so please let me know if I totally misunderstood the message, but at least to me it is paramount to have an idea of what's being generated because otherwise the language is simply unusable: an abstract discussion of what is being enabled that is devoid of at least a mention, not necessarily a discussion, of potential runtime implications would just prompt me to never consider using that feature at all.

Again, I apologize that it's getting into such minutia, but I wanted to make sure that it's really desired to have some info on that. It is already hard to get that kind of information on well-established features, even more so on newer features within the timeframe of a proposal pitch/review.

I don't think this was the intended message, and I absolutely do not want community members to feel like they should not learn about implementation details if they're interested in doing so.

I think I have been pretty clear that I'm encouraging folks who are interested to discuss the implementation more, and that those of us who work on the compiler are happy to share knowledge. I think the message is that the review thread is not the best place for an in-depth exploration/discussion of the implementation approach. Yes, some high-level details do belong in the proposal, but there are many people who like to have a deeper understanding. I really like the idea to discuss these details in greater depth on a separate thread in Compiler Development if folks want to dig deeper than the abstraction level in the proposal proper, and that thread could even be linked to in the proposal. Then, anybody can offer up their knowledge or suggest resources without having to put up a PR to swift-evolution.

6 Likes

Agree with the above completely.


It's important to distinguish a language's design from its implementation. C isn't Clang, Javascript isn't V8, and Swift (the language) isn't Swift (the compiler). Swift Evolution proposals are about the language's design, and the language simply isn't designed as sugar for assembly even though the compiler generates native code. Recall that, semantically, values are copied each time they are bound to another variable or passed to a function; whether any particular copy is elided or not is an implementation detail. Similarly, the existence of a small string optimization is an implementation detail, and so on and so forth. The compiler's implementation is open source (and there's even a discussion forum about it hosted on this very site!), and the folks who work on it full time are incredibly welcoming and helpful, but the implementation is not subject to the Swift Evolution process.

3 Likes
Terms of Service

Privacy Policy

Cookie Policy