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

Code before:
  // Add an argument to hold the result of the call, if necessary.
  auto return_type_id = callee_function.GetDeclaredReturnType(context.sem_ir());
  if (thunk_takes_return_address) {
    // Create a temporary if the caller didn't provide a return slot.
    if (!return_slot_id.has_value()) {
      return_slot_id = AddInst<SemIR::TemporaryStorage>(
          context, loc_id, {.type_id = return_type_id});
    }

    auto arg_id = AddInst<SemIR::AddrOf>(
        context, loc_id,
        {.type_id = GetPointerType(
             context, context.types().GetInstId(
                          context.insts().Get(return_slot_id).type_id())),
         .lvalue_id = return_slot_id});
    thunk_arg_ids.push_back(arg_id);
  } else if (return_slot_id.has_value()) {
    thunk_arg_ids.push_back(return_slot_id);
  }

  // Compute the return type of the call to the thunk.
  auto thunk_return_type_id =
      thunk_function.GetDeclaredReturnType(context.sem_ir());
  if (!thunk_return_type_id.has_value()) {
    thunk_return_type_id = GetTupleType(context, {});
    CARBON_CHECK(thunk_takes_return_address || !return_type_id.has_value());
  } else {
    CARBON_CHECK(thunk_return_type_id == return_type_id);
  }

  auto result_id = GetOrAddInst<SemIR::Call>(
      context, loc_id,
      {.type_id = thunk_return_type_id,
       .callee_id = thunk_callee_id,
       .args_id = context.inst_blocks().Add(thunk_arg_ids)});

  // Produce the result of the call, taking the value from the return storage.
  if (thunk_takes_return_address) {
    result_id = AddInst<SemIR::InPlaceInit>(context, loc_id,
                                            {.type_id = return_type_id,
                                             .src_id = result_id,
                                             .dest_id = return_slot_id});
  }

  return result_id;
}

}  // namespace Carbon::Check


Review comment (suggestion):
```suggestion
    CARBON_CHECK(thunk_takes_return_address || !return_type_id.has_value());
    thunk_return_type_id = GetTupleType(context, {});
```
Can the `CHECK` be moved above the assignment? (establish the invariant as early as possible - and also I misread this code a few times missing the difference between `thunk_return_type_id` and `return_type_id` & was confused why `return_type_id` would ever not have a value when it (really `think_return_type_id`) had been assigned to on the previous line)

Code after:
  // Add an argument to hold the result of the call, if necessary.
  auto return_type_id = callee_function.GetDeclaredReturnType(context.sem_ir());
  if (thunk_takes_return_address) {
    // Create a temporary if the caller didn't provide a return slot.
    if (!return_slot_id.has_value()) {
      return_slot_id = AddInst<SemIR::TemporaryStorage>(
          context, loc_id, {.type_id = return_type_id});
    }

    auto arg_id = AddInst<SemIR::AddrOf>(
        context, loc_id,
        {.type_id = GetPointerType(
             context, context.types().GetInstId(
                          context.insts().Get(return_slot_id).type_id())),
         .lvalue_id = return_slot_id});
    thunk_arg_ids.push_back(arg_id);
  } else if (return_slot_id.has_value()) {
    thunk_arg_ids.push_back(return_slot_id);
  }

  // Compute the return type of the call to the thunk.
  auto thunk_return_type_id =
      thunk_function.GetDeclaredReturnType(context.sem_ir());
  if (!thunk_return_type_id.has_value()) {
    CARBON_CHECK(thunk_takes_return_address || !return_type_id.has_value());
    thunk_return_type_id = GetTupleType(context, {});
  } else {
    CARBON_CHECK(thunk_return_type_id == return_type_id);
  }

  auto result_id = GetOrAddInst<SemIR::Call>(
      context, loc_id,
      {.type_id = thunk_return_type_id,
       .callee_id = thunk_callee_id,
       .args_id = context.inst_blocks().Add(thunk_arg_ids)});

  // Produce the result of the call, taking the value from the return storage.
  if (thunk_takes_return_address) {
    result_id = AddInst<SemIR::InPlaceInit>(context, loc_id,
                                            {.type_id = return_type_id,
                                             .src_id = result_id,
                                             .dest_id = return_slot_id});
  }

  return result_id;
}

}  // namespace Carbon::Check