Add a writeup for how associated constants are processed. (#4856)

Also fix a bug found when working through this.

toolchain/check/handle_let_and_var.cpp

@@ -300,6 +300,9 @@ static auto FinishAssociatedConstant(Context& context, Parse::LetDeclId node_id,
     context.name_scopes()
         .Get(context.interfaces().Get(interface_id).scope_id)
         .set_has_error();
+    if (decl_info.init_id.has_value()) {
+      DiscardGenericDecl(context);
+    }
     context.inst_block_stack().Pop();
     return;
   }

toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_binding.carbon

@@ -8,15 +8,90 @@
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interface/no_prelude/fail_assoc_const_not_binding.carbon
 
+// --- fail_tuple_pattern.carbon
+
+library "[[@TEST_NAME]]";
+
 interface I {
-  // CHECK:STDERR: fail_assoc_const_not_binding.carbon:[[@LINE+4]]:7: error: semantics TODO: `tuple pattern in let/var` [SemanticsTodo]
+  // CHECK:STDERR: fail_tuple_pattern.carbon:[[@LINE+4]]:7: error: semantics TODO: `tuple pattern in let/var` [SemanticsTodo]
   // CHECK:STDERR:   let (T:! type, U:! type);
   // CHECK:STDERR:       ^~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   let (T:! type, U:! type);
 }
 
-// CHECK:STDOUT: --- fail_assoc_const_not_binding.carbon
+// --- fail_tuple_pattern_with_default.carbon
+
+library "[[@TEST_NAME]]";
+
+interface I {
+  // CHECK:STDERR: fail_tuple_pattern_with_default.carbon:[[@LINE+4]]:15: error: semantics TODO: `tuple pattern in let/var` [SemanticsTodo]
+  // CHECK:STDERR:   default let (T:! type, U:! type) = ({}, {});
+  // CHECK:STDERR:               ^~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  default let (T:! type, U:! type) = ({}, {});
+}
+
+// --- fail_var_pattern.carbon
+
+library "[[@TEST_NAME]]";
+
+interface I {
+  // CHECK:STDERR: fail_var_pattern.carbon:[[@LINE+8]]:7: error: expected name in binding pattern [ExpectedBindingPattern]
+  // CHECK:STDERR:   let var T:! type;
+  // CHECK:STDERR:       ^~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_var_pattern.carbon:[[@LINE+4]]:7: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo]
+  // CHECK:STDERR:   let var T:! type;
+  // CHECK:STDERR:       ^~~
+  // CHECK:STDERR:
+  let var T:! type;
+}
+
+// --- fail_var_pattern_with_default.carbon
+
+library "[[@TEST_NAME]]";
+
+interface I {
+  // CHECK:STDERR: fail_var_pattern_with_default.carbon:[[@LINE+8]]:15: error: expected name in binding pattern [ExpectedBindingPattern]
+  // CHECK:STDERR:   default let var T:! type = {};
+  // CHECK:STDERR:               ^~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_var_pattern_with_default.carbon:[[@LINE+4]]:15: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo]
+  // CHECK:STDERR:   default let var T:! type = {};
+  // CHECK:STDERR:               ^~~
+  // CHECK:STDERR:
+  default let var T:! type = {};
+}
+
+// CHECK:STDOUT: --- fail_tuple_pattern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %I.type: type = facet_type <@I> [template]
+// CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .I = %I.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %I.decl: type = interface_decl @I [template = constants.%I.type] {} {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: interface @I {
+// CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = %Self
+// CHECK:STDOUT:   has_error
+// CHECK:STDOUT:   witness = ()
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: assoc_const @T T:! type;
+// CHECK:STDOUT:
+// CHECK:STDOUT: assoc_const @U U:! type;
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_tuple_pattern_with_default.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %I.type: type = facet_type <@I> [template]
@@ -43,3 +118,29 @@ interface I {
 // CHECK:STDOUT:
 // CHECK:STDOUT: assoc_const @U U:! type;
 // CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_var_pattern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {}
+// CHECK:STDOUT:
+// CHECK:STDOUT: interface @I {
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = <unexpected>.inst15
+// CHECK:STDOUT:   witness = invalid
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_var_pattern_with_default.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {}
+// CHECK:STDOUT:
+// CHECK:STDOUT: interface @I {
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = <unexpected>.inst15
+// CHECK:STDOUT:   witness = invalid
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/docs/README.md

@@ -44,7 +44,7 @@ The main components are:
        [Lex::TokenizedBuffer](/toolchain/lex/tokenized_buffer.h).
     3. [Parse](parse.md): Transform a TokenizedBuffer into a
        [Parse::Tree](/toolchain/parse/tree.h).
-    4. [Check](check.md): Transform a Tree to produce
+    4. [Check](check): Transform a Tree to produce
        [SemIR::File](/toolchain/sem_ir/file.h).
     5. [Lower](lower.md): Transform the SemIR to an
        [LLVM Module](https://llvm.org/doxygen/classllvm_1_1Module.html).

toolchain/docs/check.md → toolchain/docs/check/README.md

@@ -11,6 +11,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 ## Table of contents
 
 -   [Overview](#overview)
+-   [Subtopics](#subtopics)
 -   [Postorder processing](#postorder-processing)
 -   [Key IR concepts](#key-ir-concepts)
     -   [Instruction operands](#instruction-operands)
@@ -47,6 +48,12 @@ or SemIR. This will look closer to a series of instructions, in preparation for
 transformation to LLVM IR. Semantic analysis and type checking occurs during the
 production of SemIR. It also does any validation that requires context.
 
+## Subtopics
+
+Some particular topics have their own documentation:
+
+-   [Associated constants](associated_constant.md)
+
 ## Postorder processing
 
 The checking step is oriented on postorder processing on the `Parse::Tree` to
@@ -82,7 +89,7 @@ instruction, and `SemIR::PointerType` represents a pointer type instruction.
 Each instruction class has up to four public data members describing the
 instruction, as described in
 [sem_ir/typed_insts.h](/toolchain/sem_ir/typed_insts.h) (also see
-[adding features for Check](adding_features.md#check)):
+[adding features for Check](/toolchain/docs/adding_features.md#check)):
 
 -   An `InstKind kind;` member if the instruction has a `Kinds` constant making
     it a shorthand for multiple individual instructions.

toolchain/docs/check/associated_constant.md

@@ -0,0 +1,187 @@
+# Associated constants
+
+<!--
+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
+-->
+
+<!-- toc -->
+
+## Table of contents
+
+-   [Overview](#overview)
+-   [Declaration checking](#declaration-checking)
+-   [Specifying rewrite constraints](#specifying-rewrite-constraints)
+-   [Definition of associated constant values](#definition-of-associated-constant-values)
+-   [Use of associated constants](#use-of-associated-constants)
+    -   [Simple member access](#simple-member-access)
+    -   [Compound member access](#compound-member-access)
+    -   [Forming the constant value](#forming-the-constant-value)
+
+<!-- tocstop -->
+
+## Overview
+
+_Note:_ This document only describes non-function associated constants.
+
+An associated constant is declared within an interface scope with the syntax:
+
+```carbon
+[MODIFIERS] let NAME:! TYPE [= INITIALIZER] ;
+```
+
+Associated constants introduce a slot in the witness table for an interface that
+contains a value of type `TYPE`.
+
+Associated constants are always generic entities, because they're always
+parameterized at least by the `Self` type of the interface, as well as any other
+enclosing generic parameters. Note that the interface itself is _not_
+parameterized by its `Self`.
+
+Associated constant entities are held in the `associated_constants` value store
+as objects of type `AssociatedConstant`. Each declaration of an associated
+constant is modeled by an `AssociatedConstantDecl` instruction. Each such
+instruction is then wrapped in an `AssociatedEntity` instruction which
+represents the slot within an interface witness where the constant's value can
+be found.
+
+## Declaration checking
+
+Because associated constants share the syntax of `let` declarations, a lot of
+the checking logic is also shared. This logic is in
+[handle_let_and_var.cpp](/toolchain/check/handle_let_and_var.cpp). Associated
+constant declaration handling proceeds as follows:
+
+1.  ```carbon
+    let NAME:! TYPE [= INITIALIZER] ;
+    ^
+    ```
+
+    `StartAssociatedConstant` is called at the start of an interface-scope `let`
+    declaration. This:
+
+    -   Starts a generic declaration region.
+    -   Pushes an instruction block to hold instructions within the declaration
+        of the constant. These form the body of the generic.
+
+2.  ```carbon
+    let NAME:! TYPE [= INITIALIZER] ;
+        ~~~~^~~~~~~
+    ```
+
+    Process the symbolic binding pattern. This is done in
+    [handle_binding_pattern.cpp](/toolchain/check/handle_binding_pattern.cpp),
+    which detects that we are at interface scope, and creates an
+    `AssociatedConstantDecl` and corresponding `AssociatedConstant` entity. This
+    binding is then produced as the instruction associated with the binding
+    pattern.
+
+    _Note:_ This is somewhat unusual: usually, a pattern instruction would be
+    associated with a pattern parse node.
+
+3.  ```carbon
+    let NAME:! TYPE ;
+                    ^
+    let NAME:! TYPE = INITIALIZER ;
+                    ^
+    ```
+
+    When we reach the end of the pattern in an interface-scope `let` binding,
+    either because we reached the `=` or because we reached the `;` and there
+    was no initializer, `EndAssociatedConstantDeclRegion` is called. This:
+
+    -   Ends the generic declaration region.
+    -   Builds an `AssociatedEntity` object, reserving a slot in the interface's
+        witness table for the constant.
+    -   Adds the associated constant to name lookup.
+
+    _Note:_ The pattern might not be valid for an associated constant. In this
+    case, we won't have built an `AssociatedConstantDecl` in the previous step.
+    When this happens, we instead just discard the generic declaration region
+    and continue. The invalid pattern will be diagnosed later.
+
+4.  ```carbon
+    let NAME:! TYPE = INITIALIZER ;
+                    ^
+    ```
+
+    If there is an initializer, we start the generic definition region.
+
+5.  ```carbon
+    let NAME:! TYPE [= INITIALIZER] ;
+                                    ^
+    ```
+
+    At the end of the declaration, `FinishAssociatedConstant` is called to
+    finalize the declaration. This:
+
+    -   Diagnoses if the pattern handling didn't create an
+        `AssociatedConstantDecl`.
+    -   Finishes handling the initializer, if it's present:
+        -   Converts the initializer to the type of the constant.
+        -   Ends the generic definition region.
+    -   Pops the inst block created by `StartAssociatedConstant` and attaches it
+        to the `AssociatedConstantDecl`.
+    -   Adds the `AssociatedConstantDecl` to the enclosing inst block.
+
+## Specifying rewrite constraints
+
+TODO: Fill this out. In particular, note that we do not convert the rewrite to
+the type of the associated constant as part of forming a `where` expression if
+the constant's type is symbolic, and instead defer that until the facet type is
+resolved.
+
+## Definition of associated constant values
+
+Associated constant values are stored into witness tables as part of impl
+processing in [impl.cpp](/toolchain/check/impl.cpp).
+
+TODO: Fill this out once the new model is implemented.
+
+## Use of associated constants
+
+The work to handle uses of associated constants starts in
+[member_access.cpp](/toolchain/check/member_access.cpp).
+
+When an `AssociatedEntity` is the member in a member access, impl lookup is
+performed to find the corresponding impl witness. The self type in impl lookup
+depends on how the member name was found.
+
+### Simple member access
+
+In `LookupMemberNameInScope`, if lookup for `y` in `x.y` finds an associated
+constant from interface `I`, then a witness is determined as follows:
+
+-   If the lookup scope is the type `T` of `x`, then:
+    -   If `T` is a non-type facet, the witness for that facet is used. TODO:
+        That facet might not contain a witness for `I`. In that case we will
+        need to perform impl lookup for `T as I` instead.
+    -   Otherwise, impl lookup for `T as I` is performed to find the witness.
+-   If the lookup scope is `x` itself, then:
+    -   If `x` is a facet type or a namespace, impl lookup is not performed, and
+        the result is simply `y`. This happens for cases such as
+        `Interface.AssocConst`.
+    -   Otherwise, `x` must be a type other than a facet type, and impl lookup
+        for `x as I` is performed to find the witness.
+
+### Compound member access
+
+In `PerformCompoundMemberAccess` for `x.(y)`, if `y` is an associated constant
+then impl lookup is performed for `T as I`, where `T` is the type of `x` and `I`
+is the interface in which `y` is declared to find the witness containing the
+constant value.
+
+### Forming the constant value
+
+Once the witness is determined, `AccessMemberOfImplWitness` is called to find
+the value of the associated constant in the witness. In the case where an impl
+lookup is needed, `PerformImplLookup` calls `AccessMemberOfImplWitness`,
+otherwise it's called directly.
+
+`AccessMemberOfImplWitness` uses `GetTypeForSpecificAssociatedEntity` to form
+the type of the constant. This substitutes both the generic arguments (if any)
+for the interface and the `Self` type into the type of the associated constant.
+Then, an `ImplWitnessAccess` instruction is created to extract the relevant slot
+from the witness. Constant evaluation of this instruction reads the associated
+constant from the witness table.

toolchain/docs/parse.md

@@ -49,7 +49,7 @@ structures, but it may still be helpful for tools such as syntax highlighters or
 refactoring tools.
 
 In general, we favor doing the checking for whether something is allowed _in a
-particular context_ in [the check stage](check.md) instead of the parse stage,
+particular context_ in [the check stage](check) instead of the parse stage,
 unless the context is very local. This is for a few reasons:
 
 -   We anticipate that the parse stage will be used to operate on invalid code