From fbb4ecf3190b8f267e8f5644d71ff88f607617f2 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 14 Dec 2023 12:53:51 -0800 Subject: [PATCH] Remove `SelfParam`, add an `AddrPattern` instead. (#3506) This is intended to make the representation of a `self` pattern be more similar to other patterns. --- toolchain/check/context.cpp | 2 +- toolchain/check/convert.cpp | 24 +++++++---- toolchain/check/handle_binding_pattern.cpp | 40 ++++++++---------- toolchain/check/handle_function.cpp | 10 +++-- toolchain/check/handle_name.cpp | 42 +++++++++++++------ toolchain/check/node_stack.h | 3 +- .../check/testdata/class/base_method.carbon | 4 +- .../testdata/class/base_method_shadow.carbon | 6 +-- .../testdata/class/fail_addr_self.carbon | 4 +- .../class/fail_memaccess_category.carbon | 2 +- .../check/testdata/class/fail_self.carbon | 8 ++-- toolchain/check/testdata/class/method.carbon | 2 +- .../check/testdata/class/raw_self.carbon | 6 +-- toolchain/check/testdata/class/self.carbon | 4 +- .../testdata/class/self_conversion.carbon | 4 +- toolchain/lower/file_context.cpp | 30 +++++++++---- toolchain/lower/handle.cpp | 10 ++--- toolchain/sem_ir/file.cpp | 6 +-- toolchain/sem_ir/formatter.cpp | 19 ++++++--- toolchain/sem_ir/inst_kind.def | 2 +- toolchain/sem_ir/typed_insts.h | 18 ++++---- toolchain/sem_ir/value_stores.h | 7 ++++ 22 files changed, 152 insertions(+), 101 deletions(-) diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index ef10834bdf78..d868e95a5689 100644 --- a/toolchain/check/context.cpp +++ b/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: diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 1ae44f88f0db..dc0ef6c8eb05 100644 --- a/toolchain/check/convert.cpp +++ b/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 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()) { - 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(); + if (addr_pattern) { + pattern = context.insts().Get(addr_pattern->inner_id); + } + if (auto param = pattern.TryAs(); + 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; } diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp index e014d51981f3..906beff18511 100644 --- a/toolchain/check/handle_binding_pattern.cpp +++ b/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(); - if (auto self_param = - context.insts().Get(self_param_id).TryAs()) { - self_param->is_addr_self = SemIR::BoolValue::True; - context.insts().Set(self_param_id, *self_param); + context.node_stack().Pop(); + auto self_param = context.insts().TryGetAs(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()) { - 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()) { + 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. diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 314e9718462c..eaf8ef07d472 100644 --- a/toolchain/check/handle_function.cpp +++ b/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()) { + 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()) { context.AddNameToLookup(fn_param->parse_node, fn_param->name_id, param_id); - } else if (auto self_param = param.TryAs()) { - context.AddNameToLookup(self_param->parse_node, SemIR::NameId::SelfValue, - param_id); } else { CARBON_FATAL() << "Unexpected kind of parameter in function definition " << param; diff --git a/toolchain/check/handle_name.cpp b/toolchain/check/handle_name.cpp index b84c87bba445..a4c1195f2f31 100644 --- a/toolchain/check/handle_name.cpp +++ b/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()) { + inst = sem_ir.insts().Get(addr->inner_id); + } + if (auto param = inst.TryAs(); + 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()) { - 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; } diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h index dadb493ed2cb..d5ce348737b9 100644 --- a/toolchain/check/node_stack.h +++ b/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: diff --git a/toolchain/check/testdata/class/base_method.carbon b/toolchain/check/testdata/class/base_method.carbon index 26cbfc101697..e86402782295 100644 --- a/toolchain/check/testdata/class/base_method.carbon +++ b/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 diff --git a/toolchain/check/testdata/class/base_method_shadow.carbon b/toolchain/check/testdata/class/base_method_shadow.carbon index b7062c63aa05..9b67615c6f5c 100644 --- a/toolchain/check/testdata/class/base_method_shadow.carbon +++ b/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: diff --git a/toolchain/check/testdata/class/fail_addr_self.carbon b/toolchain/check/testdata/class/fail_addr_self.carbon index 71a197643628..d1f590713d85 100644 --- a/toolchain/check/testdata/class/fail_addr_self.carbon +++ b/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: diff --git a/toolchain/check/testdata/class/fail_memaccess_category.carbon b/toolchain/check/testdata/class/fail_memaccess_category.carbon index 5f50cc852282..efa2fbc8a4e2 100644 --- a/toolchain/check/testdata/class/fail_memaccess_category.carbon +++ b/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: diff --git a/toolchain/check/testdata/class/fail_self.carbon b/toolchain/check/testdata/class/fail_self.carbon index 659999160715..c1ada1e2c7df 100644 --- a/toolchain/check/testdata/class/fail_self.carbon +++ b/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: = name_ref self, +// 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 // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/class/method.carbon b/toolchain/check/testdata/class/method.carbon index 4beeecc4f22e..63d19814db96 100644 --- a/toolchain/check/testdata/class/method.carbon +++ b/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: diff --git a/toolchain/check/testdata/class/raw_self.carbon b/toolchain/check/testdata/class/raw_self.carbon index 5374a42802e6..5612bb3911a7 100644 --- a/toolchain/check/testdata/class/raw_self.carbon +++ b/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: } diff --git a/toolchain/check/testdata/class/self.carbon b/toolchain/check/testdata/class/self.carbon index 85446f18a2da..954ec95e20ab 100644 --- a/toolchain/check/testdata/class/self.carbon +++ b/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 diff --git a/toolchain/check/testdata/class/self_conversion.carbon b/toolchain/check/testdata/class/self_conversion.carbon index 13f1ee02191d..35974640e462 100644 --- a/toolchain/check/testdata/class/self_conversion.carbon +++ b/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 diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index 12300c929db0..36f63301b3fb 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -73,6 +73,21 @@ auto FileContext::GetGlobal(SemIR::InstId inst_id) -> llvm::Value* { CARBON_FATAL() << "Missing value: " << inst_id << " " << target; } +// Returns the parameter for the given instruction from a function parameter +// list. +static auto GetAsParam(const SemIR::File& sem_ir, SemIR::InstId pattern_id) + -> std::pair { + auto pattern = sem_ir.insts().Get(pattern_id); + + // Step over `addr`. + if (auto addr = pattern.TryAs()) { + pattern_id = addr->inner_id; + pattern = sem_ir.insts().Get(pattern_id); + } + + return {pattern_id, pattern.As()}; +} + auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id) -> llvm::Function* { const auto& function = sem_ir().functions().Get(function_id); @@ -103,7 +118,7 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id) } for (auto param_ref_id : llvm::concat(implicit_param_refs, param_refs)) { - auto param_type_id = sem_ir().insts().Get(param_ref_id).type_id(); + auto param_type_id = GetAsParam(sem_ir(), param_ref_id).second.type_id; switch (auto value_rep = SemIR::GetValueRepr(sem_ir(), param_type_id); value_rep.kind) { case SemIR::ValueRepr::Unknown: @@ -148,16 +163,13 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id) // Set up parameters and the return slot. for (auto [inst_id, arg] : llvm::zip_equal(param_inst_ids, llvm_function->args())) { - auto inst = sem_ir().insts().Get(inst_id); auto name_id = SemIR::NameId::Invalid; if (inst_id == function.return_slot_id) { name_id = SemIR::NameId::ReturnSlot; arg.addAttr(llvm::Attribute::getWithStructRetType( llvm_context(), GetType(function.return_type_id))); - } else if (inst.Is()) { - name_id = SemIR::NameId::SelfValue; } else { - name_id = inst.As().name_id; + name_id = GetAsParam(sem_ir(), inst_id).second.name_id; } arg.setName(sem_ir().names().GetIRBaseName(name_id)); } @@ -194,14 +206,14 @@ auto FileContext::BuildFunctionDefinition(SemIR::FunctionId function_id) } for (auto param_ref_id : llvm::concat(implicit_param_refs, param_refs)) { - auto param_type_id = sem_ir().insts().Get(param_ref_id).type_id(); + auto [param_id, param] = GetAsParam(sem_ir(), param_ref_id); + auto param_type_id = param.type_id; if (SemIR::GetValueRepr(sem_ir(), param_type_id).kind == SemIR::ValueRepr::None) { function_lowering.SetLocal( - param_ref_id, llvm::PoisonValue::get(GetType(param_type_id))); + param_id, llvm::PoisonValue::get(GetType(param_type_id))); } else { - function_lowering.SetLocal(param_ref_id, - llvm_function->getArg(param_index)); + function_lowering.SetLocal(param_id, llvm_function->getArg(param_index)); ++param_index; } } diff --git a/toolchain/lower/handle.cpp b/toolchain/lower/handle.cpp index 7fdad7f9c5d0..9b8cd0e0e500 100644 --- a/toolchain/lower/handle.cpp +++ b/toolchain/lower/handle.cpp @@ -30,6 +30,11 @@ auto HandleAddressOf(FunctionContext& context, SemIR::InstId inst_id, context.SetLocal(inst_id, context.GetValue(inst.lvalue_id)); } +auto HandleAddrPattern(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/, + SemIR::AddrPattern /*inst*/) -> void { + CARBON_FATAL() << "`addr` should be lowered by `BuildFunctionDefinition`"; +} + auto HandleArrayIndex(FunctionContext& context, SemIR::InstId inst_id, SemIR::ArrayIndex inst) -> void { auto* array_value = context.GetValue(inst.array_id); @@ -277,11 +282,6 @@ auto HandleReturnExpr(FunctionContext& context, SemIR::InstId /*inst_id*/, } } -auto HandleSelfParam(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/, - SemIR::SelfParam /*inst*/) -> void { - CARBON_FATAL() << "Parameters should be lowered by `BuildFunctionDefinition`"; -} - auto HandleSpliceBlock(FunctionContext& context, SemIR::InstId inst_id, SemIR::SpliceBlock inst) -> void { context.LowerBlock(inst.block_id); diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index e5d6ab6266f7..2ec788e23d43 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -184,6 +184,7 @@ static auto GetTypePrecedence(InstKind kind) -> int { return 0; case AddressOf::Kind: + case AddrPattern::Kind: case ArrayIndex::Kind: case ArrayInit::Kind: case Assign::Kind: @@ -215,7 +216,6 @@ static auto GetTypePrecedence(InstKind kind) -> int { case RealLiteral::Kind: case Return::Kind: case ReturnExpr::Kind: - case SelfParam::Kind: case SpliceBlock::Kind: case StringLiteral::Kind: case StructAccess::Kind: @@ -384,6 +384,7 @@ auto File::StringifyTypeExpr(InstId outer_inst_id) const -> std::string { break; } case AddressOf::Kind: + case AddrPattern::Kind: case ArrayIndex::Kind: case ArrayInit::Kind: case Assign::Kind: @@ -417,7 +418,6 @@ auto File::StringifyTypeExpr(InstId outer_inst_id) const -> std::string { case RealLiteral::Kind: case Return::Kind: case ReturnExpr::Kind: - case SelfParam::Kind: case SpliceBlock::Kind: case StringLiteral::Kind: case StructAccess::Kind: @@ -494,6 +494,7 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { } case AddressOf::Kind: + case AddrPattern::Kind: case ArrayType::Kind: case BindValue::Kind: case BlockArg::Kind: @@ -505,7 +506,6 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { case Param::Kind: case PointerType::Kind: case RealLiteral::Kind: - case SelfParam::Kind: case StringLiteral::Kind: case StructValue::Kind: case StructType::Kind: diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 437576849b96..16e0a1dd06b3 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -447,6 +447,14 @@ class InstNamer { }; switch (inst.kind()) { + case AddrPattern::Kind: { + // TODO: We need to assign names to parameters that appear in + // function declarations, which may be nested within a pattern. For + // now, just look through `addr`, but we should find a better way to + // visit parameters. + CollectNamesInBlock(scope_idx, inst.As().inner_id); + break; + } case Branch::Kind: { AddBlockLabel(scope_idx, inst.As().target_id, inst); break; @@ -507,11 +515,6 @@ class InstNamer { add_inst_name_id(inst.As().name_id); continue; } - case SelfParam::Kind: { - add_inst_name(inst.As().is_addr_self.index ? "self.addr" - : "self"); - continue; - } case VarStorage::Kind: { add_inst_name_id(inst.As().name_id, ".var"); continue; @@ -682,12 +685,16 @@ class Formatter { auto FormatParamList(InstBlockId param_refs_id) -> void { llvm::ListSeparator sep; - for (const InstId param_id : sem_ir_.inst_blocks().Get(param_refs_id)) { + for (InstId param_id : sem_ir_.inst_blocks().Get(param_refs_id)) { out_ << sep; if (!param_id.is_valid()) { out_ << "invalid"; continue; } + if (auto addr = sem_ir_.insts().TryGetAs(param_id)) { + out_ << "addr "; + param_id = addr->inner_id; + } FormatInstName(param_id); out_ << ": "; FormatType(sem_ir_.insts().Get(param_id).type_id()); diff --git a/toolchain/sem_ir/inst_kind.def b/toolchain/sem_ir/inst_kind.def index 53e28e669597..617549d33669 100644 --- a/toolchain/sem_ir/inst_kind.def +++ b/toolchain/sem_ir/inst_kind.def @@ -18,6 +18,7 @@ // For each instruction kind declared here there is a matching definition in // `typed_insts.h`. CARBON_SEM_IR_INST_KIND(AddressOf) +CARBON_SEM_IR_INST_KIND(AddrPattern) CARBON_SEM_IR_INST_KIND(ArrayIndex) CARBON_SEM_IR_INST_KIND(ArrayInit) CARBON_SEM_IR_INST_KIND(ArrayType) @@ -56,7 +57,6 @@ CARBON_SEM_IR_INST_KIND(PointerType) CARBON_SEM_IR_INST_KIND(RealLiteral) CARBON_SEM_IR_INST_KIND(ReturnExpr) CARBON_SEM_IR_INST_KIND(Return) -CARBON_SEM_IR_INST_KIND(SelfParam) CARBON_SEM_IR_INST_KIND(SpliceBlock) CARBON_SEM_IR_INST_KIND(StringLiteral) CARBON_SEM_IR_INST_KIND(StructAccess) diff --git a/toolchain/sem_ir/typed_insts.h b/toolchain/sem_ir/typed_insts.h index 89d4783ecee4..36ea1be67bab 100644 --- a/toolchain/sem_ir/typed_insts.h +++ b/toolchain/sem_ir/typed_insts.h @@ -44,6 +44,15 @@ struct AddressOf { InstId lvalue_id; }; +struct AddrPattern { + static constexpr auto Kind = InstKind::AddrPattern.Define("addr_pattern"); + + Parse::NodeId parse_node; + TypeId type_id; + // The `self` parameter. + InstId inner_id; +}; + struct ArrayIndex { static constexpr auto Kind = InstKind::ArrayIndex.Define("array_index"); @@ -420,15 +429,6 @@ struct ReturnExpr { InstId expr_id; }; -struct SelfParam { - static constexpr auto Kind = InstKind::SelfParam.Define("self_param"); - static constexpr llvm::StringLiteral Name = "self"; - - Parse::NodeId parse_node; - TypeId type_id; - BoolValue is_addr_self; -}; - struct SpliceBlock { static constexpr auto Kind = InstKind::SpliceBlock.Define("splice_block"); diff --git a/toolchain/sem_ir/value_stores.h b/toolchain/sem_ir/value_stores.h index 2bd87e90a686..aaef8b951e4b 100644 --- a/toolchain/sem_ir/value_stores.h +++ b/toolchain/sem_ir/value_stores.h @@ -33,6 +33,13 @@ class InstStore { return Get(inst_id).As(); } + // Returns the requested instruction as the specified type, if it is of that + // type. + template + auto TryGetAs(InstId inst_id) const -> std::optional { + return Get(inst_id).TryAs(); + } + // Overwrites a given instruction with a new value. auto Set(InstId inst_id, Inst inst) -> void { values_.Get(inst_id) = inst; }