Allow implicit conversion to a value expression to remove const. (#6253)

`const` doesn't mean much on the type of a value expression; it's valid
to remove it because we can't perform modifications to a const value
regardless.

We already allowed most of this, but only as part of adapter conversion
rather than in general, and we didn't previously allow it when the
source of the conversion was a reference expression.

---------

Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>

toolchain/check/convert.cpp

@@ -799,11 +799,16 @@ static auto CanAddQualifiers(SemIR::TypeQualifiers quals,
 // Determine whether the given set of qualifiers can be removed by a conversion
 // of an expression of the given category.
 static auto CanRemoveQualifiers(SemIR::TypeQualifiers quals,
-                                SemIR::ExprCategory cat, bool allow_unsafe)
-    -> bool {
+                                SemIR::ExprCategory cat,
+                                ConversionTarget::Kind kind) -> bool {
+  bool allow_unsafe = kind == ConversionTarget::ExplicitUnsafeAs;
+
   if (quals.HasAnyOf(SemIR::TypeQualifiers::Const) && !allow_unsafe &&
-      SemIR::IsRefCategory(cat)) {
-    // Removing `const` is an unsafe conversion for a reference expression.
+      SemIR::IsRefCategory(cat) &&
+      IsValidExprCategoryForConversionTarget(cat, kind)) {
+    // Removing `const` is an unsafe conversion for a reference expression. But
+    // it's OK if we will be converting to a different category as part of this
+    // overall conversion anyway.
     return false;
   }
 
@@ -964,21 +969,26 @@ static auto PerformBuiltinConversion(
     }
   }
 
-  // T explicitly converts to U if T is compatible with U, and we're allowed to
+  // T implicitly converts to U if T and U are the same ignoring qualifiers, and
+  // we're allowed to remove / add any qualifiers that differ. Similarly, T
+  // explicitly converts to U if T is compatible with U, and we're allowed to
   // remove / add any qualifiers that differ.
-  if (target.is_explicit_as() && target.type_id != value_type_id) {
+  if (target.type_id != value_type_id) {
     auto [target_foundation_id, target_quals] =
-        context.types().GetTransitiveUnqualifiedAdaptedType(target.type_id);
+        target.is_explicit_as()
+            ? context.types().GetTransitiveUnqualifiedAdaptedType(
+                  target.type_id)
+            : context.types().GetUnqualifiedTypeAndQualifiers(target.type_id);
     auto [value_foundation_id, value_quals] =
-        context.types().GetTransitiveUnqualifiedAdaptedType(value_type_id);
+        target.is_explicit_as()
+            ? context.types().GetTransitiveUnqualifiedAdaptedType(value_type_id)
+            : context.types().GetUnqualifiedTypeAndQualifiers(value_type_id);
     if (target_foundation_id == value_foundation_id) {
       auto category = SemIR::GetExprCategory(context.sem_ir(), value_id);
       auto added_quals = target_quals & ~value_quals;
       auto removed_quals = value_quals & ~target_quals;
       if (CanAddQualifiers(added_quals, category) &&
-          CanRemoveQualifiers(
-              removed_quals, category,
-              target.kind == ConversionTarget::ExplicitUnsafeAs)) {
+          CanRemoveQualifiers(removed_quals, category, target.kind)) {
         // For a struct or tuple literal, perform a category conversion if
         // necessary.
         if (category == SemIR::ExprCategory::Mixed) {
@@ -1072,6 +1082,9 @@ static auto PerformBuiltinConversion(
     }
 
     // An expression of type T converts to U if T is a class derived from U.
+    //
+    // TODO: Combine this with the qualifiers and adapter conversion logic above
+    // to allow qualifiers and inheritance conversions to be performed together.
     if (auto path = ComputeInheritancePath(context, loc_id, value_type_id,
                                            target.type_id);
         path && !path->empty()) {

toolchain/check/testdata/const/basics.carbon

@@ -61,6 +61,101 @@ fn G(p: const (const C)**) -> C** {
   return p;
 }
 
+// --- add_or_remove_while_forming_value.carbon
+library "[[@TEST_NAME]]";
+
+class X {}
+
+fn TakeValue(x: X) {}
+fn TakeConstValue(x: const X) {}
+
+fn PassConstValueToValue(a: const X) {
+  TakeValue(a);
+}
+
+fn PassValueToConstValue(a: X) {
+  TakeConstValue(a);
+}
+
+fn PassConstReferenceToValue(p: const X*) {
+  TakeValue(*p);
+}
+
+fn PassReferenceToConstValue(p: X*) {
+  TakeConstValue(*p);
+}
+
+// --- fail_todo_add_or_remove_while_initializing.carbon
+library "[[@TEST_NAME]]";
+
+class X {}
+
+// TODO: None of these should require `X` to be copyable.
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+7]]:40: error: cannot copy value of type `X` [CopyOfUncopyableType]
+// CHECK:STDERR: var init_non_const_from_non_const: X = {} as X;
+// CHECK:STDERR:                                        ^~~~~~~
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+4]]:40: note: type `X` does not implement interface `Core.Copy` [MissingImplInMemberAccessNote]
+// CHECK:STDERR: var init_non_const_from_non_const: X = {} as X;
+// CHECK:STDERR:                                        ^~~~~~~
+// CHECK:STDERR:
+var init_non_const_from_non_const: X = {} as X;
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+7]]:1: error: cannot copy value of type `X` [CopyOfUncopyableType]
+// CHECK:STDERR: var init_non_const_from_const: X = ({} as X) as const X;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+4]]:1: note: type `X` does not implement interface `Core.Copy` [MissingImplInMemberAccessNote]
+// CHECK:STDERR: var init_non_const_from_const: X = ({} as X) as const X;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+var init_non_const_from_const: X = ({} as X) as const X;
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+7]]:1: error: cannot copy value of type `const X` [CopyOfUncopyableType]
+// CHECK:STDERR: var init_const_from_non_const: const X = {} as X;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+4]]:1: note: type `const X` does not implement interface `Core.Copy` [MissingImplInMemberAccessNote]
+// CHECK:STDERR: var init_const_from_non_const: const X = {} as X;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+var init_const_from_non_const: const X = {} as X;
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+7]]:38: error: cannot copy value of type `const X` [CopyOfUncopyableType]
+// CHECK:STDERR: var init_const_from_const: const X = ({} as X) as const X;
+// CHECK:STDERR:                                      ^~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_todo_add_or_remove_while_initializing.carbon:[[@LINE+4]]:38: note: type `const X` does not implement interface `Core.Copy` [MissingImplInMemberAccessNote]
+// CHECK:STDERR: var init_const_from_const: const X = ({} as X) as const X;
+// CHECK:STDERR:                                      ^~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+var init_const_from_const: const X = ({} as X) as const X;
+
+// --- add_while_forming_reference.carbon
+library "[[@TEST_NAME]]";
+
+class X {
+  fn TakeConstSelf[addr self: const Self*]();
+}
+
+fn PassReferenceToConstReference(p: X*) {
+  p->TakeConstSelf();
+}
+
+// --- fail_remove_while_forming_reference.carbon
+library "[[@TEST_NAME]]";
+
+class X {
+  fn TakeSelf[addr self: Self*]();
+}
+
+fn PassConstReferenceToReference(p: const X*) {
+  // CHECK:STDERR: fail_remove_while_forming_reference.carbon:[[@LINE+10]]:3: error: cannot implicitly convert expression of type `const X*` to `X*` [ConversionFailure]
+  // CHECK:STDERR:   p->(X.TakeSelf)();
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~
+  // CHECK:STDERR: fail_remove_while_forming_reference.carbon:[[@LINE+7]]:3: note: type `const X*` does not implement interface `Core.ImplicitAs(X*)` [MissingImplInMemberAccessNote]
+  // CHECK:STDERR:   p->(X.TakeSelf)();
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~
+  // CHECK:STDERR: fail_remove_while_forming_reference.carbon:[[@LINE-10]]:20: note: initializing function parameter [InCallToFunctionParam]
+  // CHECK:STDERR:   fn TakeSelf[addr self: Self*]();
+  // CHECK:STDERR:                    ^~~~~~~~~~~
+  // CHECK:STDERR:
+  p->(X.TakeSelf)();
+}
+
 // CHECK:STDOUT: --- basic.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {

toolchain/check/testdata/interop/cpp/class/method.carbon

@@ -79,7 +79,7 @@ fn Value(v: Cpp.HasQualifiers) {
   // CHECK:STDERR:       |        ^
   v.ref_ref_this();
 
-  // CHECK:STDERR: fail_bad_object_param_qualifiers_by_value.carbon:[[@LINE+29]]:3: note: in thunk for C++ function used here [InCppThunk]
+  // CHECK:STDERR: fail_bad_object_param_qualifiers_by_value.carbon:[[@LINE+21]]:3: note: in thunk for C++ function used here [InCppThunk]
   // CHECK:STDERR:   v.const_ref_ref_this();
   // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
@@ -100,14 +100,6 @@ fn Value(v: Cpp.HasQualifiers) {
   // CHECK:STDERR:     7 |   void ref_this() &;
   // CHECK:STDERR:       |        ^
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_bad_object_param_qualifiers_by_value.carbon:[[@LINE+8]]:3: error: cannot implicitly convert expression of type `Cpp.HasQualifiers` to `const Cpp.HasQualifiers` [ConversionFailure]
-  // CHECK:STDERR:   v.const_ref_ref_this();
-  // CHECK:STDERR:   ^
-  // CHECK:STDERR: fail_bad_object_param_qualifiers_by_value.carbon:[[@LINE+5]]:3: note: type `Cpp.HasQualifiers` does not implement interface `Core.ImplicitAs(const Cpp.HasQualifiers)` [MissingImplInMemberAccessNote]
-  // CHECK:STDERR:   v.const_ref_ref_this();
-  // CHECK:STDERR:   ^
-  // CHECK:STDERR: fail_bad_object_param_qualifiers_by_value.carbon: note: initializing function parameter [InCallToFunctionParam]
-  // CHECK:STDERR:
   v.const_ref_ref_this();
 }