Skip to content

Commit

Permalink
Remove CalleeParamsInfo (#4452)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonmeow authored Nov 4, 2024
1 parent dd43bb9 commit bd2fa3a
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 46 deletions.
21 changes: 9 additions & 12 deletions toolchain/check/call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 12 additions & 6 deletions toolchain/check/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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;
}
Expand Down
35 changes: 7 additions & 28 deletions toolchain/check/convert.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit bd2fa3a

Please sign in to comment.