Repository: carbon-language/carbon-lang
File: toolchain/check/import_ref.cpp (C++)

Code before:
template <typename InstT>
static auto AddImportedInst(
    ImportContext& context, SemIR::InstId import_inst_id, InstT inst,
    std::optional<SemIR::ConstantId> const_id = std::nullopt) -> SemIR::InstId {
  auto inst_id = context.local_insts().AddInNoBlock(MakeImportedLocIdAndInst(
      context.local_context(), AddImportIRInst(context, import_inst_id), inst));
  CARBON_VLOG_TO(context.local_context().vlog_stream(),
                 "AddImportedInst: {0}\n", static_cast<SemIR::Inst>(inst));
  ComputeOrSetConstantValue(context.local_context(), inst_id, inst, const_id);
  return inst_id;
}

// Convenience for AddImportedInst for placeholder instructions. The constant
// value is set to `None`; the instruction should later be updated by
// `ReplaceImoprtedInstAndSetConstantValue`.
template <typename InstT>
static auto AddPlaceholderImportedInst(ImportContext& context,
                                       SemIR::InstId import_decl_id, InstT inst)
    -> SemIR::InstId {
  return AddImportedInst(context, import_decl_id, inst,
                         SemIR::ConstantId::None);
}

// Replace an imported instruction with a new instruction. The constant value of
// the instruction must previously have been set to `None`, for example by using
// `AddPlaceholderImportedInst`. The new constant value can be specified if
// known, and is otherwise computed. Sets and returns the constant value.
static auto ReplaceImportedInstAndSetConstantValue(
    Context& context, SemIR::InstId inst_id, SemIR::Inst inst,
    std::optional<SemIR::ConstantId> const_id = std::nullopt)
    -> SemIR::ConstantId {
  CARBON_VLOG_TO(context.vlog_stream(), "ReplaceImportedInst: {0} -> {1}\n",
                 inst_id, inst);
  context.sem_ir().insts().Set(inst_id, inst);

  CARBON_CHECK(context.constant_values().Get(inst_id) ==
               SemIR::ConstantId::None);
  return ComputeOrSetConstantValue(context, inst_id, inst, const_id);
}

// Returns the ConstantId for an InstId. Adds unresolved constants to
// the resolver's work stack.
static auto GetLocalConstantId(ImportRefResolver& resolver,
                               SemIR::InstId inst_id) -> SemIR::ConstantId {
  return resolver.GetLocalConstantValueOrPush(inst_id);
}

// Returns the ConstantId for an imported ConstantId. Adds unresolved
// constants to the resolver's work stack.
static auto GetLocalConstantId(ImportRefResolver& resolver,
                               SemIR::ConstantId const_id)

Review comment (suggestion):
You note "for example": are there other ways import code will do that? I think we should be specific if possible. One answer might be to drop "for example", another might be to add a second example.

IIUC, the `GetLocalConstantIdChecked` call in `SymbolicBindingPattern` code won't ever return `None`, but this comment has me wondering if I'm missing something.

Code after:
//
// Normally `AddImportedConstant` should be used to create the result of
// resolving a constant, for example by calling `ResolveAs*`. However, we
// sometimes need to create a new instruction, not just obtain a constant value,
// such as when importing a declaration or a pattern. In that case, this
// function should be used to create it.
//
// Do not use `AddInst*`, as it will add symbolic constants to the eval block of
// whatever generic the import happens within.
template <typename InstT>
static auto AddImportedInst(ImportContext& context,
                            SemIR::InstId import_inst_id, InstT inst)
    -> SemIR::InstId {
  auto inst_id = AddPlaceholderImportedInst(context, import_inst_id, inst);
  SetConstantValue(context.local_context(), inst_id, inst);
  return inst_id;
}

// Replace an imported instruction that was added by
// `AddPlaceholderImportedInst` with a new instruction. Computes, sets, and
// returns the new constant value.
static auto ReplacePlaceholderImportedInst(ImportContext& context,
                                           SemIR::InstId inst_id,
                                           SemIR::Inst inst)
    -> SemIR::ConstantId {
  CARBON_VLOG_TO(context.local_context().vlog_stream(),
                 "ReplaceImportedInst: {0} -> {1}\n", inst_id, inst);
  context.local_insts().Set(inst_id, inst);

  CARBON_CHECK(context.local_constant_values().Get(inst_id) ==
               SemIR::ConstantId::NotConstant);
  return SetConstantValue(context.local_context(), inst_id, inst);
}

// Returns the ConstantId for an InstId. Adds unresolved constants to
// the resolver's work stack.
static auto GetLocalConstantId(ImportRefResolver& resolver,
                               SemIR::InstId inst_id) -> SemIR::ConstantId {
  return resolver.GetLocalConstantValueOrPush(inst_id);
}

// Returns the ConstantId for an imported ConstantId. Adds unresolved
// constants to the resolver's work stack.
static auto GetLocalConstantId(ImportRefResolver& resolver,
                               SemIR::ConstantId const_id)
    -> SemIR::ConstantId {
  return GetLocalConstantId(
      resolver, GetInstWithConstantValue(resolver.import_ir(), const_id));
}

// Returns the local constant InstId for an imported InstId.