Fix handling of generic self: Self. (#7922)

We previously discarded a `generic` applied to `self`, resulting in
silently treating `generic self: Self` as a runtime parameter and
producing a weird error on `template self: Self`.

Instead of forcibly making all `self` parameters non-runtime, change the
*default* for a parameter named `self` to be runtime.

This is only a partial fix; more work is required to support `fn
F(generic self)` with an omitted type.

toolchain/check/testdata/class/self/generic_self.carbon

@@ -0,0 +1,40 @@
+// 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
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/destroy.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/self/generic_self.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/self/generic_self.carbon
+
+// --- generic_self.carbon
+library "[[@TEST_NAME]]";
+
+class Class {
+  fn F(unused generic self: Self) {}
+}
+
+fn G() {
+  ({} as Class).F();
+}
+
+// --- fail_runtime_arg.carbon
+library "[[@TEST_NAME]]";
+
+class Class {
+  fn F(unused generic self: Self) {}
+}
+
+fn G(c: Class) {
+  // CHECK:STDERR: fail_runtime_arg.carbon:[[@LINE+7]]:3: error: argument for generic parameter is not a compile-time constant [CompTimeArgumentNotConstant]
+  // CHECK:STDERR:   c.F();
+  // CHECK:STDERR:   ^~~~~
+  // CHECK:STDERR: fail_runtime_arg.carbon:[[@LINE-7]]:23: note: initializing generic parameter `self` declared here [InitializingGenericParam]
+  // CHECK:STDERR:   fn F(unused generic self: Self) {}
+  // CHECK:STDERR:                       ^~~~~~~~~~
+  // CHECK:STDERR:
+  c.F();
+}

toolchain/parse/handle_binding_pattern.cpp

@@ -26,6 +26,7 @@ static auto ResolveBindingPhase(Context& context, Context::State& state,
                                 std::optional<Lex::TokenIndex> template_token,
                                 std::optional<Lex::TokenIndex> generic_token,
                                 std::optional<Lex::TokenIndex> runtime_token,
+                                std::optional<Lex::TokenIndex> self_token,
                                 bool& redundant_modifier) -> bool {
   // `template`/`generic` force a generic binding, `runtime` forces a runtime
   // binding, and otherwise the context's default applies.
@@ -35,7 +36,11 @@ static auto ResolveBindingPhase(Context& context, Context::State& state,
   } else if (runtime_token) {
     resolved_generic = false;
   } else {
-    resolved_generic = state.binding_context != BindingContext::ExplicitParam;
+    // For `self`, the default is always `runtime` so that `[ref self]` is
+    // diagnosed for being in the wrong place, not for being a generic ref
+    // binding.
+    resolved_generic =
+        state.binding_context != BindingContext::ExplicitParam && !self_token;
   }
 
   // A form binding's phase is fixed, so phase keywords don't apply to it, and
@@ -157,6 +162,7 @@ auto HandleBindingPattern(Context& context) -> void {
 
   auto token_kind = context.PositionKind();
   if (!token_kind.is_binding_pattern_operator()) {
+    // TODO: We should allow omitting the type in a generic `self` binding.
     if (self_token && !template_token && !generic_token && !runtime_token) {
       // A `self` binding may omit its type; checking supplies the implicit
       // `Self` type. There is no type node, so this produces a
@@ -186,22 +192,9 @@ auto HandleBindingPattern(Context& context) -> void {
   bool is_form = token_kind == Lex::TokenKind::ColonQuestion;
 
   bool redundant_modifier = false;
-  bool resolved_generic =
-      ResolveBindingPhase(context, state, is_form, template_token,
-                          generic_token, runtime_token, redundant_modifier);
-
-  // `self` is always a runtime receiver binding; its phase never comes from the
-  // enclosing context's default. Forcing runtime here means that a misplaced
-  // `self` (in a deduced `[]` list or a compile-time entity's parameters, where
-  // the default would otherwise be generic) is reported by `check` as a
-  // misplaced `self` — the relevant error — rather than also producing a
-  // `ref`-on-generic error from that default.
-  //
-  // TODO: This is wrong: it rejects `fn F(template self: Self)` and treats
-  // `fn F(generic self: Self)` as being non-generic.
-  if (self_token) {
-    resolved_generic = false;
-  }
+  bool resolved_generic = ResolveBindingPhase(
+      context, state, is_form, template_token, generic_token, runtime_token,
+      self_token, redundant_modifier);
 
   // `template` and `ref` wrap the binding name, and each is only meaningful on
   // a particular kind of binding: `template` on a generic binding, and `ref` on

toolchain/parse/state.def

@@ -1375,6 +1375,8 @@ CARBON_PARSE_STATE(ExprPattern)
 // ^~~~~~~~~~~~~~~~~~~~~~
 // template/generic _: <...>
 // ^~~~~~~~~~~~~~~~~~~
+// template/generic self: <...>
+// ^~~~~~~~~~~~~~~~~~~~~~
 //   1. Expr
 //   2. BindingPatternFinishAsGeneric
 //

toolchain/parse/testdata/generics/generic_params/self.carbon

@@ -0,0 +1,85 @@
+// 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
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/generics/generic_params/self.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/generics/generic_params/self.carbon
+
+// --- generic_self.carbon
+
+class C {
+  fn A(generic self: Self);
+  fn B(template self: Self);
+}
+
+// --- fail_todo_generic_self_no_type.carbon
+
+class C {
+  // CHECK:STDERR: fail_todo_generic_self_no_type.carbon:[[@LINE+4]]:20: error: expected `:` or `:?` in binding pattern [ExpectedBindingPattern]
+  // CHECK:STDERR:   fn A(generic self);
+  // CHECK:STDERR:                    ^
+  // CHECK:STDERR:
+  fn A(generic self);
+  // CHECK:STDERR: fail_todo_generic_self_no_type.carbon:[[@LINE+4]]:21: error: expected `:` or `:?` in binding pattern [ExpectedBindingPattern]
+  // CHECK:STDERR:   fn B(template self);
+  // CHECK:STDERR:                     ^
+  // CHECK:STDERR:
+  fn B(template self);
+}
+
+// CHECK:STDOUT: - filename: generic_self.carbon
+// CHECK:STDOUT:   ╭─FileStart ''
+// CHECK:STDOUT:   │   ╭─ClassIntroducer 'class'
+// CHECK:STDOUT:   │   ├─IdentifierNameNotBeforeSignature 'C'
+// CHECK:STDOUT:   │ ╭─ClassDefinitionStart '{'
+// CHECK:STDOUT:   │ │ ╭─FunctionIntroducer 'fn'
+// CHECK:STDOUT:   │ │ ├─IdentifierNameMaybeBeforeSignature 'A'
+// CHECK:STDOUT:   │ │ │ ╭─ExplicitParamListStart '('
+// CHECK:STDOUT:   │ │ │ │ ╭─SelfValueName 'self'
+// CHECK:STDOUT:   │ │ │ │ ├─CompileTimeBindingPatternTypeStart ':'
+// CHECK:STDOUT:   │ │ │ │ ├─SelfTypeNameExpr 'Self'
+// CHECK:STDOUT:   │ │ │ ├─CompileTimeBindingPattern ':'
+// CHECK:STDOUT:   │ │ ├─ExplicitParamList ')'
+// CHECK:STDOUT:   │ ├─FunctionDecl ';'
+// CHECK:STDOUT:   │ │ ╭─FunctionIntroducer 'fn'
+// CHECK:STDOUT:   │ │ ├─IdentifierNameMaybeBeforeSignature 'B'
+// CHECK:STDOUT:   │ │ │ ╭─ExplicitParamListStart '('
+// CHECK:STDOUT:   │ │ │ │   ╭─SelfValueName 'self'
+// CHECK:STDOUT:   │ │ │ │ ╭─TemplateBindingName 'template'
+// CHECK:STDOUT:   │ │ │ │ ├─CompileTimeBindingPatternTypeStart ':'
+// CHECK:STDOUT:   │ │ │ │ ├─SelfTypeNameExpr 'Self'
+// CHECK:STDOUT:   │ │ │ ├─CompileTimeBindingPattern ':'
+// CHECK:STDOUT:   │ │ ├─ExplicitParamList ')'
+// CHECK:STDOUT:   │ ├─FunctionDecl ';'
+// CHECK:STDOUT:   ├─ClassDefinition '}'
+// CHECK:STDOUT:   ├─FileEnd ''
+// CHECK:STDOUT:   (root)
+// CHECK:STDOUT: - filename: fail_todo_generic_self_no_type.carbon
+// CHECK:STDOUT:   ╭─FileStart ''
+// CHECK:STDOUT:   │   ╭─ClassIntroducer 'class'
+// CHECK:STDOUT:   │   ├─IdentifierNameNotBeforeSignature 'C'
+// CHECK:STDOUT:   │ ╭─ClassDefinitionStart '{'
+// CHECK:STDOUT:   │ │ ╭─FunctionIntroducer 'fn'
+// CHECK:STDOUT:   │ │ ├─IdentifierNameMaybeBeforeSignature 'A'
+// CHECK:STDOUT:   │ │ │ ╭─ExplicitParamListStart '('
+// CHECK:STDOUT:   │ │ │ │ ╭─SelfValueName 'self'
+// CHECK:STDOUT:   │ │ │ │ ├─InvalidParse ')' has_error
+// CHECK:STDOUT:   │ │ │ │ ├─InvalidParse ')' has_error
+// CHECK:STDOUT:   │ │ │ ├─LetBindingPattern 'generic' has_error
+// CHECK:STDOUT:   │ │ ├─ExplicitParamList ')' has_error
+// CHECK:STDOUT:   │ ├─FunctionDecl ';'
+// CHECK:STDOUT:   │ │ ╭─FunctionIntroducer 'fn'
+// CHECK:STDOUT:   │ │ ├─IdentifierNameMaybeBeforeSignature 'B'
+// CHECK:STDOUT:   │ │ │ ╭─ExplicitParamListStart '('
+// CHECK:STDOUT:   │ │ │ │ ╭─SelfValueName 'self'
+// CHECK:STDOUT:   │ │ │ │ ├─InvalidParse ')' has_error
+// CHECK:STDOUT:   │ │ │ │ ├─InvalidParse ')' has_error
+// CHECK:STDOUT:   │ │ │ ├─LetBindingPattern 'template' has_error
+// CHECK:STDOUT:   │ │ ├─ExplicitParamList ')' has_error
+// CHECK:STDOUT:   │ ├─FunctionDecl ';'
+// CHECK:STDOUT:   ├─ClassDefinition '}'
+// CHECK:STDOUT:   ├─FileEnd ''
+// CHECK:STDOUT:   (root)