Skip to content

Commit f0bd124

Browse files
committed
Fix missing implementation for identity comparison of structs.
Prior to this commit, trying to compare two structs for identity equality (`===`/`!==`) would raise an exception at compile time, because this case was not implemented. There are still other cases that are not implemented (such as dealing with the case where one side is a boxed value, and the other side is an unboxed value). But those are left to be implemented at a later time. Particularly, it's expected that we'll soon redesign the way equality works in Savi ( see https://savi.zulipchat.com/#narrow/stream/294897-general/topic/Element-wise.20Array.20comparison.20fails ) so that the compiler will automatically generate a default `==` implementation for all types (which can be overridden by the user). At that time, we could also generate a `===` implementation that cannot be overridden, which would allow us to simplify the code being modified here in this commit, as we could change this call site to a basic method call (for the `===` method) after type comparison, rather than coding the field-wise comparison as LLVM IR as we did here, and sidestepping issues with boxed/unboxed things, as the typical virtual call mechanism will take care of resolving the appropriate unboxed values into method args.
1 parent 507da40 commit f0bd124

File tree

2 files changed

+81
-9
lines changed

2 files changed

+81
-9
lines changed

spec/language/semantics/identity_spec.savi

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
:module IdentitySpec
22
:fun run(test MicroTest)
3+
test["string literal ==="].pass = "example" === "example"
4+
test["string literal !=="].pass = "example" !== "differs"
35
test["identity_digest_of string literal =="].pass =
46
identity_digest_of "example" ==
57
identity_digest_of "example"
@@ -9,6 +11,8 @@
911

1012
class_instance_a = Container(String).new("example")
1113
class_instance_b = Container(String).new("example")
14+
test["class ==="].pass = class_instance_a === class_instance_a
15+
test["class !=="].pass = class_instance_a !== class_instance_b
1216
test["identity_digest_of class !="].pass =
1317
identity_digest_of class_instance_a !=
1418
identity_digest_of class_instance_b
@@ -19,6 +23,8 @@
1923
struct_instance_a = ContainerStruct(String).new("example")
2024
struct_instance_b = ContainerStruct(String).new("example")
2125
struct_instance_c = ContainerStruct(String).new("differs")
26+
test["struct ==="].pass = struct_instance_a === struct_instance_b
27+
test["struct !=="].pass = struct_instance_a !== struct_instance_c
2228
test["identity_digest_of struct =="].pass =
2329
identity_digest_of struct_instance_a ==
2430
identity_digest_of struct_instance_b

src/savi/compiler/code_gen.cr

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2182,11 +2182,25 @@ class Savi::Compiler::CodeGen
21822182
old_value
21832183
end
21842184

2185-
def gen_check_identity_is(relate : AST::Relate, positive_check : Bool)
2186-
lhs_type = type_of(relate.lhs)
2187-
rhs_type = type_of(relate.rhs)
2188-
lhs = gen_expr(relate.lhs)
2189-
rhs = gen_expr(relate.rhs)
2185+
def gen_check_identity_equal(relate : AST::Relate, positive_check : Bool)
2186+
gen_check_identity_equal_inner(
2187+
gen_expr(relate.lhs),
2188+
gen_expr(relate.rhs),
2189+
type_of(relate.lhs),
2190+
type_of(relate.rhs),
2191+
relate.pos,
2192+
positive_check,
2193+
)
2194+
end
2195+
2196+
def gen_check_identity_equal_inner(
2197+
lhs : LLVM::Value,
2198+
rhs : LLVM::Value,
2199+
lhs_type : Reach::Ref,
2200+
rhs_type : Reach::Ref,
2201+
pos : Source::Pos,
2202+
positive_check : Bool
2203+
)
21902204
pred = positive_check ? LLVM::IntPredicate::EQ : LLVM::IntPredicate::NE
21912205

21922206
# If the values are floating point values, they should be compared by their
@@ -2215,7 +2229,59 @@ class Savi::Compiler::CodeGen
22152229
)
22162230
else
22172231
# Integers of different types never have the same identity.
2218-
gen_bool(false)
2232+
gen_bool(!positive_check)
2233+
end
2234+
elsif lhs.type.kind == LLVM::Type::Kind::Struct \
2235+
&& rhs.type.kind == LLVM::Type::Kind::Struct
2236+
if lhs.type == rhs.type
2237+
# Structs of the same type are compared by field-wise comparison.
2238+
# TODO: This will eventually be generated by earlier stages of the
2239+
# compiler which will also be responsible for equality checks with `==`,
2240+
# which will have the same default semantics for structs as `===`,
2241+
# unless the user overrides a custom definition for `==`
2242+
# (but even in that case, we should have a standard definition of `===`)
2243+
phi_blocks = [] of LLVM::BasicBlock
2244+
phi_values = [] of LLVM::Value
2245+
post_block = gen_block("structs.identity.compare")
2246+
success_block = gen_block("structs.identity.compare.success")
2247+
2248+
last_compare = gen_bool(true) # with no fields, the comparison succeeds.
2249+
gtype = gtype_of(lhs_type)
2250+
gtype.fields.each_with_index { |(field_name, field_type), index|
2251+
next_block = gen_block("structs.identity.compare.#{field_name}")
2252+
2253+
phi_values << gen_bool(!positive_check)
2254+
phi_blocks << @builder.insert_block.not_nil!
2255+
@builder.cond(
2256+
last_compare,
2257+
positive_check ? next_block : post_block,
2258+
positive_check ? post_block : next_block
2259+
)
2260+
2261+
@builder.position_at_end(next_block)
2262+
last_compare = gen_check_identity_equal_inner(
2263+
@builder.extract_value(lhs, index, "#{lhs.name}.#{field_name}"),
2264+
@builder.extract_value(lhs, index, "#{rhs.name}.#{field_name}"),
2265+
field_type,
2266+
field_type,
2267+
pos,
2268+
positive_check,
2269+
)
2270+
}
2271+
phi_values << gen_bool(!positive_check)
2272+
phi_blocks << @builder.insert_block.not_nil!
2273+
@builder.cond(last_compare, success_block, post_block)
2274+
2275+
@builder.position_at_end(success_block)
2276+
phi_values << gen_bool(positive_check)
2277+
phi_blocks << @builder.insert_block.not_nil!
2278+
@builder.br(post_block)
2279+
2280+
@builder.position_at_end(post_block)
2281+
@builder.phi(@i1, phi_blocks, phi_values)
2282+
else
2283+
# Structs of different types never have the same identity.
2284+
gen_bool(!positive_check)
22192285
end
22202286
elsif (
22212287
lhs_type == rhs_type || lhs.type == rhs.type || \
@@ -2232,7 +2298,7 @@ class Savi::Compiler::CodeGen
22322298
"#{lhs.name}.is.#{rhs.name}",
22332299
)
22342300
else
2235-
raise NotImplementedError.new("this comparison:\n#{relate.pos.show}")
2301+
raise NotImplementedError.new("this comparison:\n#{pos.show}")
22362302
end
22372303
end
22382304

@@ -2626,8 +2692,8 @@ class Savi::Compiler::CodeGen
26262692
case expr.op.as(AST::Operator).value
26272693
when "=" then gen_eq(expr)
26282694
when "<<=" then gen_displacing_eq(expr)
2629-
when "===" then gen_check_identity_is(expr, true)
2630-
when "!==" then gen_check_identity_is(expr, false)
2695+
when "===" then gen_check_identity_equal(expr, true)
2696+
when "!==" then gen_check_identity_equal(expr, false)
26312697
when "<:" then gen_check_subtype(expr, true)
26322698
when "!<:" then gen_check_subtype(expr, false)
26332699
when "static_address_of_function" then gen_static_address_of_function(expr)

0 commit comments

Comments
 (0)