Browse Source

Keep design documents current (#5606)

Require language design proposals to either update the design documents
to
reflect the proposed changes, or add "TODO" comments to mark where those
changes
will be needed, with links back to the proposal. This is intended to
ensure that
the design documentation accurately informs readers about the current
language
design, without excessively burdening the proposal process.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Geoff Romer 7 tháng trước cách đây
mục cha
commit
c713279a3d

+ 3 - 0
docs/design/README.md

@@ -862,6 +862,9 @@ or restrictions on casts between pointers and integers.
 
 ### Arrays and slices
 
+> **TODO:** The provisional array syntax documented here has been superseded by
+> [p4682: The Core.Array type for direct-storage immutably-sized buffers](/proposals/p4682.md).
+
 The type of an array of holding 4 `i32` values is written `[i32; 4]`. There is
 an [implicit conversion](expressions/implicit_conversions.md) from tuples to
 arrays of the same length as long as every component of the tuple may be

+ 4 - 0
docs/design/classes.md

@@ -752,6 +752,10 @@ class GraphNode {
 **Open question:** What is specifically allowed and forbidden with an incomplete
 type has not yet been decided.
 
+> **TODO:** Document that qualified names can be looked up in an incomplete
+> type, as adopted in
+> [p5087: Qualified lookup into types being defined](/proposals/p5087.md).
+
 ### `Self`
 
 A `class` definition may provisionally include references to its own name in

+ 6 - 0
docs/design/expressions/member_access.md

@@ -33,6 +33,12 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 ## Overview
 
+> **TODO:** [p3720: Member binding operators](/proposals/p3720.md) introduces an
+> additional "member binding" step, redefines simple member access in terms of
+> compound member access, and defines compound member access in terms of calls
+> to user-implementable interface methods. This document must be updated to
+> reflect those changes.
+
 A _qualified name_ is a [word](../lexical_conventions/words.md) that is preceded
 by a period or a rightward arrow. The name is found within a contextually
 determined entity:

+ 7 - 0
docs/design/functions.md

@@ -24,6 +24,13 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 ## Overview
 
+> **TODO:** Update this document to reflect the introduction of function values,
+> function types, and the `Call` interface in
+> [p2875: Functions, function types, and function calls](/proposals/p2875.md).
+
+> **TODO:** Update this document to reflect the changes to named functions in
+> [p3848: Lambdas](/proposals/p3848.md).
+
 Functions are the core building block for applications. Carbon's basic function
 syntax is:
 

+ 58 - 0
docs/design/generics/details.md

@@ -447,6 +447,10 @@ impl Point_ExtendForward as Vector {
 }
 ```
 
+> **TODO:** The second `impl` in this example is no longer a valid redeclaration
+> of the first after
+> [p5366: The name of an `impl` in `class` scope](/proposals/p5366.md).
+
 More about forward declaring implementations in
 [its dedicated section](#declaring-implementations).
 
@@ -929,6 +933,9 @@ constraint DrawVectorLegoFish {
 }
 ```
 
+> **TODO:** Document that `Self` can be omitted, as adopted in
+> [P5337: Interface extension and `final impl` update](/proposals/p5337.md).
+
 In general, Carbon makes no syntactic distinction between the uses of named
 constraints and interfaces, so one may be replaced with the other without
 affecting users. To accomplish this, Carbon allows a named constraint to be used
@@ -1249,6 +1256,10 @@ fn DoHashAndEquals[T:! Hashable](x: T) {
 
 ### Interface extension
 
+> **TODO:** Update this section as needed to reflect the fact that an impl of an
+> interface doesn't impl the interfaces it extends, as adopted in
+> [p5168: Forward `impl` declaration of an incomplete interface](/proposals/p5168.md).
+
 When implementing an interface, we allow implementing the aliased names as well.
 In the case of `Hashable` above, this includes all the members of `Equatable`,
 obviating the need to implement `Equatable` itself:
@@ -1276,6 +1287,9 @@ benefits:
 We expect this concept to be common enough to warrant dedicated `interface`
 syntax:
 
+> **TODO:** Update this section to reflect the new syntax adopted in
+> [p5337: Interface extension and `final impl` update](/proposals/p5337.md).
+
 ```carbon
 interface Equatable { fn Equals[self: Self](rhs: Self) -> bool; }
 
@@ -1326,6 +1340,9 @@ interface SetAlgebra {
 }
 ```
 
+> **TODO:** Document `extend [final] impl as I`, as adopted in
+> [p5337: Interface extension and `final impl` update](/proposals/p5337.md).
+
 **Alternative considered:** The `extend` declarations are in the body of the
 `interface` definition instead of the header so we can use
 [associated constants](terminology.md#associated-entity) also defined in the
@@ -1434,6 +1451,9 @@ interface MovieCodec {
 
 #### Diamond dependency issue
 
+> **TODO:** Update this section to reflect the changes in
+> [p5168: Forward `impl` declaration of an incomplete interface](/proposals/p5168.md).
+
 Consider this set of interfaces, simplified from
 [this example generic graph library doc](https://docs.google.com/document/d/15Brjv8NO_96jseSesqer5HbghqSTJICJ_fTaZOH0Mg4/edit?usp=sharing&resourcekey=0-CYSbd6-xF8vYHv9m1rolEQ):
 
@@ -1986,6 +2006,10 @@ keyword `private` before `adapt`, so you might write
 
 ## Associated constants
 
+> **TODO:** Update this section to reflect the new rules and guidance on
+> associated constants in
+> [p5168: Forward `impl` declaration of an incomplete interface](/proposals/p5168.md).
+
 In addition to associated methods, we allow other kinds of
 [associated entities](terminology.md#associated-entity). For consistency, we use
 the same syntax to describe a compile-time constant in an interface as in a type
@@ -2112,6 +2136,10 @@ Together associated methods and associated class functions are called
 _associated functions_, much like together methods and class functions are
 called [member functions](/docs/design/classes.md#member-functions).
 
+> **TODO:** Document rules on where associated function implementations can be
+> declared, as adopted in
+> [p5168: Forward `impl` declaration of an incomplete interface](/proposals/p5168.md).
+
 ## Associated facets
 
 Associated facets are [associated constants](#associated-constants) that happen
@@ -4466,6 +4494,9 @@ difference.
 
 #### Prioritization rule
 
+> **TODO:** Document the changes to prioritization adopted in
+> [p5337: Interface extension and `final impl` update](/proposals/p5337.md).
+
 Since at most one library can contain `impl` definitions with a given type
 structure, all `impl` definitions with a given type structure must be in the
 same library. Furthermore by the [`impl` declaration access rules](#access),
@@ -4864,6 +4895,10 @@ class Optional(T:! type) {
 // ❌ Illegal: impl Optional(i32) as Deref { ... }
 ```
 
+> **TODO:** Update the following passage to reflect the relaxed overlap rule
+> adopted in
+> [p5337: Interface extension and `final impl` update](/proposals/p5337.md).
+
 This prevents any higher-priority impl that overlaps a final impl from being
 defined unless it agrees with the `final` impl on the overlap. Overlap is
 computed between two non-`template` `impl` declaration by
@@ -4996,6 +5031,10 @@ differences between the Carbon design and Rust plans:
 
 ## Forward declarations and cyclic references
 
+> **TODO:** Update this section to distinguish between _defined_ and _complete_,
+> as adopted in
+> [p5087: Qualified lookup into types being defined](/proposals/p5087.md).
+
 Interfaces, named constraints, and their implementations may be forward declared
 and then later defined. This is needed to allow cyclic references, for example
 when declaring the edges and nodes of a graph. It is also a tool that may be
@@ -5014,6 +5053,10 @@ used.
 
 ### Declaring interfaces and named constraints
 
+> **TODO:** Update this section to reflect the additional things you can do with
+> a defined but incomplete type, as adoped in
+> [p5087: Qualified lookup into types being defined](/proposals/p5087.md).
+
 The declaration for an interface or named constraint consists of:
 
 -   an optional access-control keyword like `private`,
@@ -5100,6 +5143,9 @@ An incomplete `C` cannot be used in the following contexts:
 
 ### Declaring implementations
 
+> **TODO:** Update this section to reflect the new rules adopted in
+> [p5168: Forward `impl` declaration of an incomplete interface](/proposals/p5168.md).
+
 The declaration of an interface implementation consists of:
 
 -   optional modifier keyword `final`,
@@ -5115,6 +5161,9 @@ The declaration of an interface implementation consists of:
     [associated constants](#associated-constants) including
     [associated facets](#associated-facets).
 
+> **TODO:** Document the redeclaration syntax `impl C.(as I)` adopted in
+> [p5366: The name of an `impl` in `class` scope](/proposals/p5366.md).
+
 **Note:** The type before the `as` is required except in class scope, where it
 defaults to `Self` as described in the
 [matching and agreeing section](#matching-and-agreeing).
@@ -5154,6 +5203,11 @@ these rules:
 
 ### Matching and agreeing
 
+> **TODO:** Update this section to reflect the new terminology and rules adopted
+> in [p3763: Matching redeclarations](/proposals/p3763.md), and the new rules
+> adopted in
+> [p5168: Forward `impl` declaration of an incomplete interface](/proposals/p5168.md).
+
 Carbon needs to determine if two declarations match in order to say which
 definition a forward declaration corresponds to and to verify that nothing is
 defined twice. Declarations that match must also agree, meaning they are
@@ -5189,6 +5243,10 @@ expressions match along with
     an interface, as in `constraint Equivalent { extend MyInterface; }`, is not
     considered to match.
 
+> **TODO:** Document the matching rules for the redeclaration syntax
+> `impl C.(as I)` adopted in
+> [p5366: The name of an `impl` in `class` scope](/proposals/p5366.md).
+
 For implementations to agree:
 
 -   The presence of the modifier keyword `final` before `impl` must match

+ 30 - 3
docs/project/evolution.md

@@ -314,9 +314,36 @@ The author of a proposal is not required to include changes to the design
 documentation as part of a proposal, and it may in some cases be preferable to
 decouple the proposal process from updating the design. When accepted, the
 proposal would then be implemented through a series of future PRs to the rest of
-the project, and the proposal document should describe what is being proposed in
-enough detail to validate that those future PRs properly implement the proposed
-direction.
+the project. If the proposal PR defers any documentation changes in this way,
+the proposal document should describe what is being proposed in enough detail to
+validate that those future PRs properly implement the proposed direction, and
+the proposal PR should add "TODO" comments in the places where significant
+future changes will be needed (including adding new placeholder documents if
+needed), with links back to the proposal document. These comments should be kept
+close to the passages that need to be changed. This is intended to ensure that
+readers of the design documentation know when what they're reading is out of
+date, and can easily find out what has changed. For example:
+
+```md
+> **TODO:** Document the redeclaration syntax `impl C.(as I)` adopted in
+> [p5366](/proposals/p5366.md).
+```
+
+See the `docs/design` changes in
+[#5606: Keep design documents current](https://github.com/carbon-language/carbon-lang/pull/5606)
+for additional examples of adding those comments. As with other parts of the
+proposal, these comments should be included in the initial PR, and updated as
+needed during the review.
+
+As an exception, proposals that will have a widespread, pervasive effect on the
+design documents (such as proposals to replace widely-used vocabulary) usually
+shouldn't make those changes as part of the proposal, so that changes during the
+review aren't an excessive burden on the proposal author. Such proposals usually
+shouldn't apply "TODO" comments either, because pervasive "TODO" comments are
+more likely to be disruptive than helpful to the reader. Instead, the proposal
+author should file a GitHub issue to track the need for documentation updates,
+and include a link to it in the proposal document, and then follow up with pull
+requests to make the changes after the proposal is approved by the leads.
 
 #### Open questions
 

+ 160 - 0
proposals/p5606.md

@@ -0,0 +1,160 @@
+# Keep design documents current
+
+<!--
+Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+Exceptions. See /LICENSE for license information.
+SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+-->
+
+[Pull request](https://github.com/carbon-language/carbon-lang/pull/5606)
+
+<!-- toc -->
+
+## Table of contents
+
+-   [Abstract](#abstract)
+-   [Problem](#problem)
+-   [Proposal](#proposal)
+-   [Details](#details)
+-   [Rationale](#rationale)
+-   [Alternatives considered](#alternatives-considered)
+    -   [Require documentation updates as part of the proposal](#require-documentation-updates-as-part-of-the-proposal)
+    -   [One TODO per document](#one-todo-per-document)
+    -   [Link to the PR instead of the proposal document](#link-to-the-pr-instead-of-the-proposal-document)
+
+<!-- tocstop -->
+
+## Abstract
+
+Require language design proposals to either update the design documents to
+reflect the proposed changes, or add "TODO" comments to mark where those changes
+will be needed, with links back to the proposal. This is intended to ensure that
+the design documentation accurately informs readers about the current language
+design, without excessively burdening the proposal process.
+
+## Problem
+
+The current evolution process allows us to adopt language design changes while
+deferring the corresponding changes in `/docs/design`. We have no process for
+ensuring that those follow-up changes actually happen, and in practice some
+adopted proposals have gone for well over a year without corresponding design
+document changes.
+
+This problem is not limited to proposals for new features, but also applies to
+proposals to change existing, documented features. As a result, the design
+documents are not merely incomplete, but in some cases actually misleading,
+which has led to miscommunication and wasted effort within the Carbon team.
+
+## Proposal
+
+This proposal modifies our evolution process to require proposals to either
+implement the corresponding changes to the design documents, or mark the places
+that will need to be changed with "TODO" comments that point back to the
+proposal. The presence of those comments should help ensure that readers are not
+misled by the design documentation, and the links to the proposals should help
+ensure readers can discover the actual design for a given feature with
+reasonable effort.
+
+The proposal PR also adds "TODO" comments to `/docs/design`, both as an
+illustration of what this policy asks for, and because the policy needs to be
+applied retroactively in order to actually solve the problem. However, these
+changes are necessarily best-effort, because it wasn't feasible for me to fully
+evaluate every proposal against the current state of the docs. In particular, I
+didn't look at proposals numbered below 2000, and I assumed that proposals fully
+updated `/docs/design` if they touched it at all. In addition, I did not add
+"TODO" comments for the terminology changes in [p2964](/proposals/p2964.md),
+because they would be pervasive, and probably add little value for the reader.
+Instead, I filed issue
+[#5599](https://github.com/carbon-language/carbon-lang/issues/5599) to track the
+task of making those changes.
+
+## Details
+
+See the changes in the proposal PR.
+
+## Rationale
+
+This proposal will help advance our goal of
+[promoting a healthy and vibrant community with an inclusive, welcoming, and pragmatic culture](/docs/project/goals.md#community-and-culture):
+in order to effectively participate in extending, implementing, or evaluating
+the design of Carbon, people need to be able to find accurate information about
+that design (and avoid inaccurate information) with reasonable effort.
+
+## Alternatives considered
+
+### Require documentation updates as part of the proposal
+
+We could require all language design proposals to implement the corresponding
+changes in `/docs/design`. This would more thoroughly address the risk of
+readers being misled, because you might mistakenly read documentation that's
+marked with a "TODO" comment, but you can't mistakenly read documentation that
+doesn't exist anymore.
+
+This would also push proposals to be more concrete, detailed, and fully
+integrated with the rest of the language. However, that's a double-edged sword:
+it could help us identify problems with a proposal before it's adopted, but it
+could also close off the option of deferring those problems to future work in
+order to make incremental progress. More mundanely, it would also increase both
+the up-front cost of creating a proposal, and the cost of iteratively changing
+it during review.
+
+The loss of agility from those factors is likely to outweigh the somewhat
+tenuous and speculative benefits of this approach, at least at this early stage
+of Carbon's development.
+
+### One TODO per document
+
+This policy intentionally encourages fairly granular TODO comments attached to
+the specific passages where changes are needed (see especially the proposed
+changes in
+[`details.md`](https://github.com/carbon-language/carbon-lang/pull/5606/files#diff-b84aebe5ad22a2be2b4c222cf68fd93981ebcd2451bafb56ed5fe46ec186a3c8)).
+We could instead encourage having a single TODO comment per document, in order
+to reduce the burden on proposal authors.
+
+However, this would have several related drawbacks for readers of the
+documentation:
+
+-   It increases the risk that the reader will overlook the TODO comment
+    altogether, because it may be quite far away from the passage they're
+    reading.
+-   It increases the risk of false positives, where passages that remain valid
+    are caught up in the scope of a file-level TODO comment.
+-   It may force the TODO comment to be less specific about how the proposal
+    changes the content of the document. This compounds the previous problem,
+    because it means the reader has to do more work to distinguish true from
+    false positives.
+
+Together, these drawbacks could severely undermine the purpose of this proposal.
+By contrast, the burden of granular TODOs on proposal authors seems relatively
+marginal. For example, I was able to add TODO comments for 8 proposals in a few
+hours, despite in most cases being quite unfamiliar with their contents.
+
+### Link to the PR instead of the proposal document
+
+This policy specifies that the TODOs should include a link to the proposal
+document, but we could instead link to the proposal PR. This would be more
+consistent with existing documentation, which rarely links to proposal documents
+(among other things, this makes it easier to find all references to a given
+proposal). The PR also surfaces some information not available in the proposal
+document, such as the discussion associated with a proposal, and the other
+places where TODOs were added. Finally, the PR UI gives you access to a rendered
+view of the proposal document (accessible through "View file" in the drop-down
+menu for the file), in which links to other documents take you to the versions
+that existed as of when the proposal was committed, which may help clarify the
+historical context of the proposal if those documents have changed in the
+meantime.
+
+However, the TODO links serve a very different purpose than other proposal
+links: when we link to a proposal in a "References" or "Alternatives considered"
+section of a document, we're providing interested readers with supplementary
+information about the rationale and history of the design being documented, and
+for those purposes the additional information in the PR is directly relevant.
+Here, by contrast, the proposal is acting as the sole documentation for _the
+design itself_, and for that purpose all the relevant information is in the
+proposal document itself.
+
+Finally, in the event that the reader wants additional information from the PR,
+every proposal document has a prominent PR link at the top, so the additional
+burden on those readers is a single click. By contrast, the link from a proposal
+PR to the corresponding rendered document is hidden in a drop-down menu
+somewhere in the middle of the PR's "files changed" page.