Set the location for the candidate set when looking up C++ operators (#6138)

This adds location information and prevents crashes in some cases of
template instantiation in operator lookup.

Removed `InCppOperatorLookup` note as it is no longer necessary.

Part of #5995.

toolchain/check/cpp/operators.cpp

@@ -7,6 +7,7 @@
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/Sema.h"
 #include "toolchain/check/cpp/import.h"
+#include "toolchain/check/cpp/location.h"
 #include "toolchain/check/cpp/type_mapping.h"
 #include "toolchain/check/inst.h"
 #include "toolchain/check/type.h"
@@ -157,12 +158,11 @@ static auto GetClangOperatorKind(Context& context, SemIR::LocId loc_id,
 
 auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
                        llvm::ArrayRef<SemIR::InstId> arg_ids) -> SemIR::InstId {
-  Diagnostics::AnnotationScope annotate_diagnostics(
-      &context.emitter(), [&](auto& builder) {
-        CARBON_DIAGNOSTIC(InCppOperatorLookup, Note,
-                          "in `Cpp` operator `{0}` lookup", std::string);
-        builder.Note(loc_id, InCppOperatorLookup, op.interface_name.str());
-      });
+  // Register an annotation scope to flush any Clang diagnostics when we return.
+  // This is important to ensure that Clang diagnostics are properly interleaved
+  // with Carbon diagnostics.
+  Diagnostics::AnnotationScope annotate_diagnostics(&context.emitter(),
+                                                    [](auto& /*builder*/) {});
 
   auto op_kind =
       GetClangOperatorKind(context, loc_id, op.interface_name, op.op_name);
@@ -191,9 +191,9 @@ auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
   }
 
   clang::UnresolvedSet<4> functions;
-  // TODO: Add location accordingly.
   clang::OverloadCandidateSet candidate_set(
-      clang::SourceLocation(), clang::OverloadCandidateSet::CSK_Operator);
+      GetCppLocation(context, loc_id),
+      clang::OverloadCandidateSet::CSK_Operator);
   // This works for both unary and binary operators.
   context.clang_sema().LookupOverloadedBinOp(candidate_set, *op_kind, functions,
                                              *arg_exprs);

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

@@ -462,10 +462,7 @@ fn F() {
   var c1: Cpp.C = Cpp.C.C();
 
   // Bitwise.
-  // CHECK:STDERR: fail_todo_import_unsupported_binary_operators.carbon:[[@LINE+11]]:3: error: semantics TODO: `Unsupported operator interface `LeftShiftAssignWith`` [SemanticsTodo]
-  // CHECK:STDERR:   c1 <<= 1;
-  // CHECK:STDERR:   ^~~~~~~~
-  // CHECK:STDERR: fail_todo_import_unsupported_binary_operators.carbon:[[@LINE+8]]:3: note: in `Cpp` operator `LeftShiftAssignWith` lookup [InCppOperatorLookup]
+  // CHECK:STDERR: fail_todo_import_unsupported_binary_operators.carbon:[[@LINE+8]]:3: error: semantics TODO: `Unsupported operator interface `LeftShiftAssignWith`` [SemanticsTodo]
   // CHECK:STDERR:   c1 <<= 1;
   // CHECK:STDERR:   ^~~~~~~~
   // CHECK:STDERR:
@@ -474,10 +471,7 @@ fn F() {
   // CHECK:STDERR:   ^~~~~~~~
   // CHECK:STDERR:
   c1 <<= 1;
-  // CHECK:STDERR: fail_todo_import_unsupported_binary_operators.carbon:[[@LINE+11]]:3: error: semantics TODO: `Unsupported operator interface `RightShiftAssignWith`` [SemanticsTodo]
-  // CHECK:STDERR:   c1 >>= 2;
-  // CHECK:STDERR:   ^~~~~~~~
-  // CHECK:STDERR: fail_todo_import_unsupported_binary_operators.carbon:[[@LINE+8]]:3: note: in `Cpp` operator `RightShiftAssignWith` lookup [InCppOperatorLookup]
+  // CHECK:STDERR: fail_todo_import_unsupported_binary_operators.carbon:[[@LINE+8]]:3: error: semantics TODO: `Unsupported operator interface `RightShiftAssignWith`` [SemanticsTodo]
   // CHECK:STDERR:   c1 >>= 2;
   // CHECK:STDERR:   ^~~~~~~~
   // CHECK:STDERR:
@@ -770,16 +764,13 @@ library "[[@TEST_NAME]]";
 import Cpp library "incomplete.h";
 
 fn F() {
-  // CHECK:STDERR: fail_import_incomplete_unary.carbon:[[@LINE+11]]:27: error: looking up a C++ operator with incomplete operand type `Cpp.Incomplete` [IncompleteOperandTypeInCppOperatorLookup]
+  // CHECK:STDERR: fail_import_incomplete_unary.carbon:[[@LINE+8]]:27: error: looking up a C++ operator with incomplete operand type `Cpp.Incomplete` [IncompleteOperandTypeInCppOperatorLookup]
   // CHECK:STDERR:   let result_unary: i32 = -*Cpp.CreateIncomplete();
   // CHECK:STDERR:                           ^~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR: fail_import_incomplete_unary.carbon:[[@LINE-6]]:10: in file included here [InCppInclude]
   // CHECK:STDERR: ./incomplete.h:2:7: note: class was forward declared here [ClassForwardDeclaredHere]
   // CHECK:STDERR: class Incomplete;
   // CHECK:STDERR:       ^
-  // CHECK:STDERR: fail_import_incomplete_unary.carbon:[[@LINE+4]]:27: note: in `Cpp` operator `Negate` lookup [InCppOperatorLookup]
-  // CHECK:STDERR:   let result_unary: i32 = -*Cpp.CreateIncomplete();
-  // CHECK:STDERR:                           ^~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   let result_unary: i32 = -*Cpp.CreateIncomplete();
 }
@@ -792,16 +783,13 @@ import Cpp library "incomplete.h";
 
 fn F() {
   var complete: Cpp.Complete = Cpp.Complete.Complete();
-  // CHECK:STDERR: fail_import_incomplete_binary.carbon:[[@LINE+11]]:28: error: looking up a C++ operator with incomplete operand type `Cpp.Incomplete` [IncompleteOperandTypeInCppOperatorLookup]
+  // CHECK:STDERR: fail_import_incomplete_binary.carbon:[[@LINE+8]]:28: error: looking up a C++ operator with incomplete operand type `Cpp.Incomplete` [IncompleteOperandTypeInCppOperatorLookup]
   // CHECK:STDERR:   let result_binary: i32 = complete + *Cpp.CreateIncomplete();
   // CHECK:STDERR:                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR: fail_import_incomplete_binary.carbon:[[@LINE-7]]:10: in file included here [InCppInclude]
   // CHECK:STDERR: ./incomplete.h:2:7: note: class was forward declared here [ClassForwardDeclaredHere]
   // CHECK:STDERR: class Incomplete;
   // CHECK:STDERR:       ^
-  // CHECK:STDERR: fail_import_incomplete_binary.carbon:[[@LINE+4]]:28: note: in `Cpp` operator `AddWith` lookup [InCppOperatorLookup]
-  // CHECK:STDERR:   let result_binary: i32 = complete + *Cpp.CreateIncomplete();
-  // CHECK:STDERR:                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   let result_binary: i32 = complete + *Cpp.CreateIncomplete();
 }
@@ -825,15 +813,12 @@ fn CreateIncomplete() -> Incomplete*;
 
 fn F() {
   var complete: Cpp.Complete = Cpp.Complete.Complete();
-  // CHECK:STDERR: fail_incomplete_operand_carbon_type.carbon:[[@LINE+10]]:21: error: looking up a C++ operator with incomplete operand type `Incomplete` [IncompleteOperandTypeInCppOperatorLookup]
+  // CHECK:STDERR: fail_incomplete_operand_carbon_type.carbon:[[@LINE+7]]:21: error: looking up a C++ operator with incomplete operand type `Incomplete` [IncompleteOperandTypeInCppOperatorLookup]
   // CHECK:STDERR:   let result: i32 = *CreateIncomplete() + complete;
   // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR: fail_incomplete_operand_carbon_type.carbon:[[@LINE-8]]:1: note: class was forward declared here [ClassForwardDeclaredHere]
   // CHECK:STDERR: class Incomplete;
   // CHECK:STDERR: ^~~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_incomplete_operand_carbon_type.carbon:[[@LINE+4]]:21: note: in `Cpp` operator `AddWith` lookup [InCppOperatorLookup]
-  // CHECK:STDERR:   let result: i32 = *CreateIncomplete() + complete;
-  // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   let result: i32 = *CreateIncomplete() + complete;
 }
@@ -858,13 +843,10 @@ library "[[@TEST_NAME]]";
 import Cpp library "unsupported_in_instantiation.h";
 
 fn F() {
-  // CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+10]]:21: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
-  // CHECK:STDERR:   let result: i32 = -Cpp.unsupported;
-  // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+7]]:21: note: while completing C++ type `Cpp.Unsupported` [InCppTypeCompletion]
+  // CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+7]]:21: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
   // CHECK:STDERR:   let result: i32 = -Cpp.unsupported;
   // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+4]]:21: note: in `Cpp` operator `Negate` lookup [InCppOperatorLookup]
+  // CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+4]]:21: note: while completing C++ type `Cpp.Unsupported` [InCppTypeCompletion]
   // CHECK:STDERR:   let result: i32 = -Cpp.unsupported;
   // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~
   // CHECK:STDERR:
@@ -879,19 +861,70 @@ import Cpp library "unsupported_in_instantiation.h";
 
 fn F() {
   var supported: Cpp.Supported = Cpp.Supported.Supported();
-  // CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+10]]:21: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
+  // CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+7]]:21: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
   // CHECK:STDERR:   let result: i32 = supported + Cpp.unsupported;
   // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+7]]:21: note: while completing C++ type `Cpp.Unsupported` [InCppTypeCompletion]
-  // CHECK:STDERR:   let result: i32 = supported + Cpp.unsupported;
-  // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+4]]:21: note: in `Cpp` operator `AddWith` lookup [InCppOperatorLookup]
+  // CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+4]]:21: note: while completing C++ type `Cpp.Unsupported` [InCppTypeCompletion]
   // CHECK:STDERR:   let result: i32 = supported + Cpp.unsupported;
   // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   let result: i32 = supported + Cpp.unsupported;
 }
 
+// ============================================================================
+// Indirect template instantiation
+// ============================================================================
+
+// --- indirect_template_instantiation.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+template<typename T> struct B {};
+template<typename T> struct A {};
+
+using X = B<A<int>>;
+int operator+(X, X);
+''';
+
+fn F(x: Cpp.X) -> i32 {
+  //@dump-sem-ir-begin
+  return x + x;
+  //@dump-sem-ir-end
+}
+
+// ============================================================================
+// Indirect template instantiation error
+// ============================================================================
+
+// --- fail_indirect_template_instantiation_error.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp inline '''
+template<typename T> struct B {};
+template<typename T> struct A {
+  // CHECK:STDERR: fail_indirect_template_instantiation_error.carbon:[[@LINE+3]]:19: error: type 'int' cannot be used prior to '::' because it has no members [CppInteropParseError]
+  // CHECK:STDERR:    10 |   friend typename T::error operator+(B<A>, B<A>);
+  // CHECK:STDERR:       |                   ^
+  friend typename T::error operator+(B<A>, B<A>);
+};
+
+using X = B<A<int>>;
+''';
+
+fn F(x: Cpp.X) -> i32 {
+  // CHECK:STDERR: fail_indirect_template_instantiation_error.carbon:[[@LINE+8]]:12: note: in instantiation of template class 'A<int>' requested here [CppInteropParseNote]
+  // 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:
+  return x + x;
+}
+
 // ============================================================================
 // Operator overloading
 // ============================================================================
@@ -912,7 +945,9 @@ import Cpp library "overloading.h";
 fn F() {
   let c1: Cpp.C = Cpp.C.C();
   let c2: Cpp.C = Cpp.C.C();
+  //@dump-sem-ir-begin
   let c3: Cpp.C = c1 + c2;
+  //@dump-sem-ir-end
 }
 
 // CHECK:STDOUT: --- import_unary_operators.carbon
@@ -2883,3 +2918,95 @@ fn F() {
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: --- indirect_template_instantiation.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %B: type = class_type @B [concrete]
+// CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [concrete]
+// CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [concrete]
+// CHECK:STDOUT:   %ptr.a04: type = ptr_type %B [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: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// 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: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%x.param: %B) -> %i32 {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %x.ref.loc14_10: %B = name_ref x, %x
+// CHECK:STDOUT:   %x.ref.loc14_14: %B = name_ref x, %x
+// CHECK:STDOUT:   %.loc14_10: ref %B = value_as_ref %x.ref.loc14_10
+// CHECK:STDOUT:   %addr.loc14_12.1: %ptr.a04 = addr_of %.loc14_10
+// CHECK:STDOUT:   %.loc14_14: ref %B = value_as_ref %x.ref.loc14_14
+// CHECK:STDOUT:   %addr.loc14_12.2: %ptr.a04 = addr_of %.loc14_14
+// CHECK:STDOUT:   %operator+__carbon_thunk.call: init %i32 = call imports.%operator+__carbon_thunk.decl(%addr.loc14_12.1, %addr.loc14_12.2)
+// CHECK:STDOUT:   return %operator+__carbon_thunk.call to %return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- import_overloading.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
+// CHECK:STDOUT:   %C: type = class_type @C [concrete]
+// CHECK:STDOUT:   %pattern_type.217: type = pattern_type %C [concrete]
+// CHECK:STDOUT:   %ptr.d9e: type = ptr_type %C [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 <CanAggregateDestroy>> [concrete]
+// CHECK:STDOUT:   %facet_value: %type_where = facet_value %C, () [concrete]
+// CHECK:STDOUT:   %AggregateT.as_type.as.Destroy.impl.Op.type.fc1: type = fn_type @AggregateT.as_type.as.Destroy.impl.Op, @AggregateT.as_type.as.Destroy.impl(%facet_value) [concrete]
+// CHECK:STDOUT:   %AggregateT.as_type.as.Destroy.impl.Op.6b9: %AggregateT.as_type.as.Destroy.impl.Op.type.fc1 = struct_value () [concrete]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Cpp: <namespace> = namespace file.%Cpp.import_cpp, [concrete] {
+// CHECK:STDOUT:     .C = %C.decl
+// CHECK:STDOUT:     import Cpp//...
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C.decl: type = class_decl @C [concrete = constants.%C] {} {}
+// 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: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT:   name_binding_decl {
+// CHECK:STDOUT:     %c3.patt: %pattern_type.217 = binding_pattern c3 [concrete]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %c1.ref: %C = name_ref c1, %c1
+// CHECK:STDOUT:   %c2.ref: %C = name_ref c2, %c2
+// CHECK:STDOUT:   %.loc10_22.1: ref %C = temporary_storage
+// CHECK:STDOUT:   %.loc10_19: ref %C = value_as_ref %c1.ref
+// CHECK:STDOUT:   %addr.loc10_22.1: %ptr.d9e = addr_of %.loc10_19
+// CHECK:STDOUT:   %.loc10_24: ref %C = value_as_ref %c2.ref
+// CHECK:STDOUT:   %addr.loc10_22.2: %ptr.d9e = addr_of %.loc10_24
+// CHECK:STDOUT:   %addr.loc10_22.3: %ptr.d9e = addr_of %.loc10_22.1
+// CHECK:STDOUT:   %operator+__carbon_thunk.call: init %empty_tuple.type = call imports.%operator+__carbon_thunk.decl(%addr.loc10_22.1, %addr.loc10_22.2, %addr.loc10_22.3)
+// CHECK:STDOUT:   %.loc10_22.2: init %C = in_place_init %operator+__carbon_thunk.call, %.loc10_22.1
+// CHECK:STDOUT:   %.loc10_14: type = splice_block %C.ref.loc10 [concrete = constants.%C] {
+// CHECK:STDOUT:     %Cpp.ref.loc10: <namespace> = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp]
+// CHECK:STDOUT:     %C.ref.loc10: type = name_ref C, imports.%C.decl [concrete = constants.%C]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %.loc10_22.3: ref %C = temporary %.loc10_22.1, %.loc10_22.2
+// CHECK:STDOUT:   %.loc10_22.4: %C = bind_value %.loc10_22.3
+// CHECK:STDOUT:   %c3: %C = bind_name c3, %.loc10_22.4
+// CHECK:STDOUT:   %facet_value.loc10: %type_where = facet_value constants.%C, () [concrete = constants.%facet_value]
+// CHECK:STDOUT:   %.loc10_22.5: %type_where = converted constants.%C, %facet_value.loc10 [concrete = constants.%facet_value]
+// CHECK:STDOUT:   %AggregateT.as_type.as.Destroy.impl.Op.bound.loc10: <bound method> = bound_method %.loc10_22.3, constants.%AggregateT.as_type.as.Destroy.impl.Op.6b9
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT:   %bound_method.loc10: <bound method> = bound_method %.loc10_22.3, %AggregateT.as_type.as.Destroy.impl.Op.specific_fn.1
+// CHECK:STDOUT:   %addr.loc10_22.4: %ptr.d9e = addr_of %.loc10_22.3
+// CHECK:STDOUT:   %AggregateT.as_type.as.Destroy.impl.Op.call.loc10: init %empty_tuple.type = call %bound_method.loc10(%addr.loc10_22.4)
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/diagnostics/diagnostic_kind.def

@@ -360,7 +360,6 @@ CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope)
 
 // Name lookup.
 CARBON_DIAGNOSTIC_KIND(InCppNameLookup)
-CARBON_DIAGNOSTIC_KIND(InCppOperatorLookup)
 CARBON_DIAGNOSTIC_KIND(InNameLookup)
 CARBON_DIAGNOSTIC_KIND(IncompleteOperandTypeInCppOperatorLookup)
 CARBON_DIAGNOSTIC_KIND(NameAmbiguousDueToExtend)