From 79521c9efeaa80312c90a570b448dc2f95854dcc Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 17 Oct 2023 05:39:25 +0800 Subject: [PATCH] Fix `Box(T?)` crashing on `nil` --- spec/std/box_spec.cr | 39 +++++++++++++++++++++++++++++++++++++++ src/box.cr | 29 ++++++++++++++++++++--------- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/spec/std/box_spec.cr b/spec/std/box_spec.cr index 8b889a2cdf53..b592b20ca9a6 100644 --- a/spec/std/box_spec.cr +++ b/spec/std/box_spec.cr @@ -15,10 +15,49 @@ describe "Box" do Box(String).unbox(box).should be(a) end + it "boxing a nilable reference returns the same pointer" do + a = "foo".as(String?) + box = Box.box(a) + box.address.should eq(a.object_id) + + b = Box(String?).unbox(box) + b.should be_a(String) + b.should be(a) + end + + it "boxing a nilable value returns the same value" do + a = 1.as(Int32?) + box = Box.box(a) + + b = Box(Int32?).unbox(box) + b.should be_a(Int32) + b.should eq(a) + end + + it "boxes with explicit type" do + box = Box(Int32?).box(1) + b = Box(Int32?).unbox(box) + b.should be_a(Int32) + b.should eq(1) + end + it "boxing nil returns a null pointer" do box = Box.box(nil) box.address.should eq(0) Box(Nil).unbox(box).should be_nil end + + it "boxing nil in a reference-like union returns a null pointer (#11839)" do + box = Box.box(nil.as(String?)) + box.address.should eq(0) + + Box(String?).unbox(box).should be_nil + end + + it "boxing nil in a value-like union doesn't crash (#11839)" do + box = Box.box(nil.as(Int32?)) + + Box(Int32?).unbox(box).should be_nil + end end diff --git a/src/box.cr b/src/box.cr index 652292efd080..02f64dc10b38 100644 --- a/src/box.cr +++ b/src/box.cr @@ -14,20 +14,31 @@ class Box(T) def initialize(@object : T) end - # Creates a Box for a reference type (or `nil`) and returns the same pointer (or `NULL`) - def self.box(r : Reference?) : Void* - r.as(Void*) - end - - # Creates a Box for an object and returns it as a `Void*`. - def self.box(object) : Void* - new(object).as(Void*) + # Turns *object* into a `Void*`. + # + # If `T` is not a reference type, nor a union between reference types and + # `Nil`, this method effectively copies *object* to the dynamic heap. + # + # NOTE: The returned pointer might not be a null pointer even when *object* is + # `nil`. + def self.box(object : T) : Void* + {% if T.union_types.all? { |t| t == Nil || t < Reference } %} + object.as(Void*) + {% else %} + # NOTE: if `T` is explicitly specified and `typeof(object) < T` (e.g. + # `Box(Int32?).box(1)`, then `.new` will perform the appropriate upcast + new(object).as(Void*) + {% end %} end # Unboxes a `Void*` into an object of type `T`. Note that for this you must # specify T: `Box(T).unbox(data)`. + # + # WARNING: It is undefined behavior to box an object in one type and unbox it + # via a different type; in particular, when boxing a `T` as a `T?`, or + # vice-versa. def self.unbox(pointer : Void*) : T - {% if T <= Reference || T == Nil %} + {% if T.union_types.all? { |t| t == Nil || t < Reference } %} pointer.as(T) {% else %} pointer.as(self).object