Segfault of is.sorted on list/vector containing only NA_character_

[ben-schwen]

Closes #5070

[codecov]

Codecov Report

Merging #5170 (373eb5b) into master (b82eb68) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5170   +/-   ##
=======================================
  Coverage   99.38%   99.38%           
=======================================
  Files          77       77           
  Lines       14506    14507    +1     
=======================================
+ Hits        14417    14418    +1     
  Misses         89       89           

Impacted Files	Coverage Δ
src/forder.c	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b82eb68...373eb5b. Read the comment docs.

[MichaelChirico]

awesome!

[mattdowle]

Yes: great, thanks!
is.sorted isn't exported though so another tweak to the news item pending ...

[mattdowle]

And the original test reported with merge should be tested too in case that example doesn't use is.sorted in future. Will do. We try and test from the bottom up and from the top down too: as many ways of testing -- just add them all. Then they all get covered by valgrind and ASAN in CRAN_Release.cmd procedures before release.

[mattdowle]

Thanks for adding the merge test from the issue! I was looking at it too and it's a little odd isn't it, the crash actually happens in the [,.(x1,x2)] part when that calls is.sorted, not in the merge. It's just that the merge creates the result which is keyed by the all-NA_character_ column. Anyway, all I focus on is trying to understand and follow the tests myself, and I run the tests in last released version to make sure they do segfault there (which these do). I wasn't sure why you used copy() though, and I looked at the last key test and wondered about that. I ended up shortening the test block, reducing the number of objects in play so I could compare it to the example in the issue, and included checking the key rather than removing the key from both sides.