Remove SelfParam, add an AddrPattern instead. (carbon-language#3506)

This is intended to make the representation of a `self` pattern be more
similar to other patterns.

toolchain/check/context.cpp

@@ -922,6 +922,7 @@ class TypeCompleter {
  // NOLINTNEXTLINE(bugprone-switch-missing-default-case)
  switch (inst.kind()) {
  case SemIR::AddressOf::Kind:
+ case SemIR::AddrPattern::Kind:
  case SemIR::ArrayIndex::Kind:
  case SemIR::ArrayInit::Kind:
  case SemIR::Assign::Kind:
@@ -954,7 +955,6 @@ class TypeCompleter {
  case SemIR::RealLiteral::Kind:
  case SemIR::Return::Kind:
  case SemIR::ReturnExpr::Kind:
- case SemIR::SelfParam::Kind:
  case SemIR::SpliceBlock::Kind:
  case SemIR::StringLiteral::Kind:
  case SemIR::StructAccess::Kind:

toolchain/check/convert.cpp

@@ -1064,7 +1064,8 @@ CARBON_DIAGNOSTIC(InCallToFunction, Note, "Calling function declared here.");
 // Convert the object argument in a method call to match the `self` parameter.
 static auto ConvertSelf(Context& context, Parse::NodeId call_parse_node,
  Parse::NodeId callee_parse_node,
- SemIR::SelfParam self_param, SemIR::InstId self_id)
+ std::optional<SemIR::AddrPattern> addr_pattern,
+ SemIR::Param self_param, SemIR::InstId self_id)
  -> SemIR::InstId {
  if (!self_id.is_valid()) {
  CARBON_DIAGNOSTIC(MissingObjectInMethodCall, Error,
@@ -1083,14 +1084,13 @@ static auto ConvertSelf(Context& context, Parse::NodeId call_parse_node,
  "Initializing `{0}` parameter of method declared here.",
  llvm::StringLiteral);
  builder.Note(self_param.parse_node, InCallToFunctionSelf,
- self_param.is_addr_self.index
- ? llvm::StringLiteral("addr self")
- : llvm::StringLiteral("self"));
+ addr_pattern ? llvm::StringLiteral("addr self")
+ : llvm::StringLiteral("self"));
  });
 
  // For `addr self`, take the address of the object argument.
  auto self_or_addr_id = self_id;
- if (self_param.is_addr_self.index) {
+ if (addr_pattern) {
  self_or_addr_id = ConvertToValueOrRefExpr(context, self_or_addr_id);
  auto self = context.insts().Get(self_or_addr_id);
  switch (SemIR::GetExprCategory(context.sem_ir(), self_id)) {
@@ -1146,10 +1146,16 @@ auto ConvertCallArgs(Context& context, Parse::NodeId call_parse_node,
 
  // Check implicit parameters.
  for (auto implicit_param_id : implicit_param_refs) {
- auto param = context.insts().Get(implicit_param_id);
- if (auto self_param = param.TryAs<SemIR::SelfParam>()) {
- auto converted_self_id = ConvertSelf(
- context, call_parse_node, callee_parse_node, *self_param, self_id);
+ auto pattern = context.insts().Get(implicit_param_id);
+ auto addr_pattern = pattern.TryAs<SemIR::AddrPattern>();
+ if (addr_pattern) {
+ pattern = context.insts().Get(addr_pattern->inner_id);
+ }
+ if (auto param = pattern.TryAs<SemIR::Param>();
+ param && param->name_id == SemIR::NameId::SelfValue) {
+ auto converted_self_id =
+ ConvertSelf(context, call_parse_node, callee_parse_node, addr_pattern,
+ *param, self_id);
  if (converted_self_id == SemIR::InstId::BuiltinError) {
  return SemIR::InstBlockId::Invalid;
  }

toolchain/check/handle_binding_pattern.cpp

@@ -11,15 +11,19 @@ namespace Carbon::Check {
 
 auto HandleAddress(Context& context, Parse::NodeId parse_node) -> bool {
  auto self_param_id =
- context.node_stack().Peek<Parse::NodeKind::BindingPattern>();
- if (auto self_param =
- context.insts().Get(self_param_id).TryAs<SemIR::SelfParam>()) {
- self_param->is_addr_self = SemIR::BoolValue::True;
- context.insts().Set(self_param_id, *self_param);
+ context.node_stack().Pop<Parse::NodeKind::BindingPattern>();
+ auto self_param = context.insts().TryGetAs<SemIR::Param>(self_param_id);
+ if (self_param && self_param->name_id == SemIR::NameId::SelfValue) {
+ // TODO: The type of an `addr_pattern` should probably be the non-pointer
+ // type, because that's the type that the pattern matches.
+ context.AddInstAndPush(
+ parse_node,
+ SemIR::AddrPattern{parse_node, self_param->type_id, self_param_id});
  } else {
  CARBON_DIAGNOSTIC(AddrOnNonSelfParam, Error,
  "`addr` can only be applied to a `self` parameter.");
  context.emitter().Emit(TokenOnly(parse_node), AddrOnNonSelfParam);
+ context.node_stack().Push(parse_node, self_param_id);
  }
  return true;
 }
@@ -35,28 +39,20 @@ auto HandleBindingPattern(Context& context, Parse::NodeId parse_node) -> bool {
  auto type_node_copy = type_node;
  auto cast_type_id = ExprAsType(context, type_node, parsed_type_id);
 
- // A `self` binding doesn't have a name.
- if (auto self_node =
- context.node_stack()
- .PopForSoloParseNodeIf<Parse::NodeKind::SelfValueName>()) {
- if (context.parse_tree().node_kind(context.node_stack().PeekParseNode()) !=
- Parse::NodeKind::ImplicitParamListStart) {
- CARBON_DIAGNOSTIC(
- SelfOutsideImplicitParamList, Error,
- "`self` can only be declared in an implicit parameter list.");
- context.emitter().Emit(parse_node, SelfOutsideImplicitParamList);
- }
- context.AddInstAndPush(
- parse_node, SemIR::SelfParam{*self_node, cast_type_id,
- /*is_addr_self=*/SemIR::BoolValue::False});
- return true;
- }
-
  // TODO: Handle `_` bindings.
 
  // Every other kind of pattern binding has a name.
  auto [name_node, name_id] = context.node_stack().PopNameWithParseNode();
 
+ // A `self` binding can only appear in an implicit parameter list.
+ if (name_id == SemIR::NameId::SelfValue &&
+ !context.node_stack().PeekIs<Parse::NodeKind::ImplicitParamListStart>()) {
+ CARBON_DIAGNOSTIC(
+ SelfOutsideImplicitParamList, Error,
+ "`self` can only be declared in an implicit parameter list.");
+ context.emitter().Emit(parse_node, SelfOutsideImplicitParamList);
+ }
+
  // Allocate an instruction of the appropriate kind, linked to the name for
  // error locations.
  // TODO: The node stack is a fragile way of getting context information.

toolchain/check/handle_function.cpp

@@ -241,6 +241,13 @@ auto HandleFunctionDefinitionStart(Context& context, Parse::NodeId parse_node)
  context.inst_blocks().Get(function.param_refs_id))) {
  auto param = context.insts().Get(param_id);
 
+ // Find the parameter in the pattern.
+ // TODO: More general pattern handling?
+ if (auto addr_pattern = param.TryAs<SemIR::AddrPattern>()) {
+ param_id = addr_pattern->inner_id;
+ param = context.insts().Get(param_id);
+ }
+
  // The parameter types need to be complete.
  context.TryToCompleteType(param.type_id(), [&] {
  CARBON_DIAGNOSTIC(
@@ -255,9 +262,6 @@ auto HandleFunctionDefinitionStart(Context& context, Parse::NodeId parse_node)
  if (auto fn_param = param.TryAs<SemIR::Param>()) {
  context.AddNameToLookup(fn_param->parse_node, fn_param->name_id,
  param_id);
- } else if (auto self_param = param.TryAs<SemIR::SelfParam>()) {
- context.AddNameToLookup(self_param->parse_node, SemIR::NameId::SelfValue,
- param_id);
  } else {
  CARBON_FATAL() << "Unexpected kind of parameter in function definition "
  << param;

toolchain/check/handle_name.cpp

@@ -72,6 +72,26 @@ static auto GetClassElementIndex(Context& context, SemIR::InstId element_id)
  << " in class element name";
 }
 
+// Returns whether `function_id` is an instance method, that is, whether it has
+// an implicit `self` parameter.
+static auto IsInstanceMethod(const SemIR::File& sem_ir,
+ SemIR::FunctionId function_id) -> bool {
+ auto& function = sem_ir.functions().Get(function_id);
+ for (auto param_id :
+ sem_ir.inst_blocks().Get(function.implicit_param_refs_id)) {
+ auto inst = sem_ir.insts().Get(param_id);
+ if (auto addr = inst.TryAs<SemIR::AddrPattern>()) {
+ inst = sem_ir.insts().Get(addr->inner_id);
+ }
+ if (auto param = inst.TryAs<SemIR::Param>();
+ param && param->name_id == SemIR::NameId::SelfValue) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
 auto HandleMemberAccessExpr(Context& context, Parse::NodeId parse_node)
  -> bool {
  SemIR::NameId name_id = context.node_stack().PopName();
@@ -170,18 +190,14 @@ auto HandleMemberAccessExpr(Context& context, Parse::NodeId parse_node)
  CARBON_CHECK(function_decl)
  << "Unexpected value " << context.insts().Get(function_name_id)
  << " of function type";
- auto& function = context.functions().Get(function_decl->function_id);
- for (auto param_id :
- context.inst_blocks().Get(function.implicit_param_refs_id)) {
- if (context.insts().Get(param_id).Is<SemIR::SelfParam>()) {
- context.AddInstAndPush(
- parse_node,
- SemIR::BoundMethod{
- parse_node,
- context.GetBuiltinType(SemIR::BuiltinKind::BoundMethodType),
- base_id, member_id});
- return true;
- }
+ if (IsInstanceMethod(context.sem_ir(), function_decl->function_id)) {
+ context.AddInstAndPush(
+ parse_node,
+ SemIR::BoundMethod{
+ parse_node,
+ context.GetBuiltinType(SemIR::BuiltinKind::BoundMethodType),
+ base_id, member_id});
+ return true;
  }
  }
 
@@ -293,7 +309,7 @@ auto HandleSelfTypeNameExpr(Context& context, Parse::NodeId parse_node)
 }
 
 auto HandleSelfValueName(Context& context, Parse::NodeId parse_node) -> bool {
- context.node_stack().Push(parse_node);
+ context.node_stack().Push(parse_node, SemIR::NameId::SelfValue);
  return true;
 }
 

toolchain/check/node_stack.h

@@ -322,6 +322,7 @@ class NodeStack {
  // Translate a parse node kind to the enum ID kind it should always provide.
  static constexpr auto ParseNodeKindToIdKind(Parse::NodeKind kind) -> IdKind {
  switch (kind) {
+ case Parse::NodeKind::Address:
  case Parse::NodeKind::ArrayExpr:
  case Parse::NodeKind::BindingPattern:
  case Parse::NodeKind::CallExpr:
@@ -361,6 +362,7 @@ class NodeStack {
  return IdKind::InterfaceId;
  case Parse::NodeKind::BaseName:
  case Parse::NodeKind::IdentifierName:
+ case Parse::NodeKind::SelfValueName:
  return IdKind::NameId;
  case Parse::NodeKind::ArrayExprSemi:
  case Parse::NodeKind::ClassIntroducer:
@@ -375,7 +377,6 @@ class NodeStack {
  case Parse::NodeKind::ReturnedModifier:
  case Parse::NodeKind::ReturnStatementStart:
  case Parse::NodeKind::ReturnVarModifier:
- case Parse::NodeKind::SelfValueName:
  case Parse::NodeKind::StructLiteralOrStructTypeLiteralStart:
  case Parse::NodeKind::TuplePatternStart:
  case Parse::NodeKind::VariableInitializer:

toolchain/check/testdata/class/base_method.carbon

@@ -64,9 +64,9 @@ fn Call(p: Derived*) {
 // CHECK:STDOUT: extend name_scope1
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F[%self.addr: Base*]() {
+// CHECK:STDOUT: fn @F[addr %self: Base*]() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT: %self.ref: Base* = name_ref self, %self.addr
+// CHECK:STDOUT: %self.ref: Base* = name_ref self, %self
 // CHECK:STDOUT: %.loc14_4: ref Base = deref %self.ref
 // CHECK:STDOUT: %.loc14_10: ref i32 = class_element_access %.loc14_4, element0
 // CHECK:STDOUT: %.loc14_15: i32 = int_literal 1

toolchain/check/testdata/class/base_method_shadow.carbon

@@ -97,11 +97,11 @@ fn Call(a: A*, b: B*, c: C*, d: D*) {
 // CHECK:STDOUT: extend name_scope2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.1[%self.addr: A*]();
+// CHECK:STDOUT: fn @F.1[addr %self: A*]();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.2[%self.addr: B*]();
+// CHECK:STDOUT: fn @F.2[addr %self: B*]();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.3[%self.addr: C*]();
+// CHECK:STDOUT: fn @F.3[addr %self: C*]();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Call(%a: A*, %b: B*, %c: C*, %d: D*) {
 // CHECK:STDOUT: !entry:

toolchain/check/testdata/class/fail_addr_self.carbon

@@ -62,9 +62,9 @@ fn F(c: Class, p: Class*) {
 // CHECK:STDOUT: .G = %G
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.1[%self.addr: Class*]();
+// CHECK:STDOUT: fn @F.1[addr %self: Class*]();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @G[%self.addr: Class]();
+// CHECK:STDOUT: fn @G[addr %self: Class]();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.2(%c: Class, %p: Class*) {
 // CHECK:STDOUT: !entry:

toolchain/check/testdata/class/fail_memaccess_category.carbon

@@ -70,7 +70,7 @@ fn F(s: {.a: A}, b: B) {
 // CHECK:STDOUT: .a = %a
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.1[%self.addr: A*]();
+// CHECK:STDOUT: fn @F.1[addr %self: A*]();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.2(%s: {.a: A}, %b: B) {
 // CHECK:STDOUT: !entry:

toolchain/check/testdata/class/fail_self.carbon

@@ -30,7 +30,7 @@ fn Class.G() -> Class {
  // CHECK:STDERR: var self: Class;
  // CHECK:STDERR: ^~~~~~~~~~~
  var self: Class;
- // CHECK:STDERR: fail_self.carbon:[[@LINE+3]]:10: ERROR: Name `self` not found.
+ // CHECK:STDERR: fail_self.carbon:[[@LINE+3]]:10: ERROR: Cannot copy value of type `Class`.
  // CHECK:STDERR: return self;
  // CHECK:STDERR: ^~~~
  return self;
@@ -93,8 +93,10 @@ fn CallWrongSelf(ws: WrongSelf) {
 // CHECK:STDOUT: fn @G() -> %return: Class {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT: %Class.ref: type = name_ref Class, file.%Class
-// CHECK:STDOUT: %self: Class = self_param false
-// CHECK:STDOUT: %self.ref: <error> = name_ref self, <error>
+// CHECK:STDOUT: %self.var: ref Class = var self
+// CHECK:STDOUT: %self: ref Class = bind_name self, %self.var
+// CHECK:STDOUT: %self.ref: ref Class = name_ref self, %self
+// CHECK:STDOUT: %.loc36: Class = bind_value %self.ref
 // CHECK:STDOUT: return <error>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

toolchain/check/testdata/class/method.carbon

@@ -86,7 +86,7 @@ fn CallGOnInitializingExpr() -> i32 {
 // CHECK:STDOUT: return %.loc15_14.2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @G[%self.addr: Class*]() -> i32;
+// CHECK:STDOUT: fn @G[addr %self: Class*]() -> i32;
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Call(%c: Class) -> i32 {
 // CHECK:STDOUT: !entry:

toolchain/check/testdata/class/raw_self.carbon

@@ -49,12 +49,12 @@ fn Class.G[self: Class](r#self: i32) -> (i32, i32) {
 // CHECK:STDOUT: .n = %n
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F[%self.addr: Class*](%self: i32) {
+// CHECK:STDOUT: fn @F[addr %self.loc13_17: Class*](%self.loc13_31: i32) {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT: %self.ref.loc14_5: Class* = name_ref self, %self.addr
+// CHECK:STDOUT: %self.ref.loc14_5: Class* = name_ref self, %self.loc13_17
 // CHECK:STDOUT: %.loc14_4: ref Class = deref %self.ref.loc14_5
 // CHECK:STDOUT: %.loc14_10: ref i32 = class_element_access %.loc14_4, element0
-// CHECK:STDOUT: %self.ref.loc14_15: i32 = name_ref r#self, %self
+// CHECK:STDOUT: %self.ref.loc14_15: i32 = name_ref r#self, %self.loc13_31
 // CHECK:STDOUT: assign %.loc14_10, %self.ref.loc14_15
 // CHECK:STDOUT: return
 // CHECK:STDOUT: }

toolchain/check/testdata/class/self.carbon

@@ -55,9 +55,9 @@ fn Class.G[addr self: Class*]() -> i32 {
 // CHECK:STDOUT: return %.loc15_14.2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @G[%self.addr: Class*]() -> i32 {
+// CHECK:STDOUT: fn @G[addr %self: Class*]() -> i32 {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT: %self.ref: Class* = name_ref self, %self.addr
+// CHECK:STDOUT: %self.ref: Class* = name_ref self, %self
 // CHECK:STDOUT: %.loc19_11: ref Class = deref %self.ref
 // CHECK:STDOUT: %.loc19_17.1: ref i32 = class_element_access %.loc19_11, element0
 // CHECK:STDOUT: %.loc19_17.2: i32 = bind_value %.loc19_17.1

toolchain/check/testdata/class/self_conversion.carbon

@@ -81,9 +81,9 @@ fn Call(p: Derived*) -> i32 {
 // CHECK:STDOUT: return %.loc19_14.2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @AddrSelfBase[%self.addr: Base*]() {
+// CHECK:STDOUT: fn @AddrSelfBase[addr %self: Base*]() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT: %self.ref: Base* = name_ref self, %self.addr
+// CHECK:STDOUT: %self.ref: Base* = name_ref self, %self
 // CHECK:STDOUT: %.loc23_4: ref Base = deref %self.ref
 // CHECK:STDOUT: %.loc23_10: ref i32 = class_element_access %.loc23_4, element0
 // CHECK:STDOUT: %.loc23_15: i32 = int_literal 1