Perform overload resolution immediately in C++ operator lookup. (#6416)

Don't attempt to defer overload resolution by creating a
`CppOverloadSet`; this was incorrect as we weren't saving the complete
clang::OverloadCandidateSet, resulting in template candidates not being
found. Moreover, saving the overload candidate set would be expensive,
as the representation is surprisingly large, and is unnecessary since
we're about to build a call.

In passing, improve the diagnostics for overload resolution failure to
use Clang's operator overload resolution messages rather than its call
overload resolution messages.

This fixes calls to templated operator overloads, which is the final
piece needed for us to successfully compile an iostream-based "Hello
world" program.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

examples/interop/cpp/hello_world.carbon

@@ -2,31 +2,54 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-import Cpp inline "#include <cstdio>";
-
-fn Run() {
-  // TODO: Requires class with virtual bases (`basic_ostream`).
-  // Cpp.std.cout << "Hello world!\n";
-
-  // TODO: Requires variadic function.
-  // Cpp.printf("Hello world!\n");
-
-  // TODO: Requires nullable pointer.
-  // Cpp.puts("Hello world!\n");
-
-  // TODO: Requires nullable void pointer.
-  // Cpp.write(1, "Hello world!\n", 13);
-
-  // TODO: Requires Core.String API.
-  // let message: str = "Hello world!\n\n";
-  // for (c: char in message) {
-  //   Cpp.putchar((c as u8) as i32);
-  // }
+import Cpp library "<cstdio>";
+import Cpp library "<iostream>";
+import Cpp library "<string_view>";
+import Cpp library "<unistd.h>";
 
+// Demonstrate passing `char`s to a C++ function.
+fn HelloPutchar() {
   let message: array(char, 13) =
       ('H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!', '\n');
   for (c: char in message) {
     // TODO: u8 should probably have an implicit cast to i32.
     Cpp.putchar((c as u8) as i32);
   }
+
+  // TODO: Use a loop over a string literal instead. Requires IndexWith support
+  // for str.
+  //
+  // let message: str = "Hello world!\n";
+  // for (c: char in message) {
+  //   Cpp.putchar((c as u8) as i32);
+  // }
+}
+
+// Demonstrate passing a null-terminated string to a C++ function.
+fn HelloStdio() {
+  // TODO: Requires mapping from Optional(const char*) into C++.
+  // TODO: There should be a better way to interact with functions that expect a
+  // null-terminated string.
+  // Cpp.puts(Cpp.std.data("Hello world!\n\0"));
+
+  // TODO: Requires variadic function support.
+  // Cpp.printf("Hello world!\n\0");
+}
+
+// Demonstrate passing a string as a void pointer to a C++ function.
+fn HelloWrite() {
+  // TODO: Requires mapping from Optional(const char*) into a C++ const void*.
+  // let s: str = "Hello world!\n";
+  // Cpp.write(1, Cpp.std.data(s), Cpp.std.size(s));
+}
+
+fn HelloIostreams() {
+  Cpp.std.cout << "Hello world!\n";
+}
+
+fn Run() {
+  HelloPutchar();
+  HelloStdio();
+  HelloWrite();
+  HelloIostreams();
 }

toolchain/check/cpp/call.cpp

@@ -14,16 +14,6 @@
 
 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,
@@ -41,9 +31,6 @@ auto PerformCallToCppFunction(Context& context, SemIR::LocId loc_id,
       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);
     }

toolchain/check/cpp/operators.cpp

@@ -8,11 +8,13 @@
 #include "clang/Sema/Sema.h"
 #include "toolchain/check/cpp/import.h"
 #include "toolchain/check/cpp/location.h"
+#include "toolchain/check/cpp/overload_resolution.h"
 #include "toolchain/check/cpp/type_mapping.h"
 #include "toolchain/check/inst.h"
 #include "toolchain/check/type.h"
 #include "toolchain/check/type_completion.h"
 #include "toolchain/sem_ir/ids.h"
+#include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Check {
 
@@ -190,31 +192,87 @@ auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
     }
   }
 
-  auto arg_exprs = InventClangArgs(context, arg_ids);
-  if (!arg_exprs.has_value()) {
+  auto maybe_arg_exprs = InventClangArgs(context, arg_ids);
+  if (!maybe_arg_exprs.has_value()) {
     return SemIR::ErrorInst::InstId;
   }
+  auto& arg_exprs = *maybe_arg_exprs;
 
   clang::SourceLocation loc = GetCppLocation(context, loc_id);
   clang::OverloadCandidateSet::OperatorRewriteInfo operator_rewrite_info(
       *op_kind, loc, /*AllowRewritten=*/true);
-  clang::UnresolvedSet<4> functions;
   clang::OverloadCandidateSet candidate_set(
       loc, clang::OverloadCandidateSet::CSK_Operator, operator_rewrite_info);
+
+  clang::Sema& sema = context.clang_sema();
+
   // This works for both unary and binary operators.
-  context.clang_sema().LookupOverloadedBinOp(candidate_set, *op_kind, functions,
-                                             *arg_exprs);
+  sema.LookupOverloadedBinOp(candidate_set, *op_kind, clang::UnresolvedSet<0>{},
+                             arg_exprs);
 
-  for (auto& it : candidate_set) {
-    if (!it.Function) {
-      continue;
+  clang::OverloadCandidateSet::iterator best_viable_fn;
+  switch (candidate_set.BestViableFunction(sema, loc, best_viable_fn)) {
+    case clang::OverloadingResult::OR_Success: {
+      if (!best_viable_fn->Function) {
+        // The best viable candidate was a builtin. Let the Carbon operator
+        // machinery handle that.
+        return SemIR::InstId::None;
+      }
+      if (best_viable_fn->RewriteKind) {
+        context.TODO(
+            loc_id,
+            llvm::formatv("Rewriting operator{0} using {1} is not supported",
+                          clang::getOperatorSpelling(
+                              candidate_set.getRewriteInfo().OriginalOperator),
+                          best_viable_fn->Function->getNameAsString()));
+        return SemIR::ErrorInst::InstId;
+      }
+      sema.MarkFunctionReferenced(loc, best_viable_fn->Function);
+      auto result_id = ImportCppFunctionDecl(
+          context, loc_id, best_viable_fn->Function,
+          // If this is an operator method, the first arg will be used as self.
+          arg_ids.size() -
+              (isa<clang::CXXMethodDecl>(best_viable_fn->Function) ? 1 : 0));
+      if (auto fn_decl =
+              context.insts().TryGetAsWithId<SemIR::FunctionDecl>(result_id)) {
+        CheckCppOverloadAccess(context, loc_id, best_viable_fn->FoundDecl,
+                               fn_decl->inst_id);
+      } else {
+        CARBON_CHECK(result_id == SemIR::ErrorInst::InstId);
+      }
+      return result_id;
     }
-    functions.addDecl(it.Function, it.FoundDecl.getAccess());
+    case clang::OverloadingResult::OR_No_Viable_Function: {
+      // OK, didn't find a viable C++ candidate, but this is not an error, as
+      // there might be a Carbon candidate.
+      return SemIR::InstId::None;
+    }
+    case clang::OverloadingResult::OR_Ambiguous: {
+      const char* spelling = clang::getOperatorSpelling(*op_kind);
+      candidate_set.NoteCandidates(
+          clang::PartialDiagnosticAt(
+              loc, sema.PDiag(clang::diag::err_ovl_ambiguous_oper_binary)
+                       << spelling << arg_exprs[0]->getType()
+                       << arg_exprs[1]->getType()),
+          sema, clang::OCD_AmbiguousCandidates, arg_exprs, spelling, loc);
+      return SemIR::ErrorInst::InstId;
+    }
+    case clang::OverloadingResult::OR_Deleted:
+      const char* spelling = clang::getOperatorSpelling(*op_kind);
+      auto* message = best_viable_fn->Function->getDeletedMessage();
+      // The best viable function might be a different operator if the best
+      // candidate is a rewritten candidate, so use the operator kind of the
+      // candidate itself in the diagnostic.
+      candidate_set.NoteCandidates(
+          clang::PartialDiagnosticAt(
+              loc, sema.PDiag(clang::diag::err_ovl_deleted_oper)
+                       << clang::getOperatorSpelling(
+                              best_viable_fn->Function->getOverloadedOperator())
+                       << (message != nullptr)
+                       << (message ? message->getString() : llvm::StringRef())),
+          sema, clang::OCD_AllCandidates, arg_exprs, spelling, loc);
+      return SemIR::ErrorInst::InstId;
   }
-
-  return ImportCppOverloadSet(
-      context, loc_id, SemIR::NameScopeId::None, SemIR::NameId::CppOperator,
-      /*naming_class=*/nullptr, std::move(functions), operator_rewrite_info);
 }
 
 auto IsCppOperatorMethodDecl(clang::Decl* decl) -> bool {
@@ -222,4 +280,17 @@ auto IsCppOperatorMethodDecl(clang::Decl* decl) -> bool {
   return clang_method_decl && clang_method_decl->isOverloadedOperator();
 }
 
+auto IsCppOperatorMethod(Context& context, SemIR::InstId inst_id) -> bool {
+  auto function_type = context.types().TryGetAs<SemIR::FunctionType>(
+      context.insts().Get(inst_id).type_id());
+  if (!function_type) {
+    return false;
+  }
+  SemIR::ClangDeclId clang_decl_id =
+      context.functions().Get(function_type->function_id).clang_decl_id;
+  return clang_decl_id.has_value() &&
+         IsCppOperatorMethodDecl(
+             context.clang_decls().Get(clang_decl_id).key.decl);
+}
+
 }  // namespace Carbon::Check

toolchain/check/cpp/operators.h

@@ -20,6 +20,11 @@ auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
 // Returns whether the decl is an operator member function.
 auto IsCppOperatorMethodDecl(clang::Decl* decl) -> bool;
 
+// Returns whether the specified instruction refers to a C++ overloaded operator
+// that is a method. If so, the first operand will be passed as `self` rather
+// than as the first argument.
+auto IsCppOperatorMethod(Context& context, SemIR::InstId inst_id) -> bool;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_CPP_OPERATORS_H_

toolchain/check/cpp/overload_resolution.cpp

@@ -84,22 +84,27 @@ static auto AddOverloadCandidates(clang::Sema& sema,
   }
 }
 
-// Checks whether a selected overload is accessible and diagnoses if not.
-static auto CheckOverloadAccess(Context& context, SemIR::LocId loc_id,
-                                const SemIR::CppOverloadSet& overload_set,
-                                clang::DeclAccessPair overload,
-                                SemIR::InstId overload_inst_id) -> void {
+auto CheckCppOverloadAccess(
+    Context& context, SemIR::LocId loc_id, clang::DeclAccessPair overload,
+    SemIR::KnownInstId<SemIR::FunctionDecl> overload_inst_id,
+    SemIR::NameScopeId parent_scope_id) -> void {
   SemIR::AccessKind member_access_kind = MapCppAccess(overload);
   if (member_access_kind == SemIR::AccessKind::Public) {
     return;
   }
 
+  auto function_id = context.insts().Get(overload_inst_id).function_id;
+  auto& function = context.functions().Get(function_id);
+  if (!parent_scope_id.has_value()) {
+    parent_scope_id = function.parent_scope_id;
+  }
+
   auto name_scope_const_id = context.constant_values().Get(
-      context.name_scopes().Get(overload_set.parent_scope_id).inst_id());
+      context.name_scopes().Get(parent_scope_id).inst_id());
   SemIR::AccessKind allowed_access_kind =
       GetHighestAllowedAccess(context, loc_id, name_scope_const_id);
-  CheckAccess(context, loc_id, SemIR::LocId(overload_inst_id),
-              overload_set.name_id, member_access_kind,
+  CheckAccess(context, loc_id, SemIR::LocId(overload_inst_id), function.name_id,
+              member_access_kind,
               /*is_parent_access=*/false,
               {.constant_id = name_scope_const_id,
                .highest_allowed_access = allowed_access_kind});
@@ -155,25 +160,18 @@ auto PerformCppOverloadResolution(Context& context, SemIR::LocId loc_id,
 
   switch (overloading_result) {
     case clang::OverloadingResult::OR_Success: {
-      // TODO: Handle the cases when Function is null.
       CARBON_CHECK(best_viable_fn->Function);
-      if (best_viable_fn->RewriteKind) {
-        context.TODO(
-            loc_id,
-            llvm::formatv("Rewriting operator{0} using {1} is not supported",
-                          clang::getOperatorSpelling(
-                              candidate_set.getRewriteInfo().OriginalOperator),
-                          best_viable_fn->Function->getNameAsString()));
-        return SemIR::ErrorInst::InstId;
-      }
+      CARBON_CHECK(!best_viable_fn->RewriteKind);
       sema.MarkFunctionReferenced(loc, best_viable_fn->Function);
       SemIR::InstId result_id = ImportCppFunctionDecl(
-          context, loc_id, best_viable_fn->Function,
-          // If this is an operator method, the first arg will be used as self.
-          arg_exprs.size() -
-              (IsCppOperatorMethodDecl(best_viable_fn->Function) ? 1 : 0));
-      CheckOverloadAccess(context, loc_id, overload_set,
-                          best_viable_fn->FoundDecl, result_id);
+          context, loc_id, best_viable_fn->Function, arg_exprs.size());
+      if (auto fn_decl =
+              context.insts().TryGetAsWithId<SemIR::FunctionDecl>(result_id)) {
+        CheckCppOverloadAccess(context, loc_id, best_viable_fn->FoundDecl,
+                               fn_decl->inst_id, overload_set.parent_scope_id);
+      } else {
+        CARBON_CHECK(result_id == SemIR::ErrorInst::InstId);
+      }
       return result_id;
     }
     case clang::OverloadingResult::OR_No_Viable_Function: {

toolchain/check/cpp/overload_resolution.h

@@ -11,6 +11,15 @@
 
 namespace Carbon::Check {
 
+// Checks whether a selected overload is accessible and diagnoses if not.
+// `parent_scope_id`, if specified, describes the scope that was named to find
+// the overload. If unspecified, we assume the overload was found in the class
+// that it is a direct member of, rather than a derived class.
+auto CheckCppOverloadAccess(
+    Context& context, SemIR::LocId loc_id, clang::DeclAccessPair overload,
+    SemIR::KnownInstId<SemIR::FunctionDecl> overload_inst_id,
+    SemIR::NameScopeId parent_scope_id = SemIR::NameScopeId::None) -> void;
+
 // Resolves which function to call using Clang overloading resolution, or
 // returns an error instruction if overload resolution failed.
 //

toolchain/check/operator.cpp

@@ -8,6 +8,7 @@
 
 #include "toolchain/check/call.h"
 #include "toolchain/check/context.h"
+#include "toolchain/check/cpp/call.h"
 #include "toolchain/check/cpp/operators.h"
 #include "toolchain/check/generic.h"
 #include "toolchain/check/member_access.h"
@@ -67,27 +68,30 @@ auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
   // Operator operands don't require `ref` tags.
   context.ref_tags().Insert(operand_id, Context::RefTag::NotRequired);
 
+  SemIR::InstId op_fn_id = SemIR::InstId::None;
+
   // For unary operators with a C++ class as the operand, try to import and call
   // the C++ operator.
   // TODO: Change impl lookup instead. See
   // https://github.com/carbon-language/carbon-lang/blob/db0a00d713015436844c55e7ac190a0f95556499/toolchain/check/operator.cpp#L76
   if (IsCppClassType(context, operand_id)) {
-    SemIR::InstId cpp_inst_id =
-        LookupCppOperator(context, loc_id, op, {operand_id});
-    if (cpp_inst_id.has_value()) {
-      if (cpp_inst_id == SemIR::ErrorInst::InstId) {
-        return SemIR::ErrorInst::InstId;
-      }
-      return PerformCall(context, loc_id, cpp_inst_id, {operand_id});
+    op_fn_id = LookupCppOperator(context, loc_id, op, {operand_id});
+
+    // If C++ operator lookup found a non-method operator, call it with one call
+    // argument. Otherwise fall through to call it with a self argument.
+    if (op_fn_id.has_value() && !IsCppOperatorMethod(context, op_fn_id)) {
+      return PerformCall(context, loc_id, op_fn_id, {operand_id});
     }
   }
 
-  // Look up the operator function.
-  auto op_fn = GetOperatorOpFunction(context, loc_id, op);
+  if (!op_fn_id.has_value()) {
+    // Look up the operator function.
+    op_fn_id = GetOperatorOpFunction(context, loc_id, op);
+  }
 
   // Form `operand.(Op)`.
-  auto bound_op_id = PerformCompoundMemberAccess(context, loc_id, operand_id,
-                                                 op_fn, missing_impl_diagnoser);
+  auto bound_op_id = PerformCompoundMemberAccess(
+      context, loc_id, operand_id, op_fn_id, missing_impl_diagnoser);
   if (bound_op_id == SemIR::ErrorInst::InstId) {
     return SemIR::ErrorInst::InstId;
   }
@@ -109,6 +113,8 @@ auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
   context.ref_tags().Insert(lhs_id, Context::RefTag::NotRequired);
   context.ref_tags().Insert(rhs_id, Context::RefTag::NotRequired);
 
+  SemIR::InstId op_fn_id = SemIR::InstId::None;
+
   // For binary operators with a C++ class as at least one of the operands, try
   // to import and call the C++ operator.
   // TODO: Instead of hooking this here, change impl lookup, so that a generic
@@ -118,22 +124,24 @@ auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
   // and
   // https://github.com/carbon-language/carbon-lang/pull/5996/files/5d01fa69511b76f87efbc0387f5e40abcf4c911a#r2308664536
   if (IsCppClassType(context, lhs_id) || IsCppClassType(context, rhs_id)) {
-    SemIR::InstId cpp_inst_id =
-        LookupCppOperator(context, loc_id, op, {lhs_id, rhs_id});
-    if (cpp_inst_id.has_value()) {
-      if (cpp_inst_id == SemIR::ErrorInst::InstId) {
-        return SemIR::ErrorInst::InstId;
-      }
-      return PerformCall(context, loc_id, cpp_inst_id, {lhs_id, rhs_id});
+    op_fn_id = LookupCppOperator(context, loc_id, op, {lhs_id, rhs_id});
+
+    // If C++ operator lookup found a non-method operator, call it with two call
+    // arguments. Otherwise fall through to call it with a self argument and one
+    // call argument.
+    if (op_fn_id.has_value() && !IsCppOperatorMethod(context, op_fn_id)) {
+      return PerformCall(context, loc_id, op_fn_id, {lhs_id, rhs_id});
     }
   }
 
-  // Look up the operator function.
-  auto op_fn = GetOperatorOpFunction(context, loc_id, op);
+  if (!op_fn_id.has_value()) {
+    // Look up the operator function.
+    op_fn_id = GetOperatorOpFunction(context, loc_id, op);
+  }
 
   // Form `lhs.(Op)`.
-  auto bound_op_id = PerformCompoundMemberAccess(context, loc_id, lhs_id, op_fn,
-                                                 missing_impl_diagnoser);
+  auto bound_op_id = PerformCompoundMemberAccess(
+      context, loc_id, lhs_id, op_fn_id, missing_impl_diagnoser);
   if (bound_op_id == SemIR::ErrorInst::InstId) {
     return SemIR::ErrorInst::InstId;
   }

toolchain/check/testdata/interop/cpp/function/operators.carbon

@@ -93,22 +93,14 @@ import Cpp library "postfix_inc_and_dec.h";
 
 fn F() {
   var postfix: Cpp.Postfix = Cpp.Postfix.Postfix();
-  // CHECK:STDERR: fail_postfix_calling_prefix.carbon:[[@LINE+8]]:3: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    16 |   ++postfix;
-  // CHECK:STDERR:       |   ^
-  // CHECK:STDERR: fail_postfix_calling_prefix.carbon:[[@LINE-7]]:10: in file included here [InCppInclude]
-  // CHECK:STDERR: ./postfix_inc_and_dec.h:3:6: note: candidate function not viable: requires 2 arguments, but 1 was provided [CppInteropParseNote]
-  // CHECK:STDERR:     3 | auto operator++(Postfix& operand, int) -> Postfix&;
-  // CHECK:STDERR:       |      ^          ~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR: fail_postfix_calling_prefix.carbon:[[@LINE+4]]:3: error: cannot access member of interface `Core.Inc` in type `Cpp.Postfix` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   ++postfix;
+  // CHECK:STDERR:   ^~~~~~~~~
   // CHECK:STDERR:
   ++postfix;
-  // CHECK:STDERR: fail_postfix_calling_prefix.carbon:[[@LINE+8]]:3: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    25 |   --postfix;
-  // CHECK:STDERR:       |   ^
-  // CHECK:STDERR: fail_postfix_calling_prefix.carbon:[[@LINE-16]]:10: in file included here [InCppInclude]
-  // CHECK:STDERR: ./postfix_inc_and_dec.h:4:6: note: candidate function not viable: requires 2 arguments, but 1 was provided [CppInteropParseNote]
-  // CHECK:STDERR:     4 | auto operator--(Postfix& operand, int) -> Postfix&;
-  // CHECK:STDERR:       |      ^          ~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR: fail_postfix_calling_prefix.carbon:[[@LINE+4]]:3: error: cannot access member of interface `Core.Dec` in type `Cpp.Postfix` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   --postfix;
+  // CHECK:STDERR:   ^~~~~~~~~
   // CHECK:STDERR:
   --postfix;
 }
@@ -230,26 +222,71 @@ import Cpp library "binary_operators.h";
 
 fn F() {
   let c1: Cpp.C = Cpp.C.C();
-  // CHECK:STDERR: fail_call_with_wrong_type.carbon:[[@LINE+8]]:22: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    16 |   let c2: Cpp.C = c1 + 5;
-  // CHECK:STDERR:       |                      ^
-  // CHECK:STDERR: fail_call_with_wrong_type.carbon:[[@LINE-7]]:10: in file included here [InCppInclude]
-  // CHECK:STDERR: ./binary_operators.h:5:6: note: candidate function not viable: no known conversion from 'int' to 'C' for 2nd argument [CppInteropParseNote]
-  // CHECK:STDERR:     5 | auto operator+(C lhs, C rhs) -> C;
-  // CHECK:STDERR:       |      ^                ~~~~~
+  // TODO: Include a list of non-viable candidate operators with this diagnostic.
+  // CHECK:STDERR: fail_call_with_wrong_type.carbon:[[@LINE+4]]:19: error: cannot access member of interface `Core.AddWith(Core.IntLiteral)` in type `Cpp.C` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   let c2: Cpp.C = c1 + 5;
+  // CHECK:STDERR:                   ^~~~~~
   // CHECK:STDERR:
   let c2: Cpp.C = c1 + 5;
-  // CHECK:STDERR: fail_call_with_wrong_type.carbon:[[@LINE+8]]:21: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    25 |   let c3: Cpp.C = 6 + c1;
-  // CHECK:STDERR:       |                     ^
-  // CHECK:STDERR: fail_call_with_wrong_type.carbon:[[@LINE-16]]:10: in file included here [InCppInclude]
-  // CHECK:STDERR: ./binary_operators.h:5:6: note: candidate function not viable: no known conversion from 'int' to 'C' for 1st argument [CppInteropParseNote]
-  // CHECK:STDERR:     5 | auto operator+(C lhs, C rhs) -> C;
-  // CHECK:STDERR:       |      ^         ~~~~~
+  // CHECK:STDERR: fail_call_with_wrong_type.carbon:[[@LINE+4]]:19: error: cannot access member of interface `Core.AddWith(Cpp.C)` in type `Core.IntLiteral` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   let c3: Cpp.C = 6 + c1;
+  // CHECK:STDERR:                   ^~~~~~
   // CHECK:STDERR:
   let c3: Cpp.C = 6 + c1;
 }
 
+// ============================================================================
+// Overload resolution failures
+// ============================================================================
+
+// --- fail_ambiguous.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+class C {};
+void operator+(C, char);
+void operator+(C, short);
+''';
+
+fn Test(c: Cpp.C) {
+  // CHECK:STDERR: fail_ambiguous.carbon:[[@LINE+10]]:5: error: use of overloaded operator '+' is ambiguous (with operand types 'C' and 'int') [CppInteropParseError]
+  // CHECK:STDERR:    21 |   c + 1;
+  // CHECK:STDERR:       |     ^
+  // CHECK:STDERR: fail_ambiguous.carbon:[[@LINE-8]]:6: note: candidate function [CppInteropParseNote]
+  // CHECK:STDERR:     6 | void operator+(C, char);
+  // CHECK:STDERR:       |      ^
+  // CHECK:STDERR: fail_ambiguous.carbon:[[@LINE-10]]:6: note: candidate function [CppInteropParseNote]
+  // CHECK:STDERR:     7 | void operator+(C, short);
+  // CHECK:STDERR:       |      ^
+  // CHECK:STDERR:
+  c + 1;
+}
+
+// --- fail_deleted.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+class C {};
+void operator+(C, int) = delete;
+void operator+(C, void*) = delete;
+''';
+
+fn Test(c: Cpp.C) {
+  // CHECK:STDERR: fail_deleted.carbon:[[@LINE+10]]:5: error: overload resolution selected deleted operator '+' [CppInteropParseError]
+  // CHECK:STDERR:    21 |   c + 1;
+  // CHECK:STDERR:       |     ^
+  // CHECK:STDERR: fail_deleted.carbon:[[@LINE-8]]:6: note: candidate function has been explicitly deleted [CppInteropParseNote]
+  // CHECK:STDERR:     6 | void operator+(C, int) = delete;
+  // CHECK:STDERR:       |      ^
+  // CHECK:STDERR: fail_deleted.carbon:[[@LINE-10]]:6: note: candidate function not viable: no known conversion from 'int' to 'void *' for 2nd argument [CppInteropParseNote]
+  // CHECK:STDERR:     7 | void operator+(C, void*) = delete;
+  // CHECK:STDERR:       |      ^            ~~~~~
+  // CHECK:STDERR:
+  c + 1;
+}
+
 // ============================================================================
 // Rewrite using the spaceship operator
 // ============================================================================
@@ -294,6 +331,34 @@ fn F() {
   //@dump-sem-ir-end
 }
 
+// --- fail_rewrite_deleted.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+class B {};
+class C : public B {};
+
+namespace std { class strong_ordering {}; }
+auto operator<=>(C lhs, C rhs) -> std::strong_ordering = delete;
+auto operator<(C lhs, B rhs) -> bool;
+''';
+
+fn F(c: Cpp.C) -> bool {
+  // Note that we mention `operator '<=>'` in the diagnostic.
+  // CHECK:STDERR: fail_rewrite_deleted.carbon:[[@LINE+10]]:12: error: overload resolution selected deleted operator '<=>' [CppInteropParseError]
+  // CHECK:STDERR:    25 |   return c < c;
+  // CHECK:STDERR:       |            ^
+  // CHECK:STDERR: fail_rewrite_deleted.carbon:[[@LINE-9]]:6: note: candidate function has been explicitly deleted [CppInteropParseNote]
+  // CHECK:STDERR:     9 | auto operator<=>(C lhs, C rhs) -> std::strong_ordering = delete;
+  // CHECK:STDERR:       |      ^
+  // CHECK:STDERR: fail_rewrite_deleted.carbon:[[@LINE-11]]:6: note: candidate function [CppInteropParseNote]
+  // CHECK:STDERR:    10 | auto operator<(C lhs, B rhs) -> bool;
+  // CHECK:STDERR:       |      ^
+  // CHECK:STDERR:
+  return c < c;
+}
+
 // ============================================================================
 // Rewrite using the equal operator
 // ============================================================================
@@ -655,9 +720,9 @@ fn F() {
   //@dump-sem-ir-begin
   let c1: Cpp.N.C = Cpp.N.C.C();
   let c2: Cpp.N.C = Cpp.N.C.C();
-  // CHECK:STDERR: fail_todo_import_operands_in_namespace_operator_in_global.carbon:[[@LINE+4]]:24: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    14 |   let c3: Cpp.N.C = c1 + c2;
-  // CHECK:STDERR:       |                        ^
+  // CHECK:STDERR: fail_todo_import_operands_in_namespace_operator_in_global.carbon:[[@LINE+4]]:21: error: cannot access member of interface `Core.AddWith(Cpp.N.C)` in type `Cpp.N.C` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   let c3: Cpp.N.C = c1 + c2;
+  // CHECK:STDERR:                     ^~~~~~~
   // CHECK:STDERR:
   let c3: Cpp.N.C = c1 + c2;
   //@dump-sem-ir-end
@@ -793,9 +858,9 @@ import Cpp library "not_found.h";
 fn F() {
   let c1: Cpp.C = Cpp.C.C();
   let c2: Cpp.C = Cpp.C.C();
-  // CHECK:STDERR: fail_import_not_found.carbon:[[@LINE+4]]:22: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    13 |   let c3: Cpp.C = c1 + c2;
-  // CHECK:STDERR:       |                      ^
+  // CHECK:STDERR: fail_import_not_found.carbon:[[@LINE+4]]:19: error: cannot access member of interface `Core.AddWith(Cpp.C)` in type `Cpp.C` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   let c3: Cpp.C = c1 + c2;
+  // CHECK:STDERR:                   ^~~~~~~
   // CHECK:STDERR:
   let c3: Cpp.C = c1 + c2;
 }
@@ -899,10 +964,6 @@ fn F(x: Cpp.X) -> i32 {
   //@dump-sem-ir-end
 }
 
-// ============================================================================
-// Indirect template instantiation error
-// ============================================================================
-
 // --- fail_indirect_template_instantiation_error.carbon
 
 library "[[@TEST_NAME]]";
@@ -924,13 +985,33 @@ fn F(x: Cpp.X) -> i32 {
   // CHECK:STDERR:    25 |   return x + x;
   // CHECK:STDERR:       |            ^
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_indirect_template_instantiation_error.carbon:[[@LINE+4]]:12: error: no matching function for call to '<C++ operator>' [CppInteropParseError]
-  // CHECK:STDERR:    25 |   return x + x;
-  // CHECK:STDERR:       |            ^
+  // CHECK:STDERR: fail_indirect_template_instantiation_error.carbon:[[@LINE+4]]:10: error: cannot access member of interface `Core.AddWith(Cpp.B)` in type `Cpp.B` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   return x + x;
+  // CHECK:STDERR:          ^~~~~
   // CHECK:STDERR:
   return x + x;
 }
 
+// ============================================================================
+// Templated operators
+// ============================================================================
+
+// --- templated_operator.h
+
+template<typename T> struct X {};
+template<typename T> X<T> operator+(X<T>, X<T>);
+using Xint = X<int>;
+
+// --- templated_operator.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "templated_operator.h";
+
+fn Test(x: Cpp.Xint) -> Cpp.Xint {
+  return x + x;
+}
+
 // ============================================================================
 // Operator overloading
 // ============================================================================
@@ -2913,8 +2994,12 @@ fn F() {
 // CHECK:STDOUT:   %ptr.d9e: type = ptr_type %C [concrete]
 // CHECK:STDOUT:   %C__carbon_thunk.type: type = fn_type @C__carbon_thunk [concrete]
 // CHECK:STDOUT:   %C__carbon_thunk: %C__carbon_thunk.type = struct_value () [concrete]
+// CHECK:STDOUT:   %C.cpp_operator.type.dab96a.1: type = fn_type @C.cpp_operator.1 [concrete]
+// CHECK:STDOUT:   %C.cpp_operator.d21c75.1: %C.cpp_operator.type.dab96a.1 = struct_value () [concrete]
 // CHECK:STDOUT:   %operator-__carbon_thunk.type: type = fn_type @operator-__carbon_thunk [concrete]
 // CHECK:STDOUT:   %operator-__carbon_thunk: %operator-__carbon_thunk.type = struct_value () [concrete]
+// CHECK:STDOUT:   %C.cpp_operator.type.dab96a.2: type = fn_type @C.cpp_operator.2 [concrete]
+// CHECK:STDOUT:   %C.cpp_operator.d21c75.2: %C.cpp_operator.type.dab96a.2 = struct_value () [concrete]
 // CHECK:STDOUT:   %operator+__carbon_thunk.type: type = fn_type @operator+__carbon_thunk [concrete]
 // CHECK:STDOUT:   %operator+__carbon_thunk: %operator+__carbon_thunk.type = struct_value () [concrete]
 // CHECK:STDOUT:   %type_where: type = facet_type <type where .Self impls <CanDestroy>> [concrete]
@@ -2935,11 +3020,29 @@ fn F() {
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     <elided>
 // CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C.cpp_operator.decl.828f43.1: %C.cpp_operator.type.dab96a.1 = fn_decl @C.cpp_operator.1 [concrete = constants.%C.cpp_operator.d21c75.1] {
+// CHECK:STDOUT:     %self.patt: %pattern_type.217 = ref_binding_pattern self [concrete]
+// CHECK:STDOUT:     %self.param_patt: %pattern_type.217 = ref_param_pattern %self.patt, call_param0 [concrete]
+// CHECK:STDOUT:     <elided>
+// CHECK:STDOUT:   } {
+// CHECK:STDOUT:     %self.param: ref %C = ref_param call_param0
+// CHECK:STDOUT:     %self: ref %C = ref_binding self, %self.param
+// CHECK:STDOUT:     <elided>
+// CHECK:STDOUT:   }
 // CHECK:STDOUT:   %operator-__carbon_thunk.decl: %operator-__carbon_thunk.type = fn_decl @operator-__carbon_thunk [concrete = constants.%operator-__carbon_thunk] {
 // CHECK:STDOUT:     <elided>
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     <elided>
 // CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C.cpp_operator.decl.828f43.2: %C.cpp_operator.type.dab96a.2 = fn_decl @C.cpp_operator.2 [concrete = constants.%C.cpp_operator.d21c75.2] {
+// CHECK:STDOUT:     %self.patt: %pattern_type.217 = ref_binding_pattern self [concrete]
+// CHECK:STDOUT:     %self.param_patt: %pattern_type.217 = ref_param_pattern %self.patt, call_param0 [concrete]
+// CHECK:STDOUT:     <elided>
+// CHECK:STDOUT:   } {
+// CHECK:STDOUT:     %self.param: ref %C = ref_param call_param0
+// CHECK:STDOUT:     %self: ref %C = ref_binding self, %self.param
+// CHECK:STDOUT:     <elided>
+// CHECK:STDOUT:   }
 // CHECK:STDOUT:   %operator+__carbon_thunk.decl: %operator+__carbon_thunk.type = fn_decl @operator+__carbon_thunk [concrete = constants.%operator+__carbon_thunk] {
 // CHECK:STDOUT:     <elided>
 // CHECK:STDOUT:   } {
@@ -2973,6 +3076,7 @@ fn F() {
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %c2.var: ref %C = var %c2.var_patt
 // CHECK:STDOUT:   %c1.ref.loc9: ref %C = name_ref c1, %c1
+// CHECK:STDOUT:   %C.cpp_operator.bound.loc9: <bound method> = bound_method %c1.ref.loc9, imports.%C.cpp_operator.decl.828f43.1
 // CHECK:STDOUT:   %.loc9_3: ref %C = splice_block %c2.var {}
 // CHECK:STDOUT:   %addr.loc9: %ptr.d9e = addr_of %.loc9_3
 // CHECK:STDOUT:   %operator-__carbon_thunk.call: init %empty_tuple.type = call imports.%operator-__carbon_thunk.decl(%c1.ref.loc9, %addr.loc9)
@@ -2990,6 +3094,7 @@ fn F() {
 // CHECK:STDOUT:   %c3.var: ref %C = var %c3.var_patt
 // CHECK:STDOUT:   %c1.ref.loc10: ref %C = name_ref c1, %c1
 // CHECK:STDOUT:   %c2.ref: ref %C = name_ref c2, %c2
+// CHECK:STDOUT:   %C.cpp_operator.bound.loc10: <bound method> = bound_method %c1.ref.loc10, imports.%C.cpp_operator.decl.828f43.2
 // CHECK:STDOUT:   %.loc10_3: ref %C = splice_block %c3.var {}
 // CHECK:STDOUT:   %.loc10_24.1: %C = acquire_value %c2.ref
 // CHECK:STDOUT:   %.loc10_24.2: ref %C = value_as_ref %.loc10_24.1