Function decl lowering for incomplete parameter/return types (#5038) (#5066)

While current examples of this could also be addressed by emitting
declarations on use (by which stage the associated types would have to
be complete by construction) - it's expected that future examples
(vtables, function pointers) will need to work in this case anyway, so
might as well implement this feature.

toolchain/lower/file_context.cpp

@@ -200,25 +200,19 @@ auto FileContext::GetOrCreateFunction(SemIR::FunctionId function_id,
   return result;
 }
 
-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;
-  }
+auto FileContext::BuildFunctionTypeInfo(const SemIR::Function& function,
+                                        SemIR::SpecificId specific_id)
+    -> FunctionTypeInfo {
+  const auto return_info =
+      SemIR::ReturnTypeInfo::ForFunction(sem_ir(), function, specific_id);
 
-  // Don't lower builtins.
-  if (function.builtin_function_kind != SemIR::BuiltinFunctionKind::None) {
-    return nullptr;
+  if (!return_info.is_valid()) {
+    // The return type has not been completed, create a trivial type instead.
+    return {.type =
+                llvm::FunctionType::get(llvm::Type::getVoidTy(llvm_context()),
+                                        /*isVarArg=*/false)};
   }
 
-  // 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* {
@@ -228,10 +222,6 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id,
     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.
@@ -241,6 +231,16 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id,
   auto* return_type = get_llvm_type(return_info.type_id);
 
   llvm::SmallVector<llvm::Type*> param_types;
+  // Compute the return type to use for the LLVM function. If the initializing
+  // representation doesn't produce a value, set the return type to void.
+  // TODO: For the `Run` entry point, remap return type to i32 if it doesn't
+  // return a value.
+  llvm::Type* function_return_type =
+      (return_info.is_valid() &&
+       return_info.init_repr.kind == SemIR::InitRepr::ByCopy)
+          ? return_type
+          : llvm::Type::getVoidTy(llvm_context());
+
   // 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
@@ -273,7 +273,11 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id,
     switch (auto value_rep = SemIR::ValueRepr::ForType(sem_ir(), param_type_id);
             value_rep.kind) {
       case SemIR::ValueRepr::Unknown:
-        CARBON_FATAL("Incomplete parameter type lowering function declaration");
+        // This parameter type is incomplete. Fallback to describing the
+        // function type as `void()`.
+        return {.type = llvm::FunctionType::get(
+                    llvm::Type::getVoidTy(llvm_context()),
+                    /*isVarArg=*/false)};
       case SemIR::ValueRepr::None:
         break;
       case SemIR::ValueRepr::Copy:
@@ -285,36 +289,52 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id,
         break;
     }
   }
+  return {.type = llvm::FunctionType::get(function_return_type, param_types,
+                                          /*isVarArg=*/false),
+          .param_inst_ids = std::move(param_inst_ids),
+          .return_type = return_type,
+          .return_param_id = return_param_id};
+}
 
-  // Compute the return type to use for the LLVM function. If the initializing
-  // representation doesn't produce a value, set the return type to void.
-  // TODO: For the `Run` entry point, remap return type to i32 if it doesn't
-  // return a value.
-  llvm::Type* function_return_type =
-      return_info.init_repr.kind == SemIR::InitRepr::ByCopy
-          ? return_type
-          : llvm::Type::getVoidTy(llvm_context());
+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.
+
+  auto function_type_info = BuildFunctionTypeInfo(function, specific_id);
 
   Mangler m(*this);
   std::string mangled_name = m.Mangle(function_id, specific_id);
 
-  llvm::FunctionType* function_type = llvm::FunctionType::get(
-      function_return_type, param_types, /*isVarArg=*/false);
-  auto* llvm_function =
-      llvm::Function::Create(function_type, llvm::Function::ExternalLinkage,
-                             mangled_name, llvm_module());
+  auto* llvm_function = llvm::Function::Create(function_type_info.type,
+                                               llvm::Function::ExternalLinkage,
+                                               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(param_inst_ids, llvm_function->args())) {
+  for (auto [inst_id, arg] : llvm::zip_equal(function_type_info.param_inst_ids,
+                                             llvm_function->args())) {
     auto name_id = SemIR::NameId::None;
-    if (inst_id == return_param_id) {
+    if (inst_id == function_type_info.return_param_id) {
       name_id = SemIR::NameId::ReturnSlot;
-      arg.addAttr(
-          llvm::Attribute::getWithStructRetType(llvm_context(), return_type));
+      arg.addAttr(llvm::Attribute::getWithStructRetType(
+          llvm_context(), function_type_info.return_type));
     } else {
       name_id = SemIR::Function::GetNameFromPatternId(sem_ir(), inst_id);
     }

toolchain/lower/file_context.h

@@ -96,6 +96,20 @@ class FileContext {
   }
 
  private:
+  struct FunctionTypeInfo {
+    llvm::FunctionType* type;
+    llvm::SmallVector<SemIR::InstId> param_inst_ids;
+    llvm::Type* return_type = nullptr;
+    SemIR::InstId return_param_id = SemIR::InstId::None;
+  };
+
+  // Retrieve various features of the function's type useful for constructing
+  // the `llvm::Type` for the `llvm::Function`. If any part of the type can't be
+  // manifest (eg: incomplete return or parameter types), then the result is as
+  // if the type was `void()`.
+  auto BuildFunctionTypeInfo(const SemIR::Function& function,
+                             SemIR::SpecificId specific_id) -> FunctionTypeInfo;
+
   // Builds the declaration for the given function, which should then be cached
   // by the caller.
   auto BuildFunctionDecl(SemIR::FunctionId function_id,

toolchain/lower/testdata/function/declaration/simple.carbon

@@ -12,6 +12,10 @@ fn F(n: i32);
 
 fn G(n: i32) { F(n); }
 
+class A;
+fn H(n: A);
+fn I() -> A;
+
 // CHECK:STDOUT: ; ModuleID = 'simple.carbon'
 // CHECK:STDOUT: source_filename = "simple.carbon"
 // CHECK:STDOUT:
@@ -23,6 +27,10 @@ fn G(n: i32) { F(n); }
 // CHECK:STDOUT:   ret void, !dbg !8
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: declare void @_CH.Main()
+// CHECK:STDOUT:
+// CHECK:STDOUT: declare void @_CI.Main()
+// CHECK:STDOUT:
 // CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
 // CHECK:STDOUT: !llvm.dbg.cu = !{!2}
 // CHECK:STDOUT: