Support making method calls to C++ overload sets. (#6069)

Fixes #6059

Author: zygoloid

toolchain/check/call.cpp

@@ -300,6 +300,9 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
     return SemIR::ErrorInst::InstId;
   }
   if (callee_function.cpp_overload_set_id.has_value()) {
+    auto self_id = callee_function.self_id;
+    // TODO: Pass self_id into overload resolution so that it's taken into
+    // account when selecting the overload.
     auto resolved_fn_id = PerformCppOverloadResolution(
         context, loc_id, callee_function.cpp_overload_set_id, arg_ids);
     if (!resolved_fn_id) {
@@ -311,6 +314,10 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
       return SemIR::ErrorInst::InstId;
     }
     CARBON_CHECK(!callee_function.cpp_overload_set_id.has_value());
+
+    // Preserve the `self` argument from the original callee.
+    CARBON_CHECK(!callee_function.self_id.has_value());
+    callee_function.self_id = self_id;
   }
   if (callee_function.function_id.has_value()) {
     return PerformCallToFunction(context, loc_id, callee_id, callee_function,

toolchain/check/member_access.cpp

@@ -45,14 +45,34 @@ static auto GetClassElementIndex(Context& context, SemIR::InstId element_id)
   CARBON_FATAL("Unexpected value {0} in class element name", element_inst);
 }
 
-// Returns whether `function_id` is an instance method, that is, whether it has
-// an implicit `self` parameter.
+// Returns whether `function_id` is an instance method: in other words, whether
+// it has an implicit `self` parameter.
 static auto IsInstanceMethod(const SemIR::File& sem_ir,
                              SemIR::FunctionId function_id) -> bool {
   const auto& function = sem_ir.functions().Get(function_id);
   return function.self_param_id.has_value();
 }
 
+// Returns whether the callee function is an instance method, either because
+// it's a Carbon instance method or because it's a C++ overload set that might
+// contain an instance method.
+static auto IsInstanceMethod(const SemIR::File& sem_ir,
+                             const SemIR::CalleeFunction& callee) -> bool {
+  if (callee.function_id.has_value()) {
+    return IsInstanceMethod(sem_ir, callee.function_id);
+  }
+  if (callee.cpp_overload_set_id.has_value()) {
+    // For now, treat all C++ overload sets as potentially containing instance
+    // methods. Overload resolution will handle the case where we actually
+    // found a static method.
+    // TODO: Consider returning `false` if there are no non-instance methods in
+    // the overload set. This would cause us to reject
+    // `instance.(Class.StaticMethod)()` like we do in pure Carbon code.
+    return true;
+  }
+  return false;
+}
+
 // Return whether `type_id`, the type of an associated entity, is for an
 // instance member (currently true only for instance methods).
 static auto IsInstanceType(Context& context, SemIR::TypeId type_id) -> bool {
@@ -372,10 +392,9 @@ static auto PerformInstanceBinding(Context& context, SemIR::LocId loc_id,
                                    SemIR::InstId member_id) -> SemIR::InstId {
   // If the member is a function, check whether it's an instance method.
   if (auto callee = SemIR::GetCalleeFunction(context.sem_ir(), member_id);
-      callee.function_id.has_value()) {
-    if (!IsInstanceMethod(context.sem_ir(), callee.function_id) ||
-        callee.self_id.has_value()) {
-      // Found a static member function or an already-bound method.
+      IsInstanceMethod(context.sem_ir(), callee)) {
+    if (callee.self_id.has_value()) {
+      // Found an already-bound method.
       return member_id;
     }
 

toolchain/check/testdata/interop/cpp/class/access.carbon

@@ -161,19 +161,14 @@ struct S {
   auto foo() -> void;
 };
 
-// --- fail_todo_5891_import_function_member_public.carbon
+// --- import_function_member_public.carbon
 
 library "[[@TEST_NAME]]";
 
 import Cpp library "function_member_public.h";
 
 fn F(s: Cpp.S*) {
   //@dump-sem-ir-begin
-  // CHECK:STDERR: fail_todo_5891_import_function_member_public.carbon:[[@LINE+5]]:3: error: missing object argument in method call [MissingObjectInMethodCall]
-  // CHECK:STDERR:   s->foo();
-  // CHECK:STDERR:   ^~~~~~~~
-  // CHECK:STDERR: fail_todo_5891_import_function_member_public.carbon: note: calling function declared here [InCallToFunction]
-  // CHECK:STDERR:
   s->foo();
   //@dump-sem-ir-end
 }
@@ -432,7 +427,7 @@ fn F() {
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_5891_import_function_member_public.carbon
+// CHECK:STDOUT: --- import_function_member_public.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %S: type = class_type @S [concrete]
@@ -456,10 +451,11 @@ fn F() {
 // CHECK:STDOUT: fn @F(%s.param: %ptr.5c7) {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   %s.ref: %ptr.5c7 = name_ref s, %s
-// CHECK:STDOUT:   %.loc13: ref %S = deref %s.ref
+// CHECK:STDOUT:   %.loc8: ref %S = deref %s.ref
 // CHECK:STDOUT:   %foo.ref: %.177 = name_ref foo, imports.%.dcb [concrete = constants.%empty_struct]
-// CHECK:STDOUT:   <elided>
-// CHECK:STDOUT:   %S.foo.call: init %empty_tuple.type = call imports.%S.foo.decl(<error>)
+// CHECK:STDOUT:   %bound_method: <bound method> = bound_method %.loc8, %foo.ref
+// CHECK:STDOUT:   %addr: %ptr.5c7 = addr_of %.loc8
+// CHECK:STDOUT:   %S.foo.call: init %empty_tuple.type = call imports.%S.foo.decl(%addr)
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

toolchain/check/testdata/interop/cpp/class/base.carbon

@@ -10,8 +10,6 @@
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/class/base.carbon
 
-// TODO: Tests marked as `fail_todo_5891_` to fixed as a follow-up of https://github.com/carbon-language/carbon-lang/pull/5891.
-
 // --- derived_to_base_conversion.h
 
 class Base {};
@@ -103,34 +101,20 @@ struct Derived : Base {
   void g() const;
 };
 
-// --- fail_todo_5891_use_base_method.carbon
+// --- use_base_method.carbon
 
 library "[[@TEST_NAME]]";
 
 import Cpp library "base_method.h";
 
 fn CallDirect(d: Cpp.Derived) {
   //@dump-sem-ir-begin
-  // CHECK:STDERR: fail_todo_5891_use_base_method.carbon:[[@LINE+5]]:3: error: missing object argument in method call [MissingObjectInMethodCall]
-  // CHECK:STDERR:   d.f();
-  // CHECK:STDERR:   ^~~~~
-  // CHECK:STDERR: fail_todo_5891_use_base_method.carbon: note: calling function declared here [InCallToFunction]
-  // CHECK:STDERR:
   d.f();
   //@dump-sem-ir-end
 }
 
 fn CallQualified(d: Cpp.Derived) {
   //@dump-sem-ir-begin
-  // CHECK:STDERR: fail_todo_5891_use_base_method.carbon:[[@LINE+9]]:3: error: member name of type `<type of Cpp.g>` in compound member access is not an instance member or an interface member [CompoundMemberAccessDoesNotUseBase]
-  // CHECK:STDERR:   d.(Cpp.Base.g)();
-  // CHECK:STDERR:   ^~~~~~~~~~~~~~
-  // CHECK:STDERR:
-  // CHECK:STDERR: fail_todo_5891_use_base_method.carbon:[[@LINE+5]]:3: error: missing object argument in method call [MissingObjectInMethodCall]
-  // CHECK:STDERR:   d.(Cpp.Base.g)();
-  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_todo_5891_use_base_method.carbon: note: calling function declared here [InCallToFunction]
-  // CHECK:STDERR:
   d.(Cpp.Base.g)();
   //@dump-sem-ir-end
 }
@@ -472,7 +456,7 @@ class V {
 // CHECK:STDOUT:   return %Int.as.Copy.impl.Op.call to %return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_5891_use_base_method.carbon
+// CHECK:STDOUT: --- use_base_method.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Derived: type = class_type @Derived [concrete]
@@ -517,23 +501,33 @@ class V {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   %d.ref: %Derived = name_ref d, %d
 // CHECK:STDOUT:   %f.ref: %.5b0 = name_ref f, imports.%.e54 [concrete = constants.%empty_struct.3f3]
-// CHECK:STDOUT:   %addr: %ptr.fb2 = addr_of <error> [concrete = <error>]
-// CHECK:STDOUT:   %.loc13_7.1: %ptr.a97 = as_compatible %addr [concrete = <error>]
-// CHECK:STDOUT:   %.loc13_7.2: %ptr.a97 = converted %addr, %.loc13_7.1 [concrete = <error>]
-// CHECK:STDOUT:   %f__carbon_thunk.call: init %empty_tuple.type = call imports.%f__carbon_thunk.decl(%.loc13_7.2)
+// CHECK:STDOUT:   %bound_method: <bound method> = bound_method %d.ref, %f.ref
+// CHECK:STDOUT:   %.loc8_3.1: ref %Base = class_element_access %d.ref, element0
+// CHECK:STDOUT:   %.loc8_3.2: ref %Base = converted %d.ref, %.loc8_3.1
+// CHECK:STDOUT:   %.loc8_3.3: %Base = bind_value %.loc8_3.2
+// CHECK:STDOUT:   %.loc8_3.4: ref %Base = value_as_ref %.loc8_3.3
+// CHECK:STDOUT:   %addr: %ptr.fb2 = addr_of %.loc8_3.4
+// CHECK:STDOUT:   %.loc8_7.1: %ptr.a97 = as_compatible %addr
+// CHECK:STDOUT:   %.loc8_7.2: %ptr.a97 = converted %addr, %.loc8_7.1
+// CHECK:STDOUT:   %f__carbon_thunk.call: init %empty_tuple.type = call imports.%f__carbon_thunk.decl(%.loc8_7.2)
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @CallQualified(%d.param: %Derived) {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   %d.ref: %Derived = name_ref d, %d
-// CHECK:STDOUT:   %Cpp.ref.loc28: <namespace> = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp]
+// CHECK:STDOUT:   %Cpp.ref.loc14: <namespace> = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp]
 // CHECK:STDOUT:   %Base.ref: type = name_ref Base, imports.%Base.decl [concrete = constants.%Base]
 // CHECK:STDOUT:   %g.ref: %.7c6 = name_ref g, imports.%.362 [concrete = constants.%empty_struct.49e]
-// CHECK:STDOUT:   %addr: %ptr.fb2 = addr_of <error> [concrete = <error>]
-// CHECK:STDOUT:   %.loc28_18.1: %ptr.a97 = as_compatible %addr [concrete = <error>]
-// CHECK:STDOUT:   %.loc28_18.2: %ptr.a97 = converted %addr, %.loc28_18.1 [concrete = <error>]
-// CHECK:STDOUT:   %g__carbon_thunk.call: init %empty_tuple.type = call imports.%g__carbon_thunk.decl(%.loc28_18.2)
+// CHECK:STDOUT:   %bound_method: <bound method> = bound_method %d.ref, %g.ref
+// CHECK:STDOUT:   %.loc14_3.1: ref %Base = class_element_access %d.ref, element0
+// CHECK:STDOUT:   %.loc14_3.2: ref %Base = converted %d.ref, %.loc14_3.1
+// CHECK:STDOUT:   %.loc14_3.3: %Base = bind_value %.loc14_3.2
+// CHECK:STDOUT:   %.loc14_3.4: ref %Base = value_as_ref %.loc14_3.3
+// CHECK:STDOUT:   %addr: %ptr.fb2 = addr_of %.loc14_3.4
+// CHECK:STDOUT:   %.loc14_18.1: %ptr.a97 = as_compatible %addr
+// CHECK:STDOUT:   %.loc14_18.2: %ptr.a97 = converted %addr, %.loc14_18.1
+// CHECK:STDOUT:   %g__carbon_thunk.call: init %empty_tuple.type = call imports.%g__carbon_thunk.decl(%.loc14_18.2)
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT: