When looking up C++ operators, make sure all operands are complete (#6132)

This diagnoses instead of crashing in some cases:
* When one of the operands is an incomplete Carbon type.
* When one of the operands is a C++ class that can't be completed due to
lack of Carbon supported.
The new tests cover these cases.

Part of #5995.

Author: bricknerb

toolchain/check/cpp/operators.cpp

@@ -10,6 +10,7 @@
 #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"
 
 namespace Carbon::Check {
@@ -169,19 +170,33 @@ auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
     return SemIR::InstId::None;
   }
 
+  // Make sure all operands are complete before lookup.
+  for (SemIR::InstId arg_id : arg_ids) {
+    SemIR::TypeId arg_type_id = context.insts().Get(arg_id).type_id();
+    if (!RequireCompleteType(context, arg_type_id, loc_id, [&] {
+          CARBON_DIAGNOSTIC(
+              IncompleteOperandTypeInCppOperatorLookup, Error,
+              "looking up a C++ operator with incomplete operand type {0}",
+              SemIR::TypeId);
+          return context.emitter().Build(
+              loc_id, IncompleteOperandTypeInCppOperatorLookup, arg_type_id);
+        })) {
+      return SemIR::ErrorInst::InstId;
+    }
+  }
+
   auto arg_exprs = InventClangArgs(context, arg_ids);
   if (!arg_exprs.has_value()) {
     return SemIR::ErrorInst::InstId;
   }
 
-  clang::Sema& sema = context.clang_sema();
-
   clang::UnresolvedSet<4> functions;
   // TODO: Add location accordingly.
   clang::OverloadCandidateSet candidate_set(
       clang::SourceLocation(), clang::OverloadCandidateSet::CSK_Operator);
   // This works for both unary and binary operators.
-  sema.LookupOverloadedBinOp(candidate_set, *op_kind, functions, *arg_exprs);
+  context.clang_sema().LookupOverloadedBinOp(candidate_set, *op_kind, functions,
+                                             *arg_exprs);
 
   for (auto& it : candidate_set) {
     if (!it.Function) {

toolchain/check/operator.cpp

@@ -48,9 +48,10 @@ static auto IsCppClassType(Context& context, SemIR::InstId inst_id) -> bool {
     return false;
   }
 
-  const SemIR::Class& class_info = context.classes().Get(class_type->class_id);
-  return class_info.is_complete() &&
-         context.name_scopes().Get(class_info.scope_id).is_cpp_scope();
+  SemIR::NameScopeId class_scope_id =
+      context.classes().Get(class_type->class_id).scope_id;
+  return class_scope_id.has_value() &&
+         context.name_scopes().Get(class_scope_id).is_cpp_scope();
 }
 
 auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
@@ -64,7 +65,10 @@ auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
   if (IsCppClassType(context, operand_id)) {
     SemIR::InstId cpp_inst_id =
         LookupCppOperator(context, loc_id, op, {operand_id});
-    if (cpp_inst_id.has_value() && cpp_inst_id != SemIR::ErrorInst::InstId) {
+    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});
     }
   }
@@ -98,7 +102,10 @@ auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
   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() && cpp_inst_id != SemIR::ErrorInst::InstId) {
+    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});
     }
   }

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

@@ -753,7 +753,7 @@ fn F() {
 }
 
 // ============================================================================
-// Incomplete operand
+// Incomplete operand C++ type
 // ============================================================================
 
 // --- incomplete.h
@@ -784,6 +784,72 @@ fn F() {
   let c3: Cpp.Complete = Cpp.foo(c1 + ({} as Cpp.Incomplete));
 }
 
+// ============================================================================
+// Incomplete operand Carbon type
+// ============================================================================
+
+// --- complete.h
+
+struct Complete {};
+
+// --- fail_incomplete_operand_carbon_type.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "complete.h";
+
+class Incomplete;
+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:   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;
+}
+
+// ============================================================================
+// Unsupported operand type in instantiation
+// ============================================================================
+
+// --- unsupported_in_instantiation.h
+
+struct Supported {};
+
+template<typename T>
+struct Unsupported : public virtual Supported {};
+using UnsupportedAlias = Unsupported<int>;
+extern UnsupportedAlias unsupported;
+
+// --- fail_import_unsupported_in_instantiation.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "unsupported_in_instantiation.h";
+
+fn F() {
+  var supported: Cpp.Supported = Cpp.Supported.Supported();
+  // CHECK:STDERR: fail_import_unsupported_in_instantiation.carbon:[[@LINE+10]]: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.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.carbon:[[@LINE+4]]:21: note: in `Cpp` operator `AddWith` lookup [InCppOperatorLookup]
+  // CHECK:STDERR:   let result: i32 = supported + Cpp.unsupported;
+  // CHECK:STDERR:                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  let result: i32 = supported + Cpp.unsupported;
+}
+
 // ============================================================================
 // Operator overloading
 // ============================================================================

toolchain/diagnostics/diagnostic_kind.def

@@ -369,6 +369,7 @@ CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope)
 CARBON_DIAGNOSTIC_KIND(InCppNameLookup)
 CARBON_DIAGNOSTIC_KIND(InCppOperatorLookup)
 CARBON_DIAGNOSTIC_KIND(InNameLookup)
+CARBON_DIAGNOSTIC_KIND(IncompleteOperandTypeInCppOperatorLookup)
 CARBON_DIAGNOSTIC_KIND(NameAmbiguousDueToExtend)
 CARBON_DIAGNOSTIC_KIND(NameNotFound)
 CARBON_DIAGNOSTIC_KIND(MemberNameNotFound)