Repository: carbon-language/carbon-lang
File: toolchain/lower/file_context.cpp (C++)

Code before:
                                    SemIR::SpecificId specific_id)
    -> llvm::Function* {
  const auto& function = sem_ir().functions().Get(function_id);

  // Don't lower generic functions. Note that associated functions in interfaces
  // have `Self` in scope, so are implicitly generic functions.
  if (function.generic_id.has_value() && !specific_id.has_value()) {
    return nullptr;
  }

  // Don't lower builtins.
  if (function.builtin_function_kind != SemIR::BuiltinFunctionKind::None) {
    return nullptr;
  }

  // TODO: Consider tracking whether the function has been used, and only
  // lowering it if it's needed.

  // TODO nit: add is_symbolic() to type_id to forward to
  // type_id.AsConstantId().is_symbolic(). Update call below too.
  auto get_llvm_type = [&](SemIR::TypeId type_id) -> llvm::Type* {
    return type_id.has_value() ? (type_id.AsConstantId().is_symbolic()
                                      ? GetType(SemIR::GetTypeInSpecific(
                                            sem_ir(), specific_id, type_id))
                                      : GetType(type_id))
                               : nullptr;
  };

  const auto return_info =
      SemIR::ReturnTypeInfo::ForFunction(sem_ir(), function, specific_id);
  CARBON_CHECK(return_info.is_valid(), "Should not lower invalid functions.");

  auto implicit_param_patterns =
      sem_ir().inst_blocks().GetOrEmpty(function.implicit_param_patterns_id);
  // TODO: Include parameters corresponding to positional parameters.
  auto param_patterns =
      sem_ir().inst_blocks().GetOrEmpty(function.param_patterns_id);

  auto* return_type = get_llvm_type(return_info.type_id);

  llvm::SmallVector<llvm::Type*> param_types;
  // TODO: Consider either storing `param_inst_ids` somewhere so that we can
  // reuse it from `BuildFunctionDefinition` and when building calls, or factor
  // out a mechanism to compute the mapping between parameters and arguments on
  // demand.
  llvm::SmallVector<SemIR::InstId> param_inst_ids;
  auto max_llvm_params = (return_info.has_return_slot() ? 1 : 0) +
                         implicit_param_patterns.size() + param_patterns.size();
  param_types.reserve(max_llvm_params);
  param_inst_ids.reserve(max_llvm_params);
  auto return_param_id = SemIR::InstId::None;

Review comment (bug):
Maybe nested conditional operators are a bit confusing/worth writing out, maybe:
```
if (!type_id.has_value()) {
  return nullptr;
}
if (type_id.AsConstantId().is_symbolic()) {
  return GetType(SemIR::GetTypeInSpecific(...));
}
return GetType(type_id);
```
and/or I guess maybe popping the `GetType(` out could be valid:
```
return GetType(type_id.isConstantId().is_symbolic() ? SemIR::GetTypeInSpecific(...) : type_id);
```

Code after:

auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id,
                                    SemIR::SpecificId specific_id)
    -> llvm::Function* {
  const auto& function = sem_ir().functions().Get(function_id);

  // Don't lower generic functions. Note that associated functions in interfaces
  // have `Self` in scope, so are implicitly generic functions.
  if (function.generic_id.has_value() && !specific_id.has_value()) {
    return nullptr;
  }

  // Don't lower builtins.
  if (function.builtin_function_kind != SemIR::BuiltinFunctionKind::None) {
    return nullptr;
  }

  // TODO: Consider tracking whether the function has been used, and only
  // lowering it if it's needed.

  // TODO nit: add is_symbolic() to type_id to forward to
  // type_id.AsConstantId().is_symbolic(). Update call below too.
  auto get_llvm_type = [&](SemIR::TypeId type_id) -> llvm::Type* {
    if (!type_id.has_value()) {
      return nullptr;
    }
    return GetType(SemIR::GetTypeInSpecific(sem_ir(), specific_id, type_id));
  };

  const auto return_info =
      SemIR::ReturnTypeInfo::ForFunction(sem_ir(), function, specific_id);
  CARBON_CHECK(return_info.is_valid(), "Should not lower invalid functions.");

  auto implicit_param_patterns =
      sem_ir().inst_blocks().GetOrEmpty(function.implicit_param_patterns_id);
  // TODO: Include parameters corresponding to positional parameters.
  auto param_patterns =
      sem_ir().inst_blocks().GetOrEmpty(function.param_patterns_id);

  auto* return_type = get_llvm_type(return_info.type_id);

  llvm::SmallVector<llvm::Type*> param_types;
  // TODO: Consider either storing `param_inst_ids` somewhere so that we can
  // reuse it from `BuildFunctionDefinition` and when building calls, or factor
  // out a mechanism to compute the mapping between parameters and arguments on
  // demand.
  llvm::SmallVector<SemIR::InstId> param_inst_ids;
  auto max_llvm_params = (return_info.has_return_slot() ? 1 : 0) +
                         implicit_param_patterns.size() + param_patterns.size();
  param_types.reserve(max_llvm_params);
  param_inst_ids.reserve(max_llvm_params);