فهرست منبع

Remove CalleeParamsInfo (#4452)

I'm seeing three issues with CalleeParamsInfo:

1. Although ResolveCalleeInCall had been extracted out and the
EntityWithParamsBase could be used directly, that had not been cleaned
up.
2. implicit_param_refs_id is unused; param_refs_id was only used by
ResolveCalleeInCall. The factoring as a struct seems to be obscuring
what's used and what isn't (creating unnecessary copies).
3. On #4446, CalleeParamsInfo seemed to be obfuscating what
ConvertCallArgs actually worked with (a Function, not a generic entity).

I'm thinking that just removing CalleeParamsInfo is the best resolution
here, it looks like it's tripping people up more than it's helping.

Note, I think Function used to be named Callable, which was where the
"callable" name originally came from (I might be wrong about this). But
"function" seems clearer about the type now.
Jon Ross-Perkins 1 سال پیش
والد
کامیت
bd2fa3ace7
3فایلهای تغییر یافته به همراه28 افزوده شده و 46 حذف شده
  1. 9 12
      toolchain/check/call.cpp
  2. 12 6
      toolchain/check/convert.cpp
  3. 7 28
      toolchain/check/convert.h

+ 9 - 12
toolchain/check/call.cpp

@@ -46,10 +46,8 @@ static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
                                 SemIR::InstId self_id,
                                 llvm::ArrayRef<SemIR::InstId> arg_ids)
     -> std::optional<SemIR::SpecificId> {
-  CalleeParamsInfo callee_info(entity);
-
   // Check that the arity matches.
-  auto params = context.inst_blocks().GetOrEmpty(callee_info.param_refs_id);
+  auto params = context.inst_blocks().GetOrEmpty(entity.param_refs_id);
   if (arg_ids.size() != params.size()) {
     CARBON_DIAGNOSTIC(CallArgCountMismatch, Error,
                       "{0} argument{0:s} passed to "
@@ -64,7 +62,7 @@ static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
     context.emitter()
         .Build(loc_id, CallArgCountMismatch, arg_ids.size(),
                static_cast<int>(entity_kind_for_diagnostic), params.size())
-        .Note(callee_info.callee_loc, InCallToEntity,
+        .Note(entity.latest_decl_id(), InCallToEntity,
               static_cast<int>(entity_kind_for_diagnostic))
         .Emit();
     return std::nullopt;
@@ -75,8 +73,8 @@ static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
   if (entity_generic_id.is_valid()) {
     specific_id = DeduceGenericCallArguments(
         context, loc_id, entity_generic_id, enclosing_specific_id,
-        callee_info.implicit_param_patterns_id, callee_info.param_patterns_id,
-        self_id, arg_ids);
+        entity.implicit_param_patterns_id, entity.param_patterns_id, self_id,
+        arg_ids);
     if (!specific_id.is_valid()) {
       return std::nullopt;
     }
@@ -153,12 +151,12 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
       }
     }
   }
-  auto& callable = context.functions().Get(callee_function.function_id);
+  auto& function = context.functions().Get(callee_function.function_id);
 
   // If the callee is a generic function, determine the generic argument values
   // for the call.
   auto callee_specific_id = ResolveCalleeInCall(
-      context, loc_id, callable, EntityKind::Function, callable.generic_id,
+      context, loc_id, function, EntityKind::Function, function.generic_id,
       callee_function.enclosing_specific_id, callee_function.self_id, arg_ids);
   if (!callee_specific_id) {
     return SemIR::InstId::BuiltinError;
@@ -181,9 +179,9 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
         &context.emitter(), [&](auto& builder) {
           CARBON_DIAGNOSTIC(IncompleteReturnTypeHere, Note,
                             "return type declared here");
-          builder.Note(callable.return_slot_id, IncompleteReturnTypeHere);
+          builder.Note(function.return_slot_id, IncompleteReturnTypeHere);
         });
-    return CheckFunctionReturnType(context, callee_id, callable,
+    return CheckFunctionReturnType(context, callee_id, function,
                                    *callee_specific_id);
   }();
   switch (return_info.init_repr.kind) {
@@ -212,8 +210,7 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
   // Convert the arguments to match the parameters.
   auto converted_args_id =
       ConvertCallArgs(context, loc_id, callee_function.self_id, arg_ids,
-                      return_slot_arg_id, CalleeParamsInfo(callable),
-                      callable.return_slot_pattern_id, *callee_specific_id);
+                      return_slot_arg_id, function, *callee_specific_id);
   auto call_inst_id =
       context.AddInst<SemIR::Call>(loc_id, {.type_id = return_info.type_id,
                                             .callee_id = callee_id,

+ 12 - 6
toolchain/check/convert.cpp

@@ -1160,15 +1160,21 @@ auto ConvertForExplicitAs(Context& context, Parse::NodeId as_node,
 }
 
 // TODO: consider moving this to pattern_match.h
-auto ConvertCallArgs(
-    Context& context, SemIR::LocId call_loc_id, SemIR::InstId self_id,
-    llvm::ArrayRef<SemIR::InstId> arg_refs, SemIR::InstId return_slot_arg_id,
-    const CalleeParamsInfo& callee, SemIR::InstId return_slot_pattern_id,
-    SemIR::SpecificId callee_specific_id) -> SemIR::InstBlockId {
+auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
+                     SemIR::InstId self_id,
+                     llvm::ArrayRef<SemIR::InstId> arg_refs,
+                     SemIR::InstId return_slot_arg_id,
+                     const SemIR::Function& callee,
+                     SemIR::SpecificId callee_specific_id)
+    -> SemIR::InstBlockId {
+  // 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());
@@ -1190,7 +1196,7 @@ auto ConvertCallArgs(
     CARBON_DIAGNOSTIC(InCallToFunction, Note, "calling function declared here");
     context.emitter()
         .Build(call_loc_id, MissingObjectInMethodCall)
-        .Note(callee.callee_loc, InCallToFunction)
+        .Note(callee_decl_id, InCallToFunction)
         .Emit();
     self_id = SemIR::InstId::BuiltinError;
   }

+ 7 - 28
toolchain/check/convert.h

@@ -89,37 +89,16 @@ auto ConvertForExplicitAs(Context& context, Parse::NodeId as_node,
                           SemIR::InstId value_id, SemIR::TypeId type_id)
     -> SemIR::InstId;
 
-// Information about the syntactic parameters of a callee (excluding the return
-// slot, for example). This information is extracted from the
-// EntityWithParamsBase before calling ConvertCallArgs, because conversion can
-// trigger importing of more entities, which can invalidate the reference to the
-// callee.
-struct CalleeParamsInfo {
-  explicit CalleeParamsInfo(const SemIR::EntityWithParamsBase& callee)
-      : callee_loc(callee.latest_decl_id()),
-        implicit_param_refs_id(callee.implicit_param_refs_id),
-        implicit_param_patterns_id(callee.implicit_param_patterns_id),
-        param_refs_id(callee.param_refs_id),
-        param_patterns_id(callee.param_patterns_id) {}
-
-  // The location of the callee to use in diagnostics.
-  SemIRLoc callee_loc;
-  // The implicit parameters of the callee.
-  SemIR::InstBlockId implicit_param_refs_id;
-  SemIR::InstBlockId implicit_param_patterns_id;
-  // The explicit parameters of the callee.
-  SemIR::InstBlockId param_refs_id;
-  SemIR::InstBlockId param_patterns_id;
-};
-
 // Implicitly converts a set of arguments to match the parameter types in a
 // function call. Returns a block containing the converted implicit and explicit
 // argument values for runtime parameters.
-auto ConvertCallArgs(
-    Context& context, SemIR::LocId call_loc_id, SemIR::InstId self_id,
-    llvm::ArrayRef<SemIR::InstId> arg_refs, SemIR::InstId return_slot_arg_id,
-    const CalleeParamsInfo& callee, SemIR::InstId return_slot_pattern_id,
-    SemIR::SpecificId callee_specific_id) -> SemIR::InstBlockId;
+auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
+                     SemIR::InstId self_id,
+                     llvm::ArrayRef<SemIR::InstId> arg_refs,
+                     SemIR::InstId return_slot_arg_id,
+                     const SemIR::Function& callee,
+                     SemIR::SpecificId callee_specific_id)
+    -> SemIR::InstBlockId;
 
 // A type that has been converted for use as a type expression.
 struct TypeExpr {