From 5705b94da840efeaac83edfe42c3243e362b1a9d Mon Sep 17 00:00:00 2001 From: Boaz Brickner Date: Mon, 29 Sep 2025 16:26:23 +0200 Subject: [PATCH] Add `PerformCallToCppFunction()` which calls simplified version of `PerformCppOverloadResolution()` before calling `PerformCallToFunction()` (#6122) Instead of calling `PerformCppOverloadResolution()` and use the complex return value to call `PerformCallToFunction()`, we call `PerformCallToCppFunction()` which will call both `PerformCppOverloadResolution()` and `PerformCallToFunction()`. Followup of #6112. Part of #5995. --- toolchain/check/BUILD | 2 + toolchain/check/call.cpp | 25 +++----- toolchain/check/call.h | 8 +++ toolchain/check/cpp/call.cpp | 59 +++++++++++++++++++ toolchain/check/cpp/call.h | 36 ++++++++++++ toolchain/check/cpp/operators.cpp | 5 ++ toolchain/check/cpp/operators.h | 3 + toolchain/check/cpp/overload_resolution.cpp | 65 ++------------------- toolchain/check/cpp/overload_resolution.h | 39 +++---------- 9 files changed, 135 insertions(+), 107 deletions(-) create mode 100644 toolchain/check/cpp/call.cpp create mode 100644 toolchain/check/cpp/call.h diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 607db96da212..dee28498957a 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -21,6 +21,7 @@ cc_library( "context.cpp", "control_flow.cpp", "convert.cpp", + "cpp/call.cpp", "cpp/custom_type_mapping.cpp", "cpp/import.cpp", "cpp/location.cpp", @@ -72,6 +73,7 @@ cc_library( "context.h", "control_flow.h", "convert.h", + "cpp/call.h", "cpp/custom_type_mapping.h", "cpp/import.h", "cpp/location.h", diff --git a/toolchain/check/call.cpp b/toolchain/check/call.cpp index 9447dfa428b7..b9f2e347da77 100644 --- a/toolchain/check/call.cpp +++ b/toolchain/check/call.cpp @@ -10,7 +10,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/control_flow.h" #include "toolchain/check/convert.h" -#include "toolchain/check/cpp/overload_resolution.h" +#include "toolchain/check/cpp/call.h" #include "toolchain/check/cpp/thunk.h" #include "toolchain/check/deduce.h" #include "toolchain/check/facet_type.h" @@ -201,11 +201,10 @@ static auto CheckCalleeFunctionReturnType(Context& context, SemIR::LocId loc_id, return CheckFunctionReturnType(context, loc_id, function, callee_specific_id); } -// Performs a call where the callee is a function. -static auto PerformCallToFunction(Context& context, SemIR::LocId loc_id, - SemIR::InstId callee_id, - const SemIR::CalleeFunction& callee_function, - llvm::ArrayRef arg_ids) +auto PerformCallToFunction(Context& context, SemIR::LocId loc_id, + SemIR::InstId callee_id, + const SemIR::CalleeFunction& callee_function, + llvm::ArrayRef arg_ids) -> SemIR::InstId { // If the callee is a generic function, determine the generic argument values // for the call. @@ -337,17 +336,9 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id, } case CARBON_KIND(SemIR::CalleeCppOverloadSet overload): { - CppOverloadResolutionResult overload_result = - PerformCppOverloadResolution(context, loc_id, - overload.cpp_overload_set_id, - overload.self_id, arg_ids); - if (overload_result.callee_id == SemIR::ErrorInst::InstId) { - return SemIR::ErrorInst::InstId; - } - CARBON_CHECK(overload_result.callee_function); - return PerformCallToFunction(context, loc_id, overload_result.callee_id, - *overload_result.callee_function, - overload_result.arg_ids); + return PerformCallToCppFunction(context, loc_id, + overload.cpp_overload_set_id, + overload.self_id, arg_ids); } } } diff --git a/toolchain/check/call.h b/toolchain/check/call.h index 3178b319355a..a42a0d9ebb08 100644 --- a/toolchain/check/call.h +++ b/toolchain/check/call.h @@ -10,6 +10,14 @@ namespace Carbon::Check { +// Checks and builds SemIR for a call to `callee_id` with arguments `args_id`, +// where the callee is a function. +auto PerformCallToFunction(Context& context, SemIR::LocId loc_id, + SemIR::InstId callee_id, + const SemIR::CalleeFunction& callee_function, + llvm::ArrayRef arg_ids) + -> SemIR::InstId; + // Checks and builds SemIR for a call to `callee_id` with arguments `args_id`. auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id, llvm::ArrayRef arg_ids) -> SemIR::InstId; diff --git a/toolchain/check/cpp/call.cpp b/toolchain/check/cpp/call.cpp new file mode 100644 index 000000000000..ecde9c8f0971 --- /dev/null +++ b/toolchain/check/cpp/call.cpp @@ -0,0 +1,59 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/check/cpp/call.h" + +#include "toolchain/base/kind_switch.h" +#include "toolchain/check/call.h" +#include "toolchain/check/cpp/operators.h" +#include "toolchain/check/cpp/overload_resolution.h" +#include "toolchain/sem_ir/function.h" +#include "toolchain/sem_ir/ids.h" +#include "toolchain/sem_ir/typed_insts.h" + +namespace Carbon::Check { + +// Returns whether the function is an imported C++ operator member function. +static auto IsCppOperatorMethod(Context& context, SemIR::FunctionId function_id) + -> bool { + SemIR::ClangDeclId clang_decl_id = + context.functions().Get(function_id).clang_decl_id; + return clang_decl_id.has_value() && + IsCppOperatorMethodDecl( + context.clang_decls().Get(clang_decl_id).key.decl); +} + +auto PerformCallToCppFunction(Context& context, SemIR::LocId loc_id, + SemIR::CppOverloadSetId overload_set_id, + SemIR::InstId self_id, + llvm::ArrayRef arg_ids) + -> SemIR::InstId { + SemIR::InstId callee_id = PerformCppOverloadResolution( + context, loc_id, overload_set_id, self_id, arg_ids); + SemIR::Callee callee = GetCallee(context.sem_ir(), callee_id); + CARBON_KIND_SWITCH(callee) { + case CARBON_KIND(SemIR::CalleeError _): { + return SemIR::ErrorInst::InstId; + } + case CARBON_KIND(SemIR::CalleeFunction fn): { + CARBON_CHECK(!fn.self_id.has_value()); + if (self_id.has_value()) { + // Preserve the `self` argument from the original callee. + fn.self_id = self_id; + } else if (IsCppOperatorMethod(context, fn.function_id)) { + // Adjust `self` and args for C++ overloaded operator methods. + fn.self_id = arg_ids.consume_front(); + } + return PerformCallToFunction(context, loc_id, callee_id, fn, arg_ids); + } + case CARBON_KIND(SemIR::CalleeCppOverloadSet _): { + CARBON_FATAL("overloads can't be recursive"); + } + case CARBON_KIND(SemIR::CalleeNonFunction _): { + CARBON_FATAL("overloads should produce functions"); + } + } +} + +} // namespace Carbon::Check diff --git a/toolchain/check/cpp/call.h b/toolchain/check/cpp/call.h new file mode 100644 index 000000000000..02e8b7907839 --- /dev/null +++ b/toolchain/check/cpp/call.h @@ -0,0 +1,36 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CARBON_TOOLCHAIN_CHECK_CPP_CALL_H_ +#define CARBON_TOOLCHAIN_CHECK_CPP_CALL_H_ + +#include "toolchain/check/context.h" +#include "toolchain/sem_ir/ids.h" + +namespace Carbon::Check { + +// Checks and builds SemIR for a call to a C++ function in the given overload +// set with self `self_id` and arguments `arg_ids`. +// +// Chooses the best viable C++ function by performing Clang overloading +// resolution over the overload set. +// +// Preserves the given self, if set. If not set, and the function is a C++ +// member operator, self will be set to the first argument, which in turn will +// be removed from the given args. +// +// A set with a single non-templated function goes through the same rules for +// overloading resolution. This is to make sure that calls that have no viable +// implicit conversion sequence are rejected even when an implicit conversion is +// possible. Keeping the same behavior here for consistency and supporting +// migrations so that the migrated callers from C++ remain valid. +auto PerformCallToCppFunction(Context& context, SemIR::LocId loc_id, + SemIR::CppOverloadSetId overload_set_id, + SemIR::InstId self_id, + llvm::ArrayRef arg_ids) + -> SemIR::InstId; + +} // namespace Carbon::Check + +#endif // CARBON_TOOLCHAIN_CHECK_CPP_CALL_H_ diff --git a/toolchain/check/cpp/operators.cpp b/toolchain/check/cpp/operators.cpp index b848ba843ef4..dda8d2d4d4b6 100644 --- a/toolchain/check/cpp/operators.cpp +++ b/toolchain/check/cpp/operators.cpp @@ -210,4 +210,9 @@ auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op, /*naming_class=*/nullptr, std::move(functions)); } +auto IsCppOperatorMethodDecl(clang::Decl* decl) -> bool { + auto* clang_method_decl = dyn_cast(decl); + return clang_method_decl && clang_method_decl->isOverloadedOperator(); +} + } // namespace Carbon::Check diff --git a/toolchain/check/cpp/operators.h b/toolchain/check/cpp/operators.h index bf4c22f2cd38..29fb9ef5f4d1 100644 --- a/toolchain/check/cpp/operators.h +++ b/toolchain/check/cpp/operators.h @@ -17,6 +17,9 @@ namespace Carbon::Check { auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op, llvm::ArrayRef arg_ids) -> SemIR::InstId; +// Returns whether the decl is an operator member function. +auto IsCppOperatorMethodDecl(clang::Decl* decl) -> bool; + } // namespace Carbon::Check #endif // CARBON_TOOLCHAIN_CHECK_CPP_OPERATORS_H_ diff --git a/toolchain/check/cpp/overload_resolution.cpp b/toolchain/check/cpp/overload_resolution.cpp index 2dd36addd3b2..b4a7bd3404ae 100644 --- a/toolchain/check/cpp/overload_resolution.cpp +++ b/toolchain/check/cpp/overload_resolution.cpp @@ -10,6 +10,7 @@ #include "toolchain/base/kind_switch.h" #include "toolchain/check/cpp/import.h" #include "toolchain/check/cpp/location.h" +#include "toolchain/check/cpp/operators.h" #include "toolchain/check/cpp/type_mapping.h" #include "toolchain/check/member_access.h" #include "toolchain/check/name_lookup.h" @@ -116,18 +117,10 @@ static auto CheckOverloadAccess(Context& context, SemIR::LocId loc_id, .highest_allowed_access = allowed_access_kind}); } -// Returns whether the decl is an operator member function. -static auto IsOperatorMethodDecl(clang::Decl* decl) -> bool { - auto* clang_method_decl = dyn_cast(decl); - return clang_method_decl && clang_method_decl->isOverloadedOperator(); -} - -// Resolve which function to call, or returns an error instruction if overload -// resolution failed. -static auto ResolveCalleeId(Context& context, SemIR::LocId loc_id, - SemIR::CppOverloadSetId overload_set_id, - SemIR::InstId self_id, - llvm::ArrayRef arg_ids) +auto PerformCppOverloadResolution(Context& context, SemIR::LocId loc_id, + SemIR::CppOverloadSetId overload_set_id, + SemIR::InstId self_id, + llvm::ArrayRef arg_ids) -> SemIR::InstId { // Register an annotation scope to flush any Clang diagnostics when we return. // This is important to ensure that Clang diagnostics are properly interleaved @@ -177,7 +170,7 @@ static auto ResolveCalleeId(Context& context, SemIR::LocId loc_id, context, loc_id, best_viable_fn->Function, // If this is an operator method, the first arg will be used as self. arg_exprs.size() - - (IsOperatorMethodDecl(best_viable_fn->Function) ? 1 : 0)); + (IsCppOperatorMethodDecl(best_viable_fn->Function) ? 1 : 0)); CheckOverloadAccess(context, loc_id, overload_set, best_viable_fn->FoundDecl, result_id); return result_id; @@ -208,50 +201,4 @@ static auto ResolveCalleeId(Context& context, SemIR::LocId loc_id, } } -// Returns whether the function is an imported C++ operator member function. -static auto IsCppOperatorMethod(Context& context, SemIR::FunctionId function_id) - -> bool { - SemIR::ClangDeclId clang_decl_id = - context.functions().Get(function_id).clang_decl_id; - return clang_decl_id.has_value() && - IsOperatorMethodDecl( - context.clang_decls().Get(clang_decl_id).key.decl); -} - -auto PerformCppOverloadResolution(Context& context, SemIR::LocId loc_id, - SemIR::CppOverloadSetId overload_set_id, - SemIR::InstId self_id, - llvm::ArrayRef arg_ids) - -> CppOverloadResolutionResult { - CppOverloadResolutionResult result = { - .callee_id = - ResolveCalleeId(context, loc_id, overload_set_id, self_id, arg_ids), - .arg_ids = arg_ids}; - SemIR::Callee callee = GetCallee(context.sem_ir(), result.callee_id); - CARBON_KIND_SWITCH(callee) { - case CARBON_KIND(SemIR::CalleeError _): { - result.callee_id = SemIR::ErrorInst::InstId; - return result; - } - case CARBON_KIND(SemIR::CalleeFunction fn): { - CARBON_CHECK(!fn.self_id.has_value()); - if (self_id.has_value()) { - // Preserve the `self` argument from the original callee. - fn.self_id = self_id; - } else if (IsCppOperatorMethod(context, fn.function_id)) { - // Adjust `self` and args for C++ overloaded operator methods. - fn.self_id = result.arg_ids.consume_front(); - } - result.callee_function = fn; - return result; - } - case CARBON_KIND(SemIR::CalleeCppOverloadSet _): { - CARBON_FATAL("overloads can't be recursive"); - } - case CARBON_KIND(SemIR::CalleeNonFunction _): { - CARBON_FATAL("overloads should produce functions"); - } - } -} - } // namespace Carbon::Check diff --git a/toolchain/check/cpp/overload_resolution.h b/toolchain/check/cpp/overload_resolution.h index cad8763caec7..c08831667887 100644 --- a/toolchain/check/cpp/overload_resolution.h +++ b/toolchain/check/cpp/overload_resolution.h @@ -11,42 +11,19 @@ namespace Carbon::Check { -// The result of performing C++ overload resolution. -struct CppOverloadResolutionResult { - // The resolved callee id, or ErrorInst::InstId on error. - SemIR::InstId callee_id; - - // The resolved callee, which may be different from the result of - // `GetCalleeFunction()` to preserve self or set it to be the first argument - // for C++ member operators. Not set if overload resolution failed. - std::optional callee_function; - - // The arguments to pass to the callee, which may be different from the - // original arguments if the callee is a C++ member operator. - llvm::ArrayRef arg_ids; -}; - -// Performs overloading resolution for a call to an overloaded C++ set. A set -// with a single non-templated function goes through the same rules for -// overloading resolution. Uses Clang to find the best viable function for the -// call. +// Resolves which function to call using Clang overloading resolution, or +// returns an error instruction if overload resolution failed. // -// The callee function preserves the given self, if set. If not set, and the -// function is a a C++ member operator, self will be set to the first argument, -// which in turn will be removed from the given args. -// -// Note on non-overloaded functions: In C++, a single non-templated function is -// also treated as an overloaded set and goes through the overload resolution to -// ensure that the function is viable for the call. This is to make sure that -// calls that have no viable implicit conversion sequence are rejected even when -// an implicit conversion is possible. Keeping the same behavior here for -// consistency and supporting migrations so that the migrated callers from C++ -// remain valid. +// A set with a single non-templated function goes through the same rules for +// overloading resolution. This is to make sure that calls that have no viable +// implicit conversion sequence are rejected even when an implicit conversion is +// possible. Keeping the same behavior here for consistency and supporting +// migrations so that the migrated callers from C++ remain valid. auto PerformCppOverloadResolution(Context& context, SemIR::LocId loc_id, SemIR::CppOverloadSetId overload_set_id, SemIR::InstId self_id, llvm::ArrayRef arg_ids) - -> CppOverloadResolutionResult; + -> SemIR::InstId; } // namespace Carbon::Check