diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 7df808ab1441..4719f124ba84 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -1450,6 +1450,178 @@ static auto CheckRefTag(Context& context, SemIR::InstId expr_id, } } +// State machine for performing category conversions. +class CategoryConverter { + public: + // Constructs a converter which converts an expression at the given location + // to the given conversion target. performed_builtin_conversion indicates + // whether builtin conversions were performed prior to this. + CategoryConverter(Context& context, SemIR::LocId loc_id, + ConversionTarget& target, bool performed_builtin_conversion) + : context_(context), + sem_ir_(context.sem_ir()), + loc_id_(loc_id), + target_(target), + performed_builtin_conversion_(performed_builtin_conversion) {} + + // Converts expr_id to the target specified in the constructor, and returns + // the converted inst. + auto Convert(SemIR::InstId expr_id) && -> SemIR::InstId { + auto category = SemIR::GetExprCategory(sem_ir_, expr_id); + while (true) { + if (expr_id == SemIR::ErrorInst::InstId) { + return expr_id; + } + CARBON_KIND_SWITCH(DoStep(expr_id, category)) { + case CARBON_KIND(NextStep next_step): { + CARBON_CHECK(next_step.expr_id != SemIR::InstId::None); + expr_id = next_step.expr_id; + category = next_step.category; + break; + } + case CARBON_KIND(Done done): { + return done.expr_id; + } + } + } + } + + private: + // State that indicates there's more work to be done. As a convenience, + // if expr_id is SemIR::ErrorInst::InstId, this is equivalent to + // Done{SemIR::ErrorInst::InstId}. + struct NextStep { + // The inst to convert. + SemIR::InstId expr_id; + // The category of expr_id. + SemIR::ExprCategory category; + }; + + // State that indicates we've finished category conversion. + struct Done { + // The result of the conversion. + SemIR::InstId expr_id; + }; + + using State = std::variant; + + // Performs the first step of converting `expr_id` with category `category` + // to the target specified in the constructor, and returns the state after + // that step. + auto DoStep(SemIR::InstId expr_id, SemIR::ExprCategory category) const + -> State; + + Context& context_; + SemIR::File& sem_ir_; + SemIR::LocId loc_id_; + ConversionTarget& target_; + bool performed_builtin_conversion_; +}; + +auto CategoryConverter::DoStep(const SemIR::InstId expr_id, + const SemIR::ExprCategory category) const + -> State { + CARBON_DCHECK(SemIR::GetExprCategory(sem_ir_, expr_id) == category); + switch (category) { + case SemIR::ExprCategory::NotExpr: + case SemIR::ExprCategory::Mixed: + case SemIR::ExprCategory::Pattern: + CARBON_FATAL("Unexpected expression {0} after builtin conversions", + sem_ir_.insts().Get(expr_id)); + + case SemIR::ExprCategory::Error: + return Done{SemIR::ErrorInst::InstId}; + + case SemIR::ExprCategory::Initializing: + if (target_.is_initializer()) { + if (!performed_builtin_conversion_) { + // Don't fill in the return slot if we created the expression through + // a builtin conversion. In that case, we will have created it with + // the target already set. + // TODO: Find a better way to track whether we need to do this, + // and then make target_ immutable if possible. + MarkInitializerFor(sem_ir_, expr_id, target_); + } + return Done{expr_id}; + } + + // Commit to using a temporary for this initializing expression. + // TODO: Don't create a temporary if the initializing representation + // is already a value representation. + // TODO: If the target is DurableRef, materialize a VarStorage instead of + // a TemporaryStorage to lifetime-extend. + if (target_.kind == ConversionTarget::Discarded) { + return Done{FinalizeTemporary(context_, expr_id, /*discarded=*/true)}; + } else { + return NextStep{.expr_id = FinalizeTemporary(context_, expr_id, + /*discarded=*/false), + .category = SemIR::ExprCategory::EphemeralRef}; + } + + case SemIR::ExprCategory::DurableRef: + if (target_.kind == ConversionTarget::DurableRef) { + return Done{expr_id}; + } + [[fallthrough]]; + + case SemIR::ExprCategory::EphemeralRef: + // If a reference expression is an acceptable result, we're done. + if (target_.kind == ConversionTarget::ValueOrRef || + target_.kind == ConversionTarget::Discarded || + target_.kind == ConversionTarget::CppThunkRef || + target_.kind == ConversionTarget::RefParam) { + return Done{expr_id}; + } + + // If we have a reference and don't want one, form a value binding. + // TODO: Support types with custom value representations. + return NextStep{.expr_id = AddInst( + context_, SemIR::LocId(expr_id), + {.type_id = target_.type_id, .value_id = expr_id}), + .category = SemIR::ExprCategory::Value}; + + case SemIR::ExprCategory::Value: + if (target_.kind == ConversionTarget::DurableRef) { + if (target_.diagnose) { + CARBON_DIAGNOSTIC(ConversionFailureNonRefToRef, Error, + "cannot bind durable reference to non-reference " + "value of type {0}", + SemIR::TypeId); + context_.emitter().Emit(loc_id_, ConversionFailureNonRefToRef, + target_.type_id); + } + return Done{SemIR::ErrorInst::InstId}; + } + if (target_.kind == ConversionTarget::RefParam) { + // Don't diagnose a non-reference scrutinee if it has a user-written + // `ref` tag, because that's diagnosed in `CheckRefTag`. + if (target_.diagnose) { + if (auto lookup_result = context_.ref_tags().Lookup(expr_id); + !lookup_result || + lookup_result.value() != Context::RefTag::Present) { + CARBON_DIAGNOSTIC(ValueForRefParam, Error, + "value expression passed to reference parameter"); + context_.emitter().Emit(loc_id_, ValueForRefParam); + } + } + return Done{SemIR::ErrorInst::InstId}; + } + + // When initializing from a value, perform a copy. + if (target_.is_initializer()) { + return Done{PerformCopy(context_, expr_id, target_)}; + } + + // When initializing a C++ thunk parameter, form a reference, creating a + // temporary if needed. + if (target_.kind == ConversionTarget::CppThunkRef) { + return Done{ConvertValueForCppThunkRef(context_, expr_id)}; + } + + return Done{expr_id}; + } +} + auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id, ConversionTarget target, SemIR::ClassType* vtable_class_type) -> SemIR::InstId { @@ -1621,108 +1793,9 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id, } // Now perform any necessary value category conversions. - // This uses fallthrough to implement a very simple state machine over the - // category of expr_id, which is tracked by current_category. - switch (auto current_category = SemIR::GetExprCategory(sem_ir, expr_id); - current_category) { - case SemIR::ExprCategory::NotExpr: - case SemIR::ExprCategory::Mixed: - case SemIR::ExprCategory::Pattern: - CARBON_FATAL("Unexpected expression {0} after builtin conversions", - sem_ir.insts().Get(expr_id)); - - case SemIR::ExprCategory::Error: - return SemIR::ErrorInst::InstId; - - case SemIR::ExprCategory::Initializing: - if (target.is_initializer()) { - if (!performed_builtin_conversion) { - // Don't fill in the return slot if we created the expression through - // a builtin conversion. In that case, we will have created it with - // the target already set. - // TODO: Find a better way to track whether we need to do this. - MarkInitializerFor(sem_ir, expr_id, target); - } - break; - } - - // Commit to using a temporary for this initializing expression. - // TODO: Don't create a temporary if the initializing representation - // is already a value representation. - // TODO: If the target is DurableRef, materialize a VarStorage instead of - // a TemporaryStorage to lifetime-extend. - expr_id = FinalizeTemporary(context, expr_id, - target.kind == ConversionTarget::Discarded); - // We now have an ephemeral reference. - current_category = SemIR::ExprCategory::EphemeralRef; - [[fallthrough]]; - - case SemIR::ExprCategory::DurableRef: - case SemIR::ExprCategory::EphemeralRef: - if (current_category == SemIR::ExprCategory::DurableRef && - target.kind == ConversionTarget::DurableRef) { - break; - } - - // If a reference expression is an acceptable result, we're done. - if (target.kind == ConversionTarget::ValueOrRef || - target.kind == ConversionTarget::Discarded || - target.kind == ConversionTarget::CppThunkRef || - target.kind == ConversionTarget::RefParam) { - break; - } - - // If we have a reference and don't want one, form a value binding. - // TODO: Support types with custom value representations. - expr_id = AddInst( - context, SemIR::LocId(expr_id), - {.type_id = target.type_id, .value_id = expr_id}); - // We now have a value expression. - current_category = SemIR::ExprCategory::Value; - [[fallthrough]]; - - case SemIR::ExprCategory::Value: - if (target.kind == ConversionTarget::DurableRef) { - if (target.diagnose) { - CARBON_DIAGNOSTIC(ConversionFailureNonRefToRef, Error, - "cannot bind durable reference to non-reference " - "value of type {0}", - SemIR::TypeId); - context.emitter().Emit(loc_id, ConversionFailureNonRefToRef, - target.type_id); - } - return SemIR::ErrorInst::InstId; - } - if (target.kind == ConversionTarget::RefParam) { - // Don't diagnose a non-reference scrutinee if it has a user-written - // `ref` tag, because that's diagnosed in `CheckRefTag`. - if (target.diagnose) { - if (auto lookup_result = context.ref_tags().Lookup(expr_id); - !lookup_result || - lookup_result.value() != Context::RefTag::Present) { - CARBON_DIAGNOSTIC(ValueForRefParam, Error, - "value expression passed to reference parameter"); - context.emitter().Emit(loc_id, ValueForRefParam); - } - } - return SemIR::ErrorInst::InstId; - } - - // When initializing from a value, perform a copy. - if (target.is_initializer()) { - expr_id = PerformCopy(context, expr_id, target); - current_category = SemIR::ExprCategory::Initializing; - } - - // When initializing a C++ thunk parameter, form a reference, creating a - // temporary if needed. - if (target.kind == ConversionTarget::CppThunkRef) { - expr_id = ConvertValueForCppThunkRef(context, expr_id); - current_category = SemIR::ExprCategory::EphemeralRef; - } - - break; - } + expr_id = + CategoryConverter(context, loc_id, target, performed_builtin_conversion) + .Convert(expr_id); // Perform a final destination store, if necessary. if (target.kind == ConversionTarget::FullInitializer) { diff --git a/toolchain/check/testdata/tuple/class_tuples.carbon b/toolchain/check/testdata/tuple/class_tuples.carbon index 20526aaf457b..79ef250d7a2d 100644 --- a/toolchain/check/testdata/tuple/class_tuples.carbon +++ b/toolchain/check/testdata/tuple/class_tuples.carbon @@ -21,10 +21,17 @@ class D {} var c: C; var d: D; -// CHECK:STDERR: fail_element_type_mismatch.carbon:[[@LINE+7]]:19: error: cannot implicitly convert expression of type `C` to `C*` [ConversionFailure] +// CHECK:STDERR: fail_element_type_mismatch.carbon:[[@LINE+14]]:19: error: cannot implicitly convert expression of type `C` to `C*` [ConversionFailure] // CHECK:STDERR: var x: (C*, C*) = (c, d); // CHECK:STDERR: ^~~~~~ -// CHECK:STDERR: fail_element_type_mismatch.carbon:[[@LINE+4]]:19: note: type `C` does not implement interface `Core.ImplicitAs(C*)` [MissingImplInMemberAccessNote] +// CHECK:STDERR: fail_element_type_mismatch.carbon:[[@LINE+11]]:19: note: type `C` does not implement interface `Core.ImplicitAs(C*)` [MissingImplInMemberAccessNote] +// CHECK:STDERR: var x: (C*, C*) = (c, d); +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_element_type_mismatch.carbon:[[@LINE+7]]:19: error: cannot implicitly convert expression of type `D` to `C*` [ConversionFailure] +// CHECK:STDERR: var x: (C*, C*) = (c, d); +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fail_element_type_mismatch.carbon:[[@LINE+4]]:19: note: type `D` does not implement interface `Core.ImplicitAs(C*)` [MissingImplInMemberAccessNote] // CHECK:STDERR: var x: (C*, C*) = (c, d); // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: