Allow a value of type `MaybeUnformed(T)` to convert to `T` with `unsafe as` (#6014)

We already allowed this for reference expressions; this extends the
support to also cover value expressions. This requires a little more
work because the value representation of `T` and `MaybeUnformed(T)`
don't necessarily match in general.

toolchain/check/convert.cpp

@@ -808,12 +808,9 @@ static auto CanRemoveQualifiers(SemIR::TypeQualifiers quals,
   }
 
   if (HasTypeQualifier(quals, SemIR::TypeQualifiers::MaybeUnformed) &&
-      (!allow_unsafe || !SemIR::IsRefCategory(cat))) {
-    // As an unsafe conversion, `MaybeUnformed` can be removed from a reference
-    // expression.
-    // TODO: We should allow this for any kind of expression, and convert the
-    // result as needed if the representation of `T` differs from that of
-    // `MaybeUnformed(T)`.
+      (!allow_unsafe || cat == SemIR::ExprCategory::Initializing)) {
+    // As an unsafe conversion, `MaybeUnformed` can be removed from a value or
+    // reference expression.
     return false;
   }
 
@@ -976,9 +973,11 @@ static auto PerformBuiltinConversion(
         context.types().GetTransitiveUnqualifiedAdaptedType(value_type_id);
     if (target_foundation_id == value_foundation_id) {
       auto category = SemIR::GetExprCategory(context.sem_ir(), value_id);
-      if (CanAddQualifiers(target_quals & ~value_quals, category) &&
+      auto added_quals = target_quals & ~value_quals;
+      auto removed_quals = value_quals & ~target_quals;
+      if (CanAddQualifiers(added_quals, category) &&
           CanRemoveQualifiers(
-              value_quals & ~target_quals, category,
+              removed_quals, category,
               target.kind == ConversionTarget::ExplicitUnsafeAs)) {
         // For a struct or tuple literal, perform a category conversion if
         // necessary.
@@ -989,9 +988,32 @@ static auto PerformBuiltinConversion(
                                                .diagnose = target.diagnose});
         }
 
-        return AddInst<SemIR::AsCompatible>(
+        // `MaybeUnformed(T)` has a pointer value representation, and `T` might
+        // not, so convert as needed when removing `MaybeUnformed`.
+        bool need_value_binding = false;
+        if ((removed_quals & SemIR::TypeQualifiers::MaybeUnformed) !=
+                SemIR::TypeQualifiers::None &&
+            category == SemIR::ExprCategory::Value) {
+          value_id = AddInst<SemIR::ValueAsRef>(
+              context, loc_id,
+              {.type_id = value_type_id, .value_id = value_id});
+          need_value_binding = true;
+        }
+
+        value_id = AddInst<SemIR::AsCompatible>(
             context, loc_id,
             {.type_id = target.type_id, .source_id = value_id});
+
+        if (need_value_binding) {
+          value_id = AddInst<SemIR::BindValue>(
+              context, loc_id,
+              {.type_id = target.type_id, .value_id = value_id});
+        }
+        return value_id;
+      } else {
+        // TODO: Produce a custom diagnostic explaining that we can't perform
+        // this conversion due to the change in qualifiers and/or the expression
+        // category.
       }
     }
   }

toolchain/check/testdata/as/maybe_unformed.carbon

@@ -78,6 +78,16 @@ fn Use() {
   // CHECK:STDERR:              ^~~~~~~~~~~
   // CHECK:STDERR:
   var i: X = Init() as X;
+  // TODO: The diagnostic should explain that the reason we can't perform this
+  // conversion is due to the expression category.
+  // CHECK:STDERR: fail_cannot_remove_unformed.carbon:[[@LINE+7]]:14: error: cannot convert expression of type `Core.MaybeUnformed(X)` to `X` with `unsafe as` [ConversionFailure]
+  // CHECK:STDERR:   var j: X = Init() unsafe as X;
+  // CHECK:STDERR:              ^~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR: fail_cannot_remove_unformed.carbon:[[@LINE+4]]:14: note: type `Core.MaybeUnformed(X)` does not implement interface `Core.UnsafeAs(X)` [MissingImplInMemberAccessNote]
+  // CHECK:STDERR:   var j: X = Init() unsafe as X;
+  // CHECK:STDERR:              ^~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  var j: X = Init() unsafe as X;
   // CHECK:STDERR: fail_cannot_remove_unformed.carbon:[[@LINE+7]]:14: error: cannot convert expression of type `Core.MaybeUnformed(X)` to `X` with `as` [ConversionFailure]
   // CHECK:STDERR:   let v: X = value as X;
   // CHECK:STDERR:              ^~~~~~~~~~
@@ -110,45 +120,19 @@ library "[[@TEST_NAME]]";
 
 class X {}
 
+fn Init() -> Core.MaybeUnformed(X);
+let value: Core.MaybeUnformed(X) = Init();
 var reference: Core.MaybeUnformed(X);
 let ptr: Core.MaybeUnformed(X)* = &reference;
 
 fn Use() {
   //@dump-sem-ir-begin
+  let v: X = value unsafe as X;
   let a: X* = &(reference unsafe as X);
   let b: X* = ptr unsafe as X*;
   //@dump-sem-ir-end
 }
 
-// --- fail_todo_remove_unformed_unsafe_notref.carbon
-
-library "[[@TEST_NAME]]";
-
-class X {}
-
-fn Init() -> Core.MaybeUnformed(X);
-let value: Core.MaybeUnformed(X) = Init();
-
-fn Use() {
-  // TODO: These should probably be valid.
-  // CHECK:STDERR: fail_todo_remove_unformed_unsafe_notref.carbon:[[@LINE+7]]:14: error: cannot convert expression of type `Core.MaybeUnformed(X)` to `X` with `unsafe as` [ConversionFailure]
-  // CHECK:STDERR:   var i: X = Init() unsafe as X;
-  // CHECK:STDERR:              ^~~~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_todo_remove_unformed_unsafe_notref.carbon:[[@LINE+4]]:14: note: type `Core.MaybeUnformed(X)` does not implement interface `Core.UnsafeAs(X)` [MissingImplInMemberAccessNote]
-  // CHECK:STDERR:   var i: X = Init() unsafe as X;
-  // CHECK:STDERR:              ^~~~~~~~~~~~~~~~~~
-  // CHECK:STDERR:
-  var i: X = Init() unsafe as X;
-  // CHECK:STDERR: fail_todo_remove_unformed_unsafe_notref.carbon:[[@LINE+7]]:14: error: cannot convert expression of type `Core.MaybeUnformed(X)` to `X` with `unsafe as` [ConversionFailure]
-  // CHECK:STDERR:   let v: X = value unsafe as X;
-  // CHECK:STDERR:              ^~~~~~~~~~~~~~~~~
-  // CHECK:STDERR: fail_todo_remove_unformed_unsafe_notref.carbon:[[@LINE+4]]:14: note: type `Core.MaybeUnformed(X)` does not implement interface `Core.UnsafeAs(X)` [MissingImplInMemberAccessNote]
-  // CHECK:STDERR:   let v: X = value unsafe as X;
-  // CHECK:STDERR:              ^~~~~~~~~~~~~~~~~
-  // CHECK:STDERR:
-  let v: X = value unsafe as X;
-}
-
 // CHECK:STDOUT: --- add_unformed.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -321,6 +305,7 @@ fn Use() {
 // CHECK:STDOUT:   %pattern_type.1c6: type = pattern_type %ptr.d17 [concrete]
 // CHECK:STDOUT:   %MaybeUnformed.275: type = class_type @MaybeUnformed, @MaybeUnformed(%X) [concrete]
 // CHECK:STDOUT:   %ptr.58e: type = ptr_type %MaybeUnformed.275 [concrete]
+// CHECK:STDOUT:   %pattern_type.019: type = pattern_type %X [concrete]
 // CHECK:STDOUT:   %reference.var: ref %X = var file.%reference.var_patt [concrete]
 // CHECK:STDOUT:   %addr.a46: %ptr.d17 = addr_of %reference.var [concrete]
 // CHECK:STDOUT: }
@@ -331,31 +316,42 @@ fn Use() {
 // CHECK:STDOUT: fn @Use() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   name_binding_decl {
+// CHECK:STDOUT:     %v.patt: %pattern_type.019 = binding_pattern v [concrete]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %value.ref: %MaybeUnformed.275 = name_ref value, file.%value
+// CHECK:STDOUT:   %X.ref.loc13_30: type = name_ref X, file.%X.decl [concrete = constants.%X]
+// CHECK:STDOUT:   %.loc13_27.1: ref %MaybeUnformed.275 = value_as_ref %value.ref
+// CHECK:STDOUT:   %.loc13_27.2: ref %X = as_compatible %.loc13_27.1
+// CHECK:STDOUT:   %.loc13_27.3: %X = bind_value %.loc13_27.2
+// CHECK:STDOUT:   %.loc13_27.4: %X = converted %value.ref, %.loc13_27.3
+// CHECK:STDOUT:   %X.ref.loc13_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
+// CHECK:STDOUT:   %v: %X = bind_name v, %.loc13_27.4
+// CHECK:STDOUT:   name_binding_decl {
 // CHECK:STDOUT:     %a.patt: %pattern_type.1c6 = binding_pattern a [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %reference.ref: ref %MaybeUnformed.275 = name_ref reference, file.%reference [concrete = file.%reference.var]
-// CHECK:STDOUT:   %X.ref.loc11_37: type = name_ref X, file.%X.decl [concrete = constants.%X]
-// CHECK:STDOUT:   %.loc11_34.1: ref %X = as_compatible %reference.ref [concrete = constants.%reference.var]
-// CHECK:STDOUT:   %.loc11_34.2: ref %X = converted %reference.ref, %.loc11_34.1 [concrete = constants.%reference.var]
-// CHECK:STDOUT:   %addr: %ptr.d17 = addr_of %.loc11_34.2 [concrete = constants.%addr.a46]
-// CHECK:STDOUT:   %.loc11_11: type = splice_block %ptr.loc11 [concrete = constants.%ptr.d17] {
-// CHECK:STDOUT:     %X.ref.loc11_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
-// CHECK:STDOUT:     %ptr.loc11: type = ptr_type %X.ref.loc11_10 [concrete = constants.%ptr.d17]
+// CHECK:STDOUT:   %X.ref.loc14_37: type = name_ref X, file.%X.decl [concrete = constants.%X]
+// CHECK:STDOUT:   %.loc14_34.1: ref %X = as_compatible %reference.ref [concrete = constants.%reference.var]
+// CHECK:STDOUT:   %.loc14_34.2: ref %X = converted %reference.ref, %.loc14_34.1 [concrete = constants.%reference.var]
+// CHECK:STDOUT:   %addr: %ptr.d17 = addr_of %.loc14_34.2 [concrete = constants.%addr.a46]
+// CHECK:STDOUT:   %.loc14_11: type = splice_block %ptr.loc14 [concrete = constants.%ptr.d17] {
+// CHECK:STDOUT:     %X.ref.loc14_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
+// CHECK:STDOUT:     %ptr.loc14: type = ptr_type %X.ref.loc14_10 [concrete = constants.%ptr.d17]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %a: %ptr.d17 = bind_name a, %addr
 // CHECK:STDOUT:   name_binding_decl {
 // CHECK:STDOUT:     %b.patt: %pattern_type.1c6 = binding_pattern b [concrete]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %ptr.ref: %ptr.58e = name_ref ptr, file.%ptr.loc7_5
-// CHECK:STDOUT:   %X.ref.loc12_29: type = name_ref X, file.%X.decl [concrete = constants.%X]
-// CHECK:STDOUT:   %ptr.loc12_30: type = ptr_type %X.ref.loc12_29 [concrete = constants.%ptr.d17]
-// CHECK:STDOUT:   %.loc12_26.1: %ptr.d17 = as_compatible %ptr.ref
-// CHECK:STDOUT:   %.loc12_26.2: %ptr.d17 = converted %ptr.ref, %.loc12_26.1
-// CHECK:STDOUT:   %.loc12_11: type = splice_block %ptr.loc12_11 [concrete = constants.%ptr.d17] {
-// CHECK:STDOUT:     %X.ref.loc12_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
-// CHECK:STDOUT:     %ptr.loc12_11: type = ptr_type %X.ref.loc12_10 [concrete = constants.%ptr.d17]
+// CHECK:STDOUT:   %ptr.ref: %ptr.58e = name_ref ptr, file.%ptr.loc9_5
+// CHECK:STDOUT:   %X.ref.loc15_29: type = name_ref X, file.%X.decl [concrete = constants.%X]
+// CHECK:STDOUT:   %ptr.loc15_30: type = ptr_type %X.ref.loc15_29 [concrete = constants.%ptr.d17]
+// CHECK:STDOUT:   %.loc15_26.1: %ptr.d17 = as_compatible %ptr.ref
+// CHECK:STDOUT:   %.loc15_26.2: %ptr.d17 = converted %ptr.ref, %.loc15_26.1
+// CHECK:STDOUT:   %.loc15_11: type = splice_block %ptr.loc15_11 [concrete = constants.%ptr.d17] {
+// CHECK:STDOUT:     %X.ref.loc15_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
+// CHECK:STDOUT:     %ptr.loc15_11: type = ptr_type %X.ref.loc15_10 [concrete = constants.%ptr.d17]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %b: %ptr.d17 = bind_name b, %.loc12_26.2
+// CHECK:STDOUT:   %b: %ptr.d17 = bind_name b, %.loc15_26.2
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT: