Switch generic type representation to be pessimistic.

Don't assume that generic types always have copy value representation
and don't need a return slot. Instead, make a conservative assumption
about the value representation, and assume a return slot might be
needed.

When lowering, if a return slot was not actually needed in a specific,
don't use one.

Author: zygoloid

core/prelude/copy.carbon

@@ -4,6 +4,8 @@
 
 package Core library "prelude/copy";
 
+import library "prelude/destroy";
+
 // Copying an object, which is a conversion from a value representation to an
 // initializing representation.
 interface Copy {
@@ -48,6 +50,7 @@ impl () as Copy {
   fn Op[self: Self]() -> Self = "no_op";
 }
 
+// TODO: This uses `T as Destroy`, but should not.
 impl forall [T:! Copy] (T,) as Copy {
   fn Op[self: Self]() -> Self {
     return (self.0.Op(),);

toolchain/check/testdata/builtins/int/make_type_signed.carbon

@@ -23,7 +23,7 @@ library "[[@TEST_NAME]]";
 
 import library "types";
 
-fn Copy[T:! type](x: T) -> T = "primitive_copy";
+fn Copy[N:! IntLiteral()](x: Int(N)) -> Int(N) = "primitive_copy";
 
 fn F(n: Int(64)) ->
     //@dump-sem-ir-begin

toolchain/check/testdata/builtins/int/make_type_unsigned.carbon

@@ -23,7 +23,7 @@ library "[[@TEST_NAME]]";
 
 import library "types";
 
-fn Copy[T:! type](x: T) -> T = "primitive_copy";
+fn Copy[N:! IntLiteral()](x: UInt(N)) -> UInt(N) = "primitive_copy";
 
 fn F(n: UInt(64)) ->
     //@dump-sem-ir-begin

toolchain/check/type_completion.cpp

@@ -66,6 +66,9 @@ class TypeCompleter {
   // state, such as empty structs and tuples.
   auto MakeEmptyValueRepr() const -> SemIR::ValueRepr;
 
+  // Makes a dependent value representation, which is used for symbolic types.
+  auto MakeDependentValueRepr(SemIR::TypeId type_id) const -> SemIR::ValueRepr;
+
   // Makes a value representation that uses pass-by-copy, copying the given
   // type.
   auto MakeCopyValueRepr(SemIR::TypeId rep_id,
@@ -165,8 +168,10 @@ class TypeCompleter {
     requires(InstT::Kind.is_symbolic_when_type())
   auto BuildInfoForInst(SemIR::TypeId type_id, InstT /*inst*/) const
       -> SemIR::CompleteTypeInfo {
-    // For symbolic types, we arbitrarily pick a copy representation.
-    return {.value_repr = MakeCopyValueRepr(type_id)};
+    // TODO: We're implicitly assuming here that dependent types are not
+    // abstract. How do we enforce that dependent types are non-abstract when
+    // they need to be?
+    return {.value_repr = MakeDependentValueRepr(type_id)};
   }
 
   // Builds and returns the `CompleteTypeInfo` for the given type. All nested
@@ -363,6 +368,11 @@ auto TypeCompleter::MakeEmptyValueRepr() const -> SemIR::ValueRepr {
           .type_id = GetTupleType(*context_, {})};
 }
 
+auto TypeCompleter::MakeDependentValueRepr(SemIR::TypeId type_id) const
+    -> SemIR::ValueRepr {
+  return {.kind = SemIR::ValueRepr::Dependent, .type_id = type_id};
+}
+
 auto TypeCompleter::MakeCopyValueRepr(
     SemIR::TypeId rep_id, SemIR::ValueRepr::AggregateKind aggregate_kind) const
     -> SemIR::ValueRepr {

toolchain/lower/file_context.cpp

@@ -357,6 +357,8 @@ auto FileContext::BuildFunctionTypeInfo(const SemIR::Function& function,
         return {.type = llvm::FunctionType::get(
                     llvm::Type::getVoidTy(llvm_context()),
                     /*isVarArg=*/false)};
+      case SemIR::ValueRepr::Dependent:
+        CARBON_FATAL("Lowering function with dependent parameter type");
       case SemIR::ValueRepr::None:
         break;
       case SemIR::ValueRepr::Copy:
@@ -630,25 +632,29 @@ auto FileContext::BuildFunctionBody(SemIR::FunctionId function_id,
     function_lowering.SetLocal(param_id, param_value);
   };
 
-  // The subset of call_param_ids that is already in the order that the LLVM
-  // calling convention expects.
-  llvm::ArrayRef<SemIR::InstId> sequential_param_ids;
+  // Lower the return slot parameter.
   if (declaration_function.return_slot_pattern_id.has_value()) {
+    auto call_param_id = call_param_ids.consume_back();
     // The LLVM calling convention has the return slot first rather than last.
     // Note that this queries whether there is a return slot at the LLVM level,
     // whereas `function.return_slot_pattern_id.has_value()` queries whether
     // there is a return slot at the SemIR level.
     if (SemIR::ReturnTypeInfo::ForFunction(sem_ir(), declaration_function,
                                            specific_id)
             .has_return_slot()) {
-      lower_param(call_param_ids.back());
+      lower_param(call_param_id);
+    } else {
+      // The return slot might still be mentioned as a destination location, but
+      // shouldn't actually be used for anything, so we can use a poison value
+      // for it.
+      function_lowering.SetLocal(call_param_id,
+                                 llvm::PoisonValue::get(llvm::PointerType::get(
+                                     llvm_context(), /*AddressSpace=*/0)));
     }
-    sequential_param_ids = call_param_ids.drop_back();
-  } else {
-    sequential_param_ids = call_param_ids;
   }
 
-  for (auto param_id : sequential_param_ids) {
+  // Lower the remaining call parameters.
+  for (auto param_id : call_param_ids) {
     lower_param(param_id);
   }
 

toolchain/lower/function_context.cpp

@@ -368,6 +368,8 @@ auto FunctionContext::CopyValue(TypeInFile type, SemIR::InstId source_id,
   switch (GetValueRepr(type).repr.kind) {
     case SemIR::ValueRepr::Unknown:
       CARBON_FATAL("Attempt to copy incomplete type");
+    case SemIR::ValueRepr::Dependent:
+      CARBON_FATAL("Attempt to copy dependent type");
     case SemIR::ValueRepr::None:
       break;
     case SemIR::ValueRepr::Copy:

toolchain/lower/handle.cpp

@@ -232,10 +232,7 @@ auto HandleInst(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/,
 
 auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
                 SemIR::ReturnSlot inst) -> void {
-  if (context.GetInitRepr(context.GetTypeIdOfInst(inst_id)).kind ==
-      SemIR::InitRepr::InPlace) {
-    context.SetLocal(inst_id, context.GetValue(inst.storage_id));
-  }
+  context.SetLocal(inst_id, context.GetValue(inst.storage_id));
 }
 
 auto HandleInst(FunctionContext& context, SemIR::InstId /*inst_id*/,

toolchain/lower/handle_aggregates.cpp

@@ -79,6 +79,8 @@ static auto GetAggregateElement(FunctionContext& context,
       switch (value_repr.repr.kind) {
         case SemIR::ValueRepr::Unknown:
           CARBON_FATAL("Lowering access to incomplete aggregate type");
+        case SemIR::ValueRepr::Dependent:
+          CARBON_FATAL("Lowering access to dependent aggregate type");
         case SemIR::ValueRepr::None:
           return aggr_value;
         case SemIR::ValueRepr::Copy:
@@ -241,7 +243,10 @@ static auto EmitAggregateValueRepr(FunctionContext& context,
   auto value_type = value_repr.type();
   switch (value_repr.repr.kind) {
     case SemIR::ValueRepr::Unknown:
-      CARBON_FATAL("Incomplete aggregate type in lowering");
+      CARBON_FATAL("Lowering value of incomplete aggregate type");
+
+    case SemIR::ValueRepr::Dependent:
+      CARBON_FATAL("Lowering value of dependent aggregate type");
 
     case SemIR::ValueRepr::None:
       // TODO: Add a helper to get a "no value representation" value.

toolchain/lower/handle_call.cpp

@@ -517,8 +517,16 @@ auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
   std::vector<llvm::Value*> args;
 
   auto inst_type = context.GetTypeIdOfInst(inst_id);
+  bool call_has_return_slot =
+      SemIR::ReturnTypeInfo::ForType(context.sem_ir(), inst.type_id)
+          .has_return_slot();
   if (context.GetReturnTypeInfo(inst_type).info.has_return_slot()) {
+    CARBON_CHECK(call_has_return_slot);
     args.push_back(context.GetValue(arg_ids.consume_back()));
+  } else if (call_has_return_slot) {
+    // Call instruction has a return slot but this specific callee does not.
+    // Just ignore it.
+    arg_ids.consume_back();
   }
 
   for (auto arg_id : arg_ids) {

toolchain/lower/handle_expr_category.cpp

@@ -15,6 +15,9 @@ auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
     case SemIR::ValueRepr::Unknown:
       CARBON_FATAL(
           "Value binding for type with incomplete value representation");
+    case SemIR::ValueRepr::Dependent:
+      CARBON_FATAL(
+          "Value binding for type with dependent value representation");
     case SemIR::ValueRepr::None:
       // Nothing should use this value, but StubRef needs a value to
       // propagate.
@@ -51,6 +54,8 @@ auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
       switch (context.GetValueRepr(type).repr.kind) {
         case SemIR::ValueRepr::Unknown:
           CARBON_FATAL("Unexpected incomplete type");
+        case SemIR::ValueRepr::Dependent:
+          CARBON_FATAL("Unexpected dependent type");
         case SemIR::ValueRepr::None:
         case SemIR::ValueRepr::Pointer:
           break;