Refactor implicit Self param into a member on SemIR::Function (carbon…

…-language#4928)

This ensures the data is available for more uses (specifically for
diagnosing virtual/abstract/impl functions on non-instance methods).

It still doesn't quite address the TODO to move the Self param search
all the way back to the param walk in all cases. To do that in the case
that still has a separate search loop, I think we'd have to change
`Check::NameComponent` to carry this information (as it carries the
implicit_param_patterns-id) - though there's comments in NameComponent
suggesting it shouldn't carry function-specific things like
`call_params_id` and `return_slot_pattern_id` - so I wasn't sure if it
was suitable to add more there, but I can - possibly in a follow-up
change.

---------

Co-authored-by: Jon Ross-Perkins <[email protected]>

toolchain/check/convert.cpp

@@ -1319,27 +1319,14 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
   // The callee reference can be invalidated by conversions, so ensure all reads
   // from it are done before conversion calls.
   auto callee_decl_id = callee.latest_decl_id();
-  auto implicit_param_patterns =
-      context.inst_blocks().GetOrEmpty(callee.implicit_param_patterns_id);
   auto param_patterns =
       context.inst_blocks().GetOrEmpty(callee.param_patterns_id);
   auto return_slot_pattern_id = callee.return_slot_pattern_id;
 
   // The caller should have ensured this callee has the right arity.
   CARBON_CHECK(arg_refs.size() == param_patterns.size());
 
-  // Find self parameter pattern.
-  // TODO: Do this during initial traversal of implicit params.
-  auto self_param_id = SemIR::InstId::None;
-  for (auto implicit_param_id : implicit_param_patterns) {
-    if (SemIR::Function::GetNameFromPatternId(
-            context.sem_ir(), implicit_param_id) == SemIR::NameId::SelfValue) {
-      CARBON_CHECK(!self_param_id.has_value());
-      self_param_id = implicit_param_id;
-    }
-  }
-
-  if (self_param_id.has_value() && !self_id.has_value()) {
+  if (callee.self_param_id.has_value() && !self_id.has_value()) {
     CARBON_DIAGNOSTIC(MissingObjectInMethodCall, Error,
                       "missing object argument in method call");
     CARBON_DIAGNOSTIC(InCallToFunction, Note, "calling function declared here");
@@ -1350,7 +1337,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
     self_id = SemIR::ErrorInst::SingletonInstId;
   }
 
-  return CallerPatternMatch(context, callee_specific_id, self_param_id,
+  return CallerPatternMatch(context, callee_specific_id, callee.self_param_id,
                             callee.param_patterns_id, return_slot_pattern_id,
                             self_id, arg_refs, return_slot_arg_id);
 }

toolchain/check/handle_function.cpp

@@ -125,6 +125,7 @@ static auto MergeFunctionRedecl(Context& context,
     // match IDs in the signature.
     prev_function.MergeDefinition(new_function);
     prev_function.return_slot_pattern_id = new_function.return_slot_pattern_id;
+    prev_function.self_param_id = new_function.self_param_id;
   }
   if (prev_import_ir_id.has_value()) {
     ReplacePrevInstForMerge(context, new_function.parent_scope_id,
@@ -258,13 +259,30 @@ static auto BuildFunctionDecl(Context& context,
   auto decl_id =
       context.AddPlaceholderInst(SemIR::LocIdAndInst(node_id, function_decl));
 
+  // Find self parameter pattern.
+  // TODO: Do this during initial traversal of implicit params.
+  auto self_param_id = SemIR::InstId::None;
+  auto implicit_param_patterns =
+      context.inst_blocks().GetOrEmpty(name.implicit_param_patterns_id);
+  if (const auto* i =
+          llvm::find_if(implicit_param_patterns,
+                        [&](auto implicit_param_id) {
+                          return SemIR::Function::GetNameFromPatternId(
+                                     context.sem_ir(), implicit_param_id) ==
+                                 SemIR::NameId::SelfValue;
+                        });
+      i != implicit_param_patterns.end()) {
+    self_param_id = *i;
+  }
+
   // Build the function entity. This will be merged into an existing function if
   // there is one, or otherwise added to the function store.
   auto function_info =
       SemIR::Function{{name_context.MakeEntityWithParamsBase(
                           name, decl_id, is_extern, introducer.extern_library)},
                       {.return_slot_pattern_id = name.return_slot_pattern_id,
-                       .virtual_modifier = virtual_modifier}};
+                       .virtual_modifier = virtual_modifier,
+                       .self_param_id = self_param_id}};
   if (is_definition) {
     function_info.definition_id = decl_id;
   }

toolchain/check/import_ref.cpp

@@ -986,9 +986,14 @@ static auto LoadLocalPatternConstantIds(ImportRefResolver& resolver,
 // take a holistic look at how to balance those concerns. For example,
 // could the same function be used to load the constants and use them, with
 // a parameter to select between the two?
+//
+// `self_param_id` is an optional out parameter, populated with the InstId in
+// the resulting parameter patterns that represents the Self parameter.
 static auto GetLocalParamPatternsId(ImportContext& context,
-                                    SemIR::InstBlockId param_patterns_id)
+                                    SemIR::InstBlockId param_patterns_id,
+                                    SemIR::InstId* self_param_id = nullptr)
     -> SemIR::InstBlockId {
+  CARBON_CHECK(!self_param_id || !self_param_id->has_value());
   if (!param_patterns_id.has_value() ||
       param_patterns_id == SemIR::InstBlockId::Empty) {
     return param_patterns_id;
@@ -1064,6 +1069,11 @@ static auto GetLocalParamPatternsId(ImportContext& context,
               AddImportIRInst(context, addr_pattern_id),
               {.type_id = type_id, .inner_id = new_param_id}));
     }
+    if (self_param_id &&
+        context.import_entity_names().Get(binding.entity_name_id).name_id ==
+            SemIR::NameId::SelfValue) {
+      *self_param_id = new_param_id;
+    }
     new_patterns.push_back(new_param_id);
   }
   return context.local_inst_blocks().Add(new_patterns);
@@ -1934,8 +1944,10 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
 
   // Add the function declaration.
   new_function.parent_scope_id = parent_scope_id;
+  SemIR::InstId self_param_id = SemIR::InstId::None;
   new_function.implicit_param_patterns_id = GetLocalParamPatternsId(
-      resolver, import_function.implicit_param_patterns_id);
+      resolver, import_function.implicit_param_patterns_id, &self_param_id);
+  new_function.self_param_id = self_param_id;
   new_function.param_patterns_id =
       GetLocalParamPatternsId(resolver, import_function.param_patterns_id);
   new_function.return_slot_pattern_id = GetLocalReturnSlotPatternId(

toolchain/sem_ir/function.h

@@ -31,6 +31,10 @@ struct FunctionFields {
   // this function.
   VirtualModifier virtual_modifier;
 
+  // The implicit self parameter, if any, in implicit_param_patterns_id from
+  // EntityWithParamsBase.
+  InstId self_param_id = SemIR::InstId::None;
+
   // The following member is set on the first call to the function, or at the
   // point where the function is defined.