search

JuliaStrings / InlineStrings.jl

Fixed-width string types for Julia
Other
45 stars 13 forks source link

Widen `Missing` eltype on first non-`missing` #35

Closed nickrobinson251 closed 2 years ago

nickrobinson251 commented 2 years ago

Before inlinestrings([missing, "e"]) would fail, even though inlinestrings(["e", missing]) would succeed as would inlinestrings([missing, "ee"]). This was because in first case the initial results vector eltype eT is set to Missing and then when we see a string y = "e" we'd have y !== missing && sizeof(y) == 1 and hit the branch which calls nonmissingtype(eT)(y). We only hit this branch when sizeof(y) == 1 since the otherside of the check, sizeof(y) < sizeof(eT), is always false (since sizeof(Missing) == 0). (This bug was accidentally introduced by me in #26) The fix is to never hit this branch if eT == Missing.

nickrobinson251 commented 2 years ago

oh i think Jacob is away this week -- i wonder if someone else with permissions is around to review (and make a patch release). Maybe @nalimilan?

nalimilan commented 2 years ago

Thanks. Any particular reason to rush a release before Jacob is back? We can do it but it would be good to double check with him.

nickrobinson251 commented 2 years ago

Thanks for reviewing!

Any particular reason to rush a release before Jacob is back? We can do it but it would be good to double check with him.

No more so than usual. Just for the benefit of @nilshg who hit this bug. Seemed an straight forward enough fix to release on its own.

Thanks

nickrobinson251 commented 2 years ago

thanks for adding me to the repo, Jacob!