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

@@ -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",

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<SemIR::InstId> arg_ids)
+auto PerformCallToFunction(Context& context, SemIR::LocId loc_id,
+                           SemIR::InstId callee_id,
+                           const SemIR::CalleeFunction& callee_function,
+                           llvm::ArrayRef<SemIR::InstId> 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);
     }
   }
 }

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<SemIR::InstId> 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<SemIR::InstId> arg_ids) -> SemIR::InstId;

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<SemIR::InstId> 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

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<SemIR::InstId> arg_ids)
+    -> SemIR::InstId;
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_CPP_CALL_H_

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<clang::CXXMethodDecl>(decl);
+  return clang_method_decl && clang_method_decl->isOverloadedOperator();
+}
+
 }  // namespace Carbon::Check

toolchain/check/cpp/operators.h

@@ -17,6 +17,9 @@ namespace Carbon::Check {
 auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
                        llvm::ArrayRef<SemIR::InstId> 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_

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<clang::CXXMethodDecl>(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<SemIR::InstId> arg_ids)
+auto PerformCppOverloadResolution(Context& context, SemIR::LocId loc_id,
+                                  SemIR::CppOverloadSetId overload_set_id,
+                                  SemIR::InstId self_id,
+                                  llvm::ArrayRef<SemIR::InstId> 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<SemIR::InstId> 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

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<SemIR::CalleeFunction> 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<SemIR::InstId> 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.
-//
-// 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.
+// Resolves which function to call using Clang overloading resolution, or
+// returns an error instruction if overload resolution failed.
 //
-// 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<SemIR::InstId> arg_ids)
-    -> CppOverloadResolutionResult;
+    -> SemIR::InstId;
 
 }  // namespace Carbon::Check