diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 770616fc6682..bb33e03d052e 100644 --- a/toolchain/check/convert.cpp +++ b/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()) { diff --git a/toolchain/check/testdata/const/basics.carbon b/toolchain/check/testdata/const/basics.carbon index f5c87d2fb1ad..4e9d8e7c5147 100644 --- a/toolchain/check/testdata/const/basics.carbon +++ b/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 { diff --git a/toolchain/check/testdata/interop/cpp/class/method.carbon b/toolchain/check/testdata/interop/cpp/class/method.carbon index 5cf4e093fae0..a1de9f5300b1 100644 --- a/toolchain/check/testdata/interop/cpp/class/method.carbon +++ b/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(); }