diff --git a/toolchain/check/cpp/operators.cpp b/toolchain/check/cpp/operators.cpp index 74a3ee0b457e..5369b1bb74cb 100644 --- a/toolchain/check/cpp/operators.cpp +++ b/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) { diff --git a/toolchain/check/operator.cpp b/toolchain/check/operator.cpp index 97eedcf659e8..95330b77ee9d 100644 --- a/toolchain/check/operator.cpp +++ b/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}); } } diff --git a/toolchain/check/testdata/interop/cpp/function/operators.carbon b/toolchain/check/testdata/interop/cpp/function/operators.carbon index 1feb86cd4a7a..1a7b8d0979d1 100644 --- a/toolchain/check/testdata/interop/cpp/function/operators.carbon +++ b/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 +struct Unsupported : public virtual Supported {}; +using UnsupportedAlias = Unsupported; +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 // ============================================================================ diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index df05dcb981ca..9cc709bdb6c3 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/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)