Skip to content

Commit f943f31

Browse files
zygoloidzygoloid
authored
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.
1 parent 51cb078 commit f943f31

File tree

2 files changed

+72
-54
lines changed

2 files changed

+72
-54
lines changed

‎toolchain/check/convert.cpp‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -808,12 +808,9 @@ static auto CanRemoveQualifiers(SemIR::TypeQualifiers quals,
808808
}
809809

810810
if (HasTypeQualifier(quals, SemIR::TypeQualifiers::MaybeUnformed) &&
811-
(!allow_unsafe || !SemIR::IsRefCategory(cat))) {
812-
// As an unsafe conversion, `MaybeUnformed` can be removed from a reference
813-
// expression.
814-
// TODO: We should allow this for any kind of expression, and convert the
815-
// result as needed if the representation of `T` differs from that of
816-
// `MaybeUnformed(T)`.
811+
(!allow_unsafe || cat == SemIR::ExprCategory::Initializing)) {
812+
// As an unsafe conversion, `MaybeUnformed` can be removed from a value or
813+
// reference expression.
817814
return false;
818815
}
819816

@@ -976,9 +973,11 @@ static auto PerformBuiltinConversion(
976973
context.types().GetTransitiveUnqualifiedAdaptedType(value_type_id);
977974
if (target_foundation_id == value_foundation_id) {
978975
auto category = SemIR::GetExprCategory(context.sem_ir(), value_id);
979-
if (CanAddQualifiers(target_quals & ~value_quals, category) &&
976+
auto added_quals = target_quals & ~value_quals;
977+
auto removed_quals = value_quals & ~target_quals;
978+
if (CanAddQualifiers(added_quals, category) &&
980979
CanRemoveQualifiers(
981-
value_quals & ~target_quals, category,
980+
removed_quals, category,
982981
target.kind == ConversionTarget::ExplicitUnsafeAs)) {
983982
// For a struct or tuple literal, perform a category conversion if
984983
// necessary.
@@ -989,9 +988,32 @@ static auto PerformBuiltinConversion(
989988
.diagnose = target.diagnose});
990989
}
991990

992-
return AddInst<SemIR::AsCompatible>(
991+
// `MaybeUnformed(T)` has a pointer value representation, and `T` might
992+
// not, so convert as needed when removing `MaybeUnformed`.
993+
bool need_value_binding = false;
994+
if ((removed_quals & SemIR::TypeQualifiers::MaybeUnformed) !=
995+
SemIR::TypeQualifiers::None &&
996+
category == SemIR::ExprCategory::Value) {
997+
value_id = AddInst<SemIR::ValueAsRef>(
998+
context, loc_id,
999+
{.type_id = value_type_id, .value_id = value_id});
1000+
need_value_binding = true;
1001+
}
1002+
1003+
value_id = AddInst<SemIR::AsCompatible>(
9931004
context, loc_id,
9941005
{.type_id = target.type_id, .source_id = value_id});
1006+
1007+
if (need_value_binding) {
1008+
value_id = AddInst<SemIR::BindValue>(
1009+
context, loc_id,
1010+
{.type_id = target.type_id, .value_id = value_id});
1011+
}
1012+
return value_id;
1013+
} else {
1014+
// TODO: Produce a custom diagnostic explaining that we can't perform
1015+
// this conversion due to the change in qualifiers and/or the expression
1016+
// category.
9951017
}
9961018
}
9971019
}

‎toolchain/check/testdata/as/maybe_unformed.carbon‎

Lines changed: 41 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ fn Use() {
7878
// CHECK:STDERR: ^~~~~~~~~~~
7979
// CHECK:STDERR:
8080
var i: X = Init() as X;
81+
// TODO: The diagnostic should explain that the reason we can't perform this
82+
// conversion is due to the expression category.
83+
// 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]
84+
// CHECK:STDERR: var j: X = Init() unsafe as X;
85+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
86+
// CHECK:STDERR: fail_cannot_remove_unformed.carbon:[[@LINE+4]]:14: note: type `Core.MaybeUnformed(X)` does not implement interface `Core.UnsafeAs(X)` [MissingImplInMemberAccessNote]
87+
// CHECK:STDERR: var j: X = Init() unsafe as X;
88+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
89+
// CHECK:STDERR:
90+
var j: X = Init() unsafe as X;
8191
// CHECK:STDERR: fail_cannot_remove_unformed.carbon:[[@LINE+7]]:14: error: cannot convert expression of type `Core.MaybeUnformed(X)` to `X` with `as` [ConversionFailure]
8292
// CHECK:STDERR: let v: X = value as X;
8393
// CHECK:STDERR: ^~~~~~~~~~
@@ -110,45 +120,19 @@ library "[[@TEST_NAME]]";
110120

111121
class X {}
112122

123+
fn Init() -> Core.MaybeUnformed(X);
124+
let value: Core.MaybeUnformed(X) = Init();
113125
var reference: Core.MaybeUnformed(X);
114126
let ptr: Core.MaybeUnformed(X)* = &reference;
115127

116128
fn Use() {
117129
//@dump-sem-ir-begin
130+
let v: X = value unsafe as X;
118131
let a: X* = &(reference unsafe as X);
119132
let b: X* = ptr unsafe as X*;
120133
//@dump-sem-ir-end
121134
}
122135

123-
// --- fail_todo_remove_unformed_unsafe_notref.carbon
124-
125-
library "[[@TEST_NAME]]";
126-
127-
class X {}
128-
129-
fn Init() -> Core.MaybeUnformed(X);
130-
let value: Core.MaybeUnformed(X) = Init();
131-
132-
fn Use() {
133-
// TODO: These should probably be valid.
134-
// 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]
135-
// CHECK:STDERR: var i: X = Init() unsafe as X;
136-
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
137-
// 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]
138-
// CHECK:STDERR: var i: X = Init() unsafe as X;
139-
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
140-
// CHECK:STDERR:
141-
var i: X = Init() unsafe as X;
142-
// 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]
143-
// CHECK:STDERR: let v: X = value unsafe as X;
144-
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~
145-
// 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]
146-
// CHECK:STDERR: let v: X = value unsafe as X;
147-
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~
148-
// CHECK:STDERR:
149-
let v: X = value unsafe as X;
150-
}
151-
152136
// CHECK:STDOUT: --- add_unformed.carbon
153137
// CHECK:STDOUT:
154138
// CHECK:STDOUT: constants {
@@ -321,6 +305,7 @@ fn Use() {
321305
// CHECK:STDOUT: %pattern_type.1c6: type = pattern_type %ptr.d17 [concrete]
322306
// CHECK:STDOUT: %MaybeUnformed.275: type = class_type @MaybeUnformed, @MaybeUnformed(%X) [concrete]
323307
// CHECK:STDOUT: %ptr.58e: type = ptr_type %MaybeUnformed.275 [concrete]
308+
// CHECK:STDOUT: %pattern_type.019: type = pattern_type %X [concrete]
324309
// CHECK:STDOUT: %reference.var: ref %X = var file.%reference.var_patt [concrete]
325310
// CHECK:STDOUT: %addr.a46: %ptr.d17 = addr_of %reference.var [concrete]
326311
// CHECK:STDOUT: }
@@ -331,31 +316,42 @@ fn Use() {
331316
// CHECK:STDOUT: fn @Use() {
332317
// CHECK:STDOUT: !entry:
333318
// CHECK:STDOUT: name_binding_decl {
319+
// CHECK:STDOUT: %v.patt: %pattern_type.019 = binding_pattern v [concrete]
320+
// CHECK:STDOUT: }
321+
// CHECK:STDOUT: %value.ref: %MaybeUnformed.275 = name_ref value, file.%value
322+
// CHECK:STDOUT: %X.ref.loc13_30: type = name_ref X, file.%X.decl [concrete = constants.%X]
323+
// CHECK:STDOUT: %.loc13_27.1: ref %MaybeUnformed.275 = value_as_ref %value.ref
324+
// CHECK:STDOUT: %.loc13_27.2: ref %X = as_compatible %.loc13_27.1
325+
// CHECK:STDOUT: %.loc13_27.3: %X = bind_value %.loc13_27.2
326+
// CHECK:STDOUT: %.loc13_27.4: %X = converted %value.ref, %.loc13_27.3
327+
// CHECK:STDOUT: %X.ref.loc13_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
328+
// CHECK:STDOUT: %v: %X = bind_name v, %.loc13_27.4
329+
// CHECK:STDOUT: name_binding_decl {
334330
// CHECK:STDOUT: %a.patt: %pattern_type.1c6 = binding_pattern a [concrete]
335331
// CHECK:STDOUT: }
336332
// CHECK:STDOUT: %reference.ref: ref %MaybeUnformed.275 = name_ref reference, file.%reference [concrete = file.%reference.var]
337-
// CHECK:STDOUT: %X.ref.loc11_37: type = name_ref X, file.%X.decl [concrete = constants.%X]
338-
// CHECK:STDOUT: %.loc11_34.1: ref %X = as_compatible %reference.ref [concrete = constants.%reference.var]
339-
// CHECK:STDOUT: %.loc11_34.2: ref %X = converted %reference.ref, %.loc11_34.1 [concrete = constants.%reference.var]
340-
// CHECK:STDOUT: %addr: %ptr.d17 = addr_of %.loc11_34.2 [concrete = constants.%addr.a46]
341-
// CHECK:STDOUT: %.loc11_11: type = splice_block %ptr.loc11 [concrete = constants.%ptr.d17] {
342-
// CHECK:STDOUT: %X.ref.loc11_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
343-
// CHECK:STDOUT: %ptr.loc11: type = ptr_type %X.ref.loc11_10 [concrete = constants.%ptr.d17]
333+
// CHECK:STDOUT: %X.ref.loc14_37: type = name_ref X, file.%X.decl [concrete = constants.%X]
334+
// CHECK:STDOUT: %.loc14_34.1: ref %X = as_compatible %reference.ref [concrete = constants.%reference.var]
335+
// CHECK:STDOUT: %.loc14_34.2: ref %X = converted %reference.ref, %.loc14_34.1 [concrete = constants.%reference.var]
336+
// CHECK:STDOUT: %addr: %ptr.d17 = addr_of %.loc14_34.2 [concrete = constants.%addr.a46]
337+
// CHECK:STDOUT: %.loc14_11: type = splice_block %ptr.loc14 [concrete = constants.%ptr.d17] {
338+
// CHECK:STDOUT: %X.ref.loc14_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
339+
// CHECK:STDOUT: %ptr.loc14: type = ptr_type %X.ref.loc14_10 [concrete = constants.%ptr.d17]
344340
// CHECK:STDOUT: }
345341
// CHECK:STDOUT: %a: %ptr.d17 = bind_name a, %addr
346342
// CHECK:STDOUT: name_binding_decl {
347343
// CHECK:STDOUT: %b.patt: %pattern_type.1c6 = binding_pattern b [concrete]
348344
// CHECK:STDOUT: }
349-
// CHECK:STDOUT: %ptr.ref: %ptr.58e = name_ref ptr, file.%ptr.loc7_5
350-
// CHECK:STDOUT: %X.ref.loc12_29: type = name_ref X, file.%X.decl [concrete = constants.%X]
351-
// CHECK:STDOUT: %ptr.loc12_30: type = ptr_type %X.ref.loc12_29 [concrete = constants.%ptr.d17]
352-
// CHECK:STDOUT: %.loc12_26.1: %ptr.d17 = as_compatible %ptr.ref
353-
// CHECK:STDOUT: %.loc12_26.2: %ptr.d17 = converted %ptr.ref, %.loc12_26.1
354-
// CHECK:STDOUT: %.loc12_11: type = splice_block %ptr.loc12_11 [concrete = constants.%ptr.d17] {
355-
// CHECK:STDOUT: %X.ref.loc12_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
356-
// CHECK:STDOUT: %ptr.loc12_11: type = ptr_type %X.ref.loc12_10 [concrete = constants.%ptr.d17]
345+
// CHECK:STDOUT: %ptr.ref: %ptr.58e = name_ref ptr, file.%ptr.loc9_5
346+
// CHECK:STDOUT: %X.ref.loc15_29: type = name_ref X, file.%X.decl [concrete = constants.%X]
347+
// CHECK:STDOUT: %ptr.loc15_30: type = ptr_type %X.ref.loc15_29 [concrete = constants.%ptr.d17]
348+
// CHECK:STDOUT: %.loc15_26.1: %ptr.d17 = as_compatible %ptr.ref
349+
// CHECK:STDOUT: %.loc15_26.2: %ptr.d17 = converted %ptr.ref, %.loc15_26.1
350+
// CHECK:STDOUT: %.loc15_11: type = splice_block %ptr.loc15_11 [concrete = constants.%ptr.d17] {
351+
// CHECK:STDOUT: %X.ref.loc15_10: type = name_ref X, file.%X.decl [concrete = constants.%X]
352+
// CHECK:STDOUT: %ptr.loc15_11: type = ptr_type %X.ref.loc15_10 [concrete = constants.%ptr.d17]
357353
// CHECK:STDOUT: }
358-
// CHECK:STDOUT: %b: %ptr.d17 = bind_name b, %.loc12_26.2
354+
// CHECK:STDOUT: %b: %ptr.d17 = bind_name b, %.loc15_26.2
359355
// CHECK:STDOUT: <elided>
360356
// CHECK:STDOUT: }
361357
// CHECK:STDOUT:

0 commit comments

Comments
 (0)