From ad0a4ea8a4cf6ae8a7d622316a34fe1d70fc83da Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Fri, 17 Apr 2026 12:03:21 -0700 Subject: [PATCH] Restructure action-dependence APIs (#7074) - Rename `ActionIsDependent` to `ActionIsPerformable` (with negated meaning), because that name is more concrete and, um, actionable. - Replace `OperandIsDependent` with `OperandDependence`, which returns a `ConstantDependence` instead of a bool. We need this additional generality for handling form actions, where we sometimes need to ask whether something has _any_ dependence, not just whether it has template dependence. --- toolchain/check/action.cpp | 62 ++++++++++++++++++++----------------- toolchain/check/action.h | 32 +++++++++---------- toolchain/check/convert.cpp | 21 +++++++------ toolchain/check/eval.cpp | 3 +- 4 files changed, 62 insertions(+), 56 deletions(-) diff --git a/toolchain/check/action.cpp b/toolchain/check/action.cpp index 7ea74e2b88d1..064f834d54f6 100644 --- a/toolchain/check/action.cpp +++ b/toolchain/check/action.cpp @@ -24,56 +24,59 @@ auto PerformAction(Context& context, SemIR::LocId loc_id, .source_id = action.inst_id}); } -static auto OperandIsDependent(Context& context, SemIR::ConstantId const_id) - -> bool { +static auto OperandDependence(Context& context, SemIR::ConstantId const_id) + -> SemIR::ConstantDependence { // A type operand makes the instruction dependent if it is a // template-dependent constant. if (!const_id.is_symbolic()) { - return false; + return SemIR::ConstantDependence::None; } - return context.constant_values().GetSymbolicConstant(const_id).dependence == - SemIR::ConstantDependence::Template; + return context.constant_values().GetSymbolicConstant(const_id).dependence; } -auto OperandIsDependent(Context& context, SemIR::TypeId type_id) -> bool { +auto OperandDependence(Context& context, SemIR::TypeId type_id) + -> SemIR::ConstantDependence { // A type operand makes the instruction dependent if it is a // template-dependent type. - return OperandIsDependent(context, context.types().GetConstantId(type_id)); + return OperandDependence(context, context.types().GetConstantId(type_id)); } -auto OperandIsDependent(Context& context, SemIR::InstId inst_id) -> bool { +auto OperandDependence(Context& context, SemIR::InstId inst_id) + -> SemIR::ConstantDependence { // An instruction operand makes the instruction dependent if its type or // constant value is dependent. - return OperandIsDependent(context, context.insts().Get(inst_id).type_id()) || - OperandIsDependent(context, context.constant_values().Get(inst_id)); + return std::max( + OperandDependence(context, context.insts().Get(inst_id).type_id()), + OperandDependence(context, context.constant_values().Get(inst_id))); } -auto OperandIsDependent(Context& context, SemIR::TypeInstId inst_id) -> bool { +auto OperandDependence(Context& context, SemIR::TypeInstId inst_id) + -> SemIR::ConstantDependence { // An instruction operand makes the instruction dependent if its type or // constant value is dependent. TypeInstId has type `TypeType` which is // concrete, so we only need to look at the constant value. - return OperandIsDependent(context, context.constant_values().Get(inst_id)); + return OperandDependence(context, context.constant_values().Get(inst_id)); } -static auto OperandIsDependent(Context& context, SemIR::Inst::ArgAndKind arg) - -> bool { +static auto OperandDependence(Context& context, SemIR::Inst::ArgAndKind arg) + -> SemIR::ConstantDependence { CARBON_KIND_SWITCH(arg) { case CARBON_KIND(SemIR::InstId inst_id): { - return OperandIsDependent(context, inst_id); + return OperandDependence(context, inst_id); } case CARBON_KIND(SemIR::MetaInstId inst_id): { - return OperandIsDependent(context, inst_id); + return OperandDependence(context, inst_id); } case CARBON_KIND(SemIR::TypeInstId inst_id): { - return OperandIsDependent(context, inst_id); + return OperandDependence(context, inst_id); } case SemIR::IdKind::None: case SemIR::IdKind::For: case SemIR::IdKind::For: - return false; + return SemIR::ConstantDependence::None; default: // TODO: Properly handle different argument kinds. @@ -81,18 +84,20 @@ static auto OperandIsDependent(Context& context, SemIR::Inst::ArgAndKind arg) } } -auto ActionIsDependent(Context& context, SemIR::Inst action_inst) -> bool { +auto ActionIsPerformable(Context& context, SemIR::Inst action_inst) -> bool { if (auto refine_action = action_inst.TryAs()) { - // `RefineTypeAction` can be performed whenever the type is non-dependent, - // even if we don't know the instruction yet. - return OperandIsDependent(context, refine_action->inst_type_inst_id); + // `RefineTypeAction` can be performed whenever the type is not template- + // dependent, even if we don't know the instruction yet. + return OperandDependence(context, refine_action->inst_type_inst_id) < + SemIR::ConstantDependence::Template; } - if (OperandIsDependent(context, action_inst.type_id())) { - return true; - } - return OperandIsDependent(context, action_inst.arg0_and_kind()) || - OperandIsDependent(context, action_inst.arg1_and_kind()); + return OperandDependence(context, action_inst.type_id()) < + SemIR::ConstantDependence::Template && + OperandDependence(context, action_inst.arg0_and_kind()) < + SemIR::ConstantDependence::Template && + OperandDependence(context, action_inst.arg1_and_kind()) < + SemIR::ConstantDependence::Template; } static auto AddDependentActionSpliceImpl(Context& context, @@ -129,7 +134,8 @@ static auto RefineOperand(Context& context, SemIR::LocId loc_id, // If the type of the action argument is dependent, refine to an instruction // with a concrete type. - if (OperandIsDependent(context, inst.type_id())) { + if (OperandDependence(context, inst.type_id()) == + SemIR::ConstantDependence::Template) { auto type_inst_id = context.types().GetTypeInstId(inst.type_id()); inst_id = AddDependentActionSpliceImpl( context, diff --git a/toolchain/check/action.h b/toolchain/check/action.h index 959670c3100f..a9635f1c32f4 100644 --- a/toolchain/check/action.h +++ b/toolchain/check/action.h @@ -28,21 +28,21 @@ auto PerformAction(Context& context, SemIR::LocId loc_id, auto PerformAction(Context& context, SemIR::LocId loc_id, SemIR::RefineTypeAction action) -> SemIR::InstId; -// Determines whether the given action depends on a template parameter in a way -// that means it cannot be performed immediately. -auto ActionIsDependent(Context& context, SemIR::Inst action_inst) -> bool; +// Determines whether the given action can be performed immediately (i.e. +// whether it is non-template-dependent). +auto ActionIsPerformable(Context& context, SemIR::Inst action_inst) -> bool; -// Determines whether the given action operand depends on a template parameter -// in a way that means the action cannot be performed immediately. -auto OperandIsDependent(Context& context, SemIR::InstId inst_id) -> bool; +// Returns the constant-dependence of `inst_id` (i.e. the maximum of the +// constant-dependences of its type and its value). +auto OperandDependence(Context& context, SemIR::InstId inst_id) + -> SemIR::ConstantDependence; +auto OperandDependence(Context& context, SemIR::TypeInstId inst_id) + -> SemIR::ConstantDependence; -// Determines whether the given action operand depends on a template parameter -// in a way that means the action cannot be performed immediately. -auto OperandIsDependent(Context& context, SemIR::TypeInstId inst_id) -> bool; - -// Determines whether the given type depends on a template parameter -// in a way that means the action cannot be performed immediately. -auto OperandIsDependent(Context& context, SemIR::TypeId type_id) -> bool; +// Returns the constant-dependence of `type_id` (i.e. the constant-dependence +// of the corresponding type constant). +auto OperandDependence(Context& context, SemIR::TypeId type_id) + -> SemIR::ConstantDependence; // Adds an instruction to the current block to splice in the result of // performing a dependent action. @@ -66,9 +66,7 @@ template auto HandleAction(Context& context, SemIR::LocId loc_id, ActionT action_inst, SemIR::TypeInstId result_type_inst_id = SemIR::TypeInstId::None) -> SemIR::InstId { - if (ActionIsDependent(context, action_inst) || - (result_type_inst_id.has_value() && - OperandIsDependent(context, result_type_inst_id))) { + if (!ActionIsPerformable(context, action_inst)) { return AddDependentActionSplice( context, SemIR::LocIdAndInst(loc_id, action_inst), result_type_inst_id); } @@ -93,7 +91,7 @@ auto EndPerformDelayedAction(Context& context, SemIR::InstId result_id) template auto PerformDelayedAction(Context& context, SemIR::LocId loc_id, ActionT action_inst) -> SemIR::InstId { - if (ActionIsDependent(context, action_inst)) { + if (!ActionIsPerformable(context, action_inst)) { return SemIR::InstId::None; } Internal::BeginPerformDelayedAction(context); diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 138b2b9a2722..c6869befa66c 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -1971,17 +1971,18 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id, // didn't apply in the template definition. // TODO: Support this for targets other than `Value`. if (sem_ir.insts().Get(expr_id).type_id() != target.type_id && - target.kind == ConversionTarget::Value && - (OperandIsDependent(context, expr_id) || - OperandIsDependent(context, target.type_id))) { + target.kind == ConversionTarget::Value) { auto target_type_inst_id = context.types().GetTypeInstId(target.type_id); - return AddDependentActionSplice( - context, loc_id, - SemIR::ConvertToValueAction{ - .type_id = GetSingletonType(context, SemIR::InstType::TypeInstId), - .inst_id = expr_id, - .target_type_inst_id = target_type_inst_id}, - target_type_inst_id); + SemIR::ConvertToValueAction convert_action = { + .type_id = GetSingletonType(context, SemIR::InstType::TypeInstId), + .inst_id = expr_id, + .target_type_inst_id = target_type_inst_id}; + // We don't use `HandleAction` here because it would call `PerformAction` + // inline if it's performable, which would lead to infinite recursion. + if (!ActionIsPerformable(context, convert_action)) { + return AddDependentActionSplice(context, loc_id, convert_action, + target_type_inst_id); + } } // If this is not a builtin conversion, try an `ImplicitAs` conversion. diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index b4103b47a963..e9b6d73da593 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -545,7 +545,8 @@ static auto GetConstantValue(EvalContext& eval_context, } // Otherwise, this is a normal instruction. - if (OperandIsDependent(eval_context.context(), inst_id)) { + if (OperandDependence(eval_context.context(), inst_id) == + SemIR::ConstantDependence::Template) { *phase = LatestPhase(*phase, Phase::TemplateSymbolic); } return inst_id;