Repository: carbon-language/carbon-lang
File: toolchain/sem_ir/formatter.cpp (C++)

Code before:
  }
  // Similarly, instructions that have a `DestInstId` as the second operand
  // typically use it for the output argument, so we omit it because it should
  // already be part of the inst's formatted form expression.
  if (arg1.kind() == IdKind::For<DestInstId>) {
    return;
  }
  out_ << ", ";
  FormatInstArgAndKind(arg1);
}

auto Formatter::FormatCallRhs(Call inst) -> void {
  out_ << " ";
  FormatArg(inst.callee_id);

  if (!inst.args_id.has_value()) {
    out_ << "(<none>)";
    return;
  }

  llvm::ArrayRef<InstId> args = sem_ir_->inst_blocks().Get(inst.args_id);

  // If there's a return argument, don't print it here, because it's printed on
  // the LHS.
  int return_arg_index = -1;
  // TODO: Should GetCalleeAsFunction handle errors better?
  if (sem_ir_->constant_values().Get(inst.callee_id) !=
      SemIR::ErrorInst::ConstantId) {
    auto callee_function = SemIR::GetCalleeAsFunction(*sem_ir_, inst.callee_id);
    auto function = sem_ir_->functions().Get(callee_function.function_id);
    auto return_form_id = function.GetDeclaredReturnForm(
        *sem_ir_, callee_function.resolved_specific_id);
    if (return_form_id.has_value()) {
      if (auto init_form =
              sem_ir_->insts().TryGetAs<SemIR::InitForm>(return_form_id)) {
        auto type_id = sem_ir_->types().GetTypeIdForTypeInstId(
            init_form->type_component_inst_id);
        if (SemIR::InitRepr::ForType(*sem_ir_, type_id).MightBeInPlace()) {
          return_arg_index = init_form->index.index;
        }
      }
    }
  }

  llvm::ListSeparator sep;
  out_ << '(';
  for (auto [i, inst_id] : llvm::enumerate(args)) {
    if (static_cast<int>(i) == return_arg_index) {
      continue;
    }
    out_ << sep;

Review comment (bug):
FWIW, my sense on this would be that since `GetCalleeAsFunction` is called in a few spots, it may be effectively asserting in those spots that it's _not_ being passed an error (i.e., this code was incorrect, and it's providing useful validation of logic)

Even here, you could switch to just `GetCallee` and return -1 if it's not a function... i.e.:

```
  auto callee = GetCallee(*sem_ir_, inst.callee_id);
  if (auto callee_function = std::get_if<CalleeFunction>(GetCallee(*sem_ir_, inst.callee_id))) {
    ...
  }
```


Code after:
  }
  // Similarly, instructions that have a `DestInstId` as the second operand
  // typically use it for the output argument, so we omit it because it should
  // already be part of the inst's formatted form expression.
  if (arg1.kind() == IdKind::For<DestInstId>) {
    return;
  }
  out_ << ", ";
  FormatInstArgAndKind(arg1);
}

auto Formatter::FormatCallRhs(Call inst) -> void {
  out_ << " ";
  FormatArg(inst.callee_id);

  if (!inst.args_id.has_value()) {
    out_ << "(<none>)";
    return;
  }

  llvm::ArrayRef<InstId> args = sem_ir_->inst_blocks().Get(inst.args_id);

  // If there's a return argument, don't print it here, because it's printed on
  // the LHS.
  int return_arg_index = -1;
  auto callee = GetCallee(*sem_ir_, inst.callee_id);
  if (auto* callee_function = std::get_if<CalleeFunction>(&callee)) {
    auto function = sem_ir_->functions().Get(callee_function->function_id);
    auto return_form_id = function.GetDeclaredReturnForm(
        *sem_ir_, callee_function->resolved_specific_id);
    if (return_form_id.has_value()) {
      if (auto init_form =
              sem_ir_->insts().TryGetAs<InitForm>(return_form_id)) {
        auto type_id = sem_ir_->types().GetTypeIdForTypeInstId(
            init_form->type_component_inst_id);
        if (InitRepr::ForType(*sem_ir_, type_id).MightBeInPlace()) {
          return_arg_index = init_form->index.index;
        }
      }
    }
  }

  llvm::ListSeparator sep;
  out_ << '(';
  for (auto [i, inst_id] : llvm::enumerate(args)) {
    if (static_cast<int>(i) == return_arg_index) {
      continue;
    }
    out_ << sep;
    FormatArg(inst_id);
  }