Fix lowering of a conversion from a type with a pointer value representation to a type with a copy value representation. (#4467)

We previously generated a `value_bind` instruction of the wrong type,
resulting in lowering building bad LLVM IR.

Also fix another issue exposed by this change, where we would import
constants without marking their types as complete, and then crash in
lowering while trying to lower them. This happens in particular for the
`FunctionType`s of functions in `ImplDecl`s. Address this by skipping
lowering for constants with incomplete types.

toolchain/check/convert.cpp

@@ -975,8 +975,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
   }
 
   // If this is not a builtin conversion, try an `ImplicitAs` conversion.
-  SemIR::Inst expr = sem_ir.insts().Get(expr_id);
-  if (expr.type_id() != target.type_id) {
+  if (sem_ir.insts().Get(expr_id).type_id() != target.type_id) {
     SemIR::InstId interface_args[] = {
         context.types().GetInstId(target.type_id)};
     Operator op = {
@@ -1019,7 +1018,8 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
   switch (SemIR::GetExprCategory(sem_ir, expr_id)) {
     case SemIR::ExprCategory::NotExpr:
     case SemIR::ExprCategory::Mixed:
-      CARBON_FATAL("Unexpected expression {0} after builtin conversions", expr);
+      CARBON_FATAL("Unexpected expression {0} after builtin conversions",
+                   sem_ir.insts().Get(expr_id));
 
     case SemIR::ExprCategory::Error:
       return SemIR::InstId::BuiltinError;
@@ -1057,7 +1057,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
       // TODO: Support types with custom value representations.
       expr_id = context.AddInst<SemIR::BindValue>(
           context.insts().GetLocId(expr_id),
-          {.type_id = expr.type_id(), .value_id = expr_id});
+          {.type_id = target.type_id, .value_id = expr_id});
       // We now have a value expression.
       [[fallthrough]];
 

toolchain/check/testdata/operators/overloaded/implicit_as.carbon

@@ -313,7 +313,7 @@ fn Test() {
 // CHECK:STDOUT:   %.loc30_18.7: init i32 = converted %Source.call.loc30, %Convert.call.loc30
 // CHECK:STDOUT:   %.loc30_18.8: ref i32 = temporary_storage
 // CHECK:STDOUT:   %.loc30_18.9: ref i32 = temporary %.loc30_18.8, %.loc30_18.7
-// CHECK:STDOUT:   %.loc30_18.10: %X = bind_value %.loc30_18.9
+// CHECK:STDOUT:   %.loc30_18.10: i32 = bind_value %.loc30_18.9
 // CHECK:STDOUT:   %Sink_i32.call: init %.1 = call %Sink_i32.ref(%.loc30_18.10)
 // CHECK:STDOUT:   %Sink_X.ref: %Sink_X.type = name_ref Sink_X, file.%Sink_X.decl [template = constants.%Sink_X]
 // CHECK:STDOUT:   %Source.ref.loc31: %Source.type = name_ref Source, file.%Source.decl [template = constants.%Source]
@@ -334,7 +334,7 @@ fn Test() {
 // CHECK:STDOUT:   %Convert.call.loc31: init %X = call %.loc31_16.7(%.loc31_16.9) to %.loc31_16.8
 // CHECK:STDOUT:   %.loc31_16.10: init %X = converted %Source.call.loc31, %Convert.call.loc31
 // CHECK:STDOUT:   %.loc31_16.11: ref %X = temporary %.loc31_16.8, %.loc31_16.10
-// CHECK:STDOUT:   %.loc31_16.12: i32 = bind_value %.loc31_16.11
+// CHECK:STDOUT:   %.loc31_16.12: %X = bind_value %.loc31_16.11
 // CHECK:STDOUT:   %Sink_X.call: init %.1 = call %Sink_X.ref(%.loc31_16.12)
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }

toolchain/lower/constant.cpp

@@ -260,6 +260,12 @@ auto LowerConstants(FileContext& file_context,
     }
 
     auto inst = file_context.sem_ir().insts().Get(inst_id);
+    if (inst.type_id().is_valid() &&
+        !file_context.sem_ir().types().IsComplete(inst.type_id())) {
+      // If a constant doesn't have a complete type, that means we imported it
+      // but didn't actually use it.
+      continue;
+    }
     llvm::Constant* value = nullptr;
     CARBON_KIND_SWITCH(inst) {
 #define CARBON_SEM_IR_INST_KIND(Name)            \

toolchain/lower/file_context.h

@@ -55,7 +55,8 @@ class FileContext {
   auto GetType(SemIR::TypeId type_id) -> llvm::Type* {
     // InvalidType should not be passed in.
     CARBON_CHECK(type_id.index >= 0, "{0}", type_id);
-    CARBON_CHECK(types_[type_id.index], "Missing type {0}", type_id);
+    CARBON_CHECK(types_[type_id.index], "Missing type {0}: {1}", type_id,
+                 sem_ir().types().GetAsInst(type_id));
     return types_[type_id.index];
   }
 

toolchain/lower/testdata/class/convert.carbon

@@ -0,0 +1,81 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/class/convert.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/class/convert.carbon
+
+class IntWrapper {
+  var n: i32;
+}
+
+impl IntWrapper as Core.ImplicitAs(i32) {
+  fn Convert[self: Self]() -> i32 { return self.n; }
+}
+
+fn Consume(n: i32) {}
+
+fn DoIt() {
+  var w: IntWrapper = {.n = 42};
+  Consume(w);
+}
+
+// CHECK:STDOUT: ; ModuleID = 'convert.carbon'
+// CHECK:STDOUT: source_filename = "convert.carbon"
+// CHECK:STDOUT:
+// CHECK:STDOUT: @struct.loc22_32 = internal constant { i32 } { i32 42 }
+// CHECK:STDOUT:
+// CHECK:STDOUT: define i32 @"_CConvert.IntWrapper.Main:ImplicitAs.Core"(ptr %self) !dbg !4 {
+// CHECK:STDOUT: entry:
+// CHECK:STDOUT:   %.loc16_48.1.n = getelementptr inbounds nuw { i32 }, ptr %self, i32 0, i32 0, !dbg !7
+// CHECK:STDOUT:   %.loc16_48.2 = load i32, ptr %.loc16_48.1.n, align 4, !dbg !7
+// CHECK:STDOUT:   ret i32 %.loc16_48.2, !dbg !8
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: define void @_CConsume.Main(i32 %n) !dbg !9 {
+// CHECK:STDOUT: entry:
+// CHECK:STDOUT:   ret void, !dbg !10
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: define void @_CDoIt.Main() !dbg !11 {
+// CHECK:STDOUT: entry:
+// CHECK:STDOUT:   %w.var = alloca { i32 }, align 8, !dbg !12
+// CHECK:STDOUT:   %.loc22_31.2.n = getelementptr inbounds nuw { i32 }, ptr %w.var, i32 0, i32 0, !dbg !13
+// CHECK:STDOUT:   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %w.var, ptr align 4 @struct.loc22_32, i64 4, i1 false), !dbg !14
+// CHECK:STDOUT:   %Convert.call = call i32 @"_CConvert.IntWrapper.Main:ImplicitAs.Core"(ptr %w.var), !dbg !15
+// CHECK:STDOUT:   %.loc23_11.6.temp = alloca i32, align 4, !dbg !15
+// CHECK:STDOUT:   store i32 %Convert.call, ptr %.loc23_11.6.temp, align 4, !dbg !15
+// CHECK:STDOUT:   %.loc23_11.8 = load i32, ptr %.loc23_11.6.temp, align 4, !dbg !15
+// CHECK:STDOUT:   call void @_CConsume.Main(i32 %.loc23_11.8), !dbg !16
+// CHECK:STDOUT:   ret void, !dbg !17
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: ; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: readwrite)
+// CHECK:STDOUT: declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg) #0
+// CHECK:STDOUT:
+// CHECK:STDOUT: attributes #0 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
+// CHECK:STDOUT:
+// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
+// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
+// CHECK:STDOUT:
+// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
+// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
+// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+// CHECK:STDOUT: !3 = !DIFile(filename: "convert.carbon", directory: "")
+// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "Convert", linkageName: "_CConvert.IntWrapper.Main:ImplicitAs.Core", scope: null, file: !3, line: 16, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
+// CHECK:STDOUT: !6 = !{}
+// CHECK:STDOUT: !7 = !DILocation(line: 16, column: 44, scope: !4)
+// CHECK:STDOUT: !8 = !DILocation(line: 16, column: 37, scope: !4)
+// CHECK:STDOUT: !9 = distinct !DISubprogram(name: "Consume", linkageName: "_CConsume.Main", scope: null, file: !3, line: 19, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !10 = !DILocation(line: 19, column: 1, scope: !9)
+// CHECK:STDOUT: !11 = distinct !DISubprogram(name: "DoIt", linkageName: "_CDoIt.Main", scope: null, file: !3, line: 21, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !12 = !DILocation(line: 22, column: 7, scope: !11)
+// CHECK:STDOUT: !13 = !DILocation(line: 22, column: 23, scope: !11)
+// CHECK:STDOUT: !14 = !DILocation(line: 22, column: 3, scope: !11)
+// CHECK:STDOUT: !15 = !DILocation(line: 23, column: 11, scope: !11)
+// CHECK:STDOUT: !16 = !DILocation(line: 23, column: 3, scope: !11)
+// CHECK:STDOUT: !17 = !DILocation(line: 21, column: 1, scope: !11)