Don't treat dependent types as having a copy value representation. (#6055)

Add `Dependent` value and initializing representations for types whose
representations are unknown because they are dependent. When generating
SemIR in such cases, use a worst-case initializing representation that
both provides a destination address and also propagates a potential
result value.

Use this to fix incorrect lowering and lowering crashes for specific
functions involving generic types that don't use a copy value
representation.

In lowering, be careful to distinguish between whether the initializing
representation for the generic return type uses a return slot (which
affects whether the SemIR declaration and call have one) and whether the
initializing representation for the specific return type uses a return
slot (which affects whether the LLVM IR declaration and call have one).

Author: zygoloid

toolchain/check/call.cpp

@@ -229,6 +229,7 @@ static auto PerformCallToFunction(Context& context, SemIR::LocId loc_id,
   SemIR::InstId return_slot_arg_id = SemIR::InstId::None;
   switch (return_info.init_repr.kind) {
     case SemIR::InitRepr::InPlace:
+    case SemIR::InitRepr::Dependent:
       // Tentatively put storage for a temporary in the function's return slot.
       // This will be replaced if necessary when we perform initialization.
       return_slot_arg_id = AddInst<SemIR::TemporaryStorage>(

toolchain/check/convert.cpp

@@ -42,8 +42,8 @@ namespace Carbon::Check {
 
 // Marks the initializer `init_id` as initializing `target_id`.
 static auto MarkInitializerFor(SemIR::File& sem_ir, SemIR::InstId init_id,
-                               SemIR::InstId target_id,
-                               PendingBlock& target_block) -> void {
+                               ConversionTarget& target) -> void {
+  CARBON_CHECK(target.is_initializer());
   auto return_slot_arg_id = FindReturnSlotArgForInitializer(sem_ir, init_id);
   if (return_slot_arg_id.has_value()) {
     // Replace the temporary in the return slot with a reference to our target.
@@ -52,7 +52,8 @@ static auto MarkInitializerFor(SemIR::File& sem_ir, SemIR::InstId init_id,
                  "Return slot for initializer does not contain a temporary; "
                  "initialized multiple times? Have {0}",
                  sem_ir.insts().Get(return_slot_arg_id));
-    target_block.MergeReplacing(return_slot_arg_id, target_id);
+    target.init_id =
+        target.init_block->MergeReplacing(return_slot_arg_id, target.init_id);
   }
 }
 
@@ -146,6 +147,29 @@ static auto MakeElementAccessInst(Context& context, SemIR::LocId loc_id,
   }
 }
 
+// Get the conversion target kind to use when initializing an element of an
+// aggregate.
+static auto GetAggregateElementConversionTargetKind(SemIR::File& sem_ir,
+                                                    ConversionTarget target)
+    -> ConversionTarget::Kind {
+  // If we're forming an initializer, then we want an initializer for each
+  // element.
+  if (target.is_initializer()) {
+    // Perform a final destination store if we're performing an in-place
+    // initialization.
+    auto init_repr = SemIR::InitRepr::ForType(sem_ir, target.type_id);
+    CARBON_CHECK(init_repr.kind != SemIR::InitRepr::Dependent,
+                 "Aggregate should not have dependent init kind");
+    if (init_repr.kind == SemIR::InitRepr::InPlace) {
+      return ConversionTarget::FullInitializer;
+    }
+    return ConversionTarget::Initializer;
+  }
+
+  // Otherwise, we want a value representation for each element.
+  return ConversionTarget::Value;
+}
+
 // Converts an element of one aggregate so that it can be used as an element of
 // another aggregate.
 //
@@ -336,17 +360,8 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
     return SemIR::ErrorInst::InstId;
   }
 
-  // If we're forming an initializer, then we want an initializer for each
-  // element. Otherwise, we want a value representation for each element.
-  // Perform a final destination store if we're performing an in-place
-  // initialization.
-  bool is_init = target.is_initializer();
   ConversionTarget::Kind inner_kind =
-      !is_init ? ConversionTarget::Value
-      : SemIR::InitRepr::ForType(sem_ir, target.type_id).kind ==
-              SemIR::InitRepr::InPlace
-          ? ConversionTarget::FullInitializer
-          : ConversionTarget::Initializer;
+      GetAggregateElementConversionTargetKind(sem_ir, target);
 
   // Initialize each element of the destination from the corresponding element
   // of the source.
@@ -373,7 +388,7 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
     new_block.Set(i, init_id);
   }
 
-  if (is_init) {
+  if (target.is_initializer()) {
     target.init_block->InsertHere();
     return AddInst<SemIR::TupleInit>(context, value_loc_id,
                                      {.type_id = target.type_id,
@@ -447,17 +462,8 @@ static auto ConvertStructToStructOrClass(
     }
   }
 
-  // If we're forming an initializer, then we want an initializer for each
-  // element. Otherwise, we want a value representation for each element.
-  // Perform a final destination store if we're performing an in-place
-  // initialization.
-  bool is_init = target.is_initializer();
   ConversionTarget::Kind inner_kind =
-      !is_init ? ConversionTarget::Value
-      : SemIR::InitRepr::ForType(sem_ir, target.type_id).kind ==
-              SemIR::InitRepr::InPlace
-          ? ConversionTarget::FullInitializer
-          : ConversionTarget::Initializer;
+      GetAggregateElementConversionTargetKind(sem_ir, target);
 
   // Initialize each element of the destination from the corresponding element
   // of the source.
@@ -545,6 +551,7 @@ static auto ConvertStructToStructOrClass(
     new_block.Set(i, init_id);
   }
 
+  bool is_init = target.is_initializer();
   if (ToClass) {
     target.init_block->InsertHere();
     CARBON_CHECK(is_init,
@@ -1282,30 +1289,33 @@ static auto IsCppEnum(Context& context, SemIR::TypeId type_id) -> bool {
 }
 
 // Given a value expression, form a corresponding initializer that copies from
-// that value, if it is possible to do so.
-static auto PerformCopy(Context& context, SemIR::InstId expr_id, bool diagnose)
-    -> SemIR::InstId {
+// that value to the specified target, if it is possible to do so.
+static auto PerformCopy(Context& context, SemIR::InstId expr_id,
+                        ConversionTarget& target) -> SemIR::InstId {
   // TODO: We don't have a mechanism yet to generate `Copy` impls for each enum
   // type imported from C++. For now we fake it by providing a direct copy.
-  if (IsCppEnum(context, context.insts().Get(expr_id).type_id())) {
-    return CopyValueToTemporary(context, expr_id);
+  auto type_id = context.insts().Get(expr_id).type_id();
+  if (IsCppEnum(context, type_id)) {
+    return expr_id;
   }
 
-  return BuildUnaryOperator(
+  auto copy_id = BuildUnaryOperator(
       context, SemIR::LocId(expr_id), {"Copy"}, expr_id, [&] {
-        if (!diagnose) {
+        if (!target.diagnose) {
           return context.emitter().BuildSuppressed();
         }
         CARBON_DIAGNOSTIC(CopyOfUncopyableType, Error,
                           "cannot copy value of type {0}", TypeOfInstId);
         return context.emitter().Build(expr_id, CopyOfUncopyableType, expr_id);
       });
+  MarkInitializerFor(context.sem_ir(), copy_id, target);
+  return copy_id;
 }
 
 // Convert a value expression so that it can be used to initialize a C++ thunk
 // parameter.
-static auto ConvertValueForCppThunkRef(Context& context, SemIR::InstId expr_id,
-                                       bool diagnose) -> SemIR::InstId {
+static auto ConvertValueForCppThunkRef(Context& context, SemIR::InstId expr_id)
+    -> SemIR::InstId {
   auto expr = context.insts().Get(expr_id);
 
   // If the expression has a pointer value representation, extract that and use
@@ -1319,8 +1329,13 @@ static auto ConvertValueForCppThunkRef(Context& context, SemIR::InstId expr_id,
 
   // Otherwise, we need a temporary to pass as the thunk argument. Create a copy
   // and initialize a temporary from it.
-  expr_id = PerformCopy(context, expr_id, diagnose);
-  return MaterializeIfInitializing(context, expr_id);
+  auto temporary_id = AddInst<SemIR::TemporaryStorage>(
+      context, SemIR::LocId(expr_id), {.type_id = expr.type_id()});
+  expr_id = Initialize(context, SemIR::LocId(expr_id), temporary_id, expr_id);
+  return AddInstWithCleanup<SemIR::Temporary>(context, SemIR::LocId(expr_id),
+                                              {.type_id = expr.type_id(),
+                                               .storage_id = temporary_id,
+                                               .init_id = expr_id});
 }
 
 // Returns the Core interface name to use for a given kind of conversion.
@@ -1524,8 +1539,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
           // a conversion. In that case, we will have created it with the
           // target already set.
           // TODO: Find a better way to track whether we need to do this.
-          MarkInitializerFor(sem_ir, expr_id, target.init_id,
-                             *target.init_block);
+          MarkInitializerFor(sem_ir, expr_id, target);
         }
         break;
       }
@@ -1577,13 +1591,13 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
 
       // When initializing from a value, perform a copy.
       if (target.is_initializer()) {
-        expr_id = PerformCopy(context, expr_id, target.diagnose);
+        expr_id = PerformCopy(context, expr_id, target);
       }
 
       // When initializing a C++ thunk parameter, form a reference, creating a
       // temporary if needed.
       if (target.kind == ConversionTarget::CppThunkRef) {
-        expr_id = ConvertValueForCppThunkRef(context, expr_id, target.diagnose);
+        expr_id = ConvertValueForCppThunkRef(context, expr_id);
       }
 
       break;
@@ -1592,7 +1606,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
   // Perform a final destination store, if necessary.
   if (target.kind == ConversionTarget::FullInitializer) {
     if (auto init_rep = SemIR::InitRepr::ForType(sem_ir, target.type_id);
-        init_rep.kind == SemIR::InitRepr::ByCopy) {
+        init_rep.MightBeByCopy()) {
       target.init_block->InsertHere();
       expr_id = AddInst<SemIR::InitializeFrom>(context, loc_id,
                                                {.type_id = target.type_id,

toolchain/check/pending_block.h

@@ -68,13 +68,17 @@ class PendingBlock {
   }
 
   // Replace the instruction at target_id with the instructions in this block.
-  // The new value for target_id should be value_id.
-  auto MergeReplacing(SemIR::InstId target_id, SemIR::InstId value_id) -> void {
+  // The new value for target_id should be value_id. Returns the InstId that
+  // should be used to refer to the result from now on.
+  auto MergeReplacing(SemIR::InstId target_id, SemIR::InstId value_id)
+      -> SemIR::InstId {
     SemIR::LocIdAndInst value = context_->insts().GetWithLocId(value_id);
 
+    auto result_id = value_id;
     if (insts_.size() == 1 && insts_[0] == value_id) {
       // The block is {value_id}. Replace `target_id` with the instruction
       // referred to by `value_id`. This is intended to be the common case.
+      result_id = target_id;
     } else {
       // Anything else: splice it into the IR, replacing `target_id`. This
       // includes empty blocks, which `Add` handles.
@@ -88,6 +92,7 @@ class PendingBlock {
 
     // Prepare to stash more pending instructions.
     insts_.clear();
+    return result_id;
   }
 
  private: