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

Code before:
    return std::nullopt;
  }

  // Don't lower C++ functions that use a thunk. We will never reference them
  // directly, and their signatures would not be expected to match the
  // corresponding C++ function anyway.
  if (function.special_function_kind ==
      SemIR::Function::SpecialFunctionKind::HasCppThunk) {
    return std::nullopt;
  }

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

  auto function_type_info =
      FunctionTypeInfoBuilder(this, specific_id).Build(function);
  auto make_function_info =
      [&](llvm::Function* llvm_function) -> std::optional<FunctionInfo> {
    return {{.type = function_type_info.type,
             .di_type = function_type_info.di_type,
             .lowered_param_pattern_ids =
                 std::move(function_type_info.lowered_param_pattern_ids),
             .unused_param_pattern_ids =
                 std::move(function_type_info.unused_param_pattern_ids),
             .llvm_function = llvm_function}};
  };

  // TODO: For an imported inline function, consider generating an
  // `available_externally` definition.
  auto linkage = specific_id.has_value() ? llvm::Function::LinkOnceODRLinkage
                                         : llvm::Function::ExternalLinkage;
  if (function_id == sem_ir().global_ctor_id()) {
    // The global constructor name would collide with global constructors for
    // other files in the same package, so use an internal linkage symbol.
    linkage = llvm::Function::InternalLinkage;
  }

  // If this is a C++ function, tell Clang that we referenced it.
  if (auto clang_decl_id = sem_ir().functions().Get(function_id).clang_decl_id;
      clang_decl_id.has_value()) {
    CARBON_CHECK(!specific_id.has_value(),
                 "Specific functions cannot have C++ definitions");
    return make_function_info(HandleReferencedCppFunction(
        sem_ir().clang_decls().Get(clang_decl_id).key.decl->getAsFunction()));
  }

  Mangler m(*this);
  std::string mangled_name = m.Mangle(function_id, specific_id);
  if (auto* existing = llvm_module().getFunction(mangled_name)) {
    // We might have already lowered this function while lowering a different
    // file. That's OK.

Review comment (bug):
Could this be done without the lambda, to have simpler flow in the function? e.g.:

```suggestion
  auto ret = std::make_optional<FunctionInfo>(
            {.type = function_type_info.type,
             .di_type = function_type_info.di_type,
             .lowered_param_pattern_ids =
                 std::move(function_type_info.lowered_param_pattern_ids),
             .unused_param_pattern_ids =
                 std::move(function_type_info.unused_param_pattern_ids),
             // Assigned before returning.
             .llvm_function = nullptr});
```

Then `ret->llvm_function = ...; return ret;` as needed.

Alternately, it looks a bit like you could create a `MakeLLVMFunction` function and assign immediately (`.llvm_function = MakeLLVMFunction(context, ...)`) -- all the below return paths seem to produce one, so perhaps a helper would be a better fit?

(ed: i.e. `return FunctionInfo({..., .llvm_function = MakeLLVMFunction(...)});` here, making this point the end of the function)

One side-effect of this is it'd make clear whether `function_type_info` can change before the lambda reads it (it doesn't look like it does, maybe I'm missing it?)

Code after:
  std::string mangled_name = m.Mangle(function_id, specific_id);
  if (auto* existing = llvm_module().getFunction(mangled_name)) {
    // We might have already lowered this function while lowering a different
    // file. That's OK.
    // TODO: Check-fail or maybe diagnose if the two LLVM functions are not
    // produced by declarations of the same Carbon function. Name collisions
    // between non-private members of the same library should have been
    // diagnosed by check if detected, but it's not clear that check will
    // always be able to see this problem. In theory, name collisions could
    // also occur due to fingerprint collision.
    return existing;
  }

  // If this is a specific function, we may need to do additional work to
  // emit its definition.
  if (specific_id.has_value()) {
    HandleReferencedSpecificFunction(function_id, specific_id,
                                     function_type_info.type);
  }

  // TODO: For an imported inline function, consider generating an
  // `available_externally` definition.
  auto linkage = specific_id.has_value() ? llvm::Function::LinkOnceODRLinkage
                                         : llvm::Function::ExternalLinkage;
  if (function_id == sem_ir().global_ctor_id()) {
    // The global constructor name would collide with global constructors for
    // other files in the same package, so use an internal linkage symbol.
    linkage = llvm::Function::InternalLinkage;
  }

  auto* llvm_function = llvm::Function::Create(function_type_info.type, linkage,
                                               mangled_name, llvm_module());
  CARBON_CHECK(llvm_function->getName() == mangled_name,
               "Mangled name collision: {0}", mangled_name);

  // Set up parameters and the return slot.
  for (auto [inst_id, arg] :
       llvm::zip_equal(function_type_info.lowered_param_pattern_ids,
                       llvm_function->args())) {
    arg.setName(sem_ir().names().GetIRBaseName(
        SemIR::GetPrettyNameFromPatternId(sem_ir(), inst_id)));
  }
  if (function_type_info.sret_type != nullptr) {
    auto& return_arg = *llvm_function->args().begin();
    return_arg.addAttr(llvm::Attribute::getWithStructRetType(
        llvm_context(), function_type_info.sret_type));
  }

  return llvm_function;
}