shift fix type coercion for integer64

[ben-schwen]

Closes #4865

Also changes type coercion for the integer case to switch neatless between integer, double and integer64.

[codecov]

Codecov Report

Merging #5189 (fdc1dc6) into master (cd81808) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5189      +/-   ##
==========================================
- Coverage   99.38%   99.38%   -0.01%     
==========================================
  Files          77       77              
  Lines       14541    14532       -9     
==========================================
- Hits        14452    14443       -9     
  Misses         89       89              

Impacted Files	Coverage Δ
src/utils.c	97.41% <ø> (-0.03%)	⬇️
src/assign.c	99.85% <100.00%> (+<0.01%)	⬆️
src/init.c	100.00% <100.00%> (ø)
src/shift.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 cd81808...fdc1dc6. Read the comment docs.

[MichaelChirico]

NB: coverage decrease is spurious

[mattdowle]

very nice! i.e. glad to see the back of NA_INT64_LL and those unsigned long long casts.

[mattdowle]

I tackled the news item by focusing it on the most easily conveyed example: nafill(xInt64, fill=0). Also added the error message so that folk searching for the error message might find it, as per contributing guide point 2 which has gotten quite dense now.
I didn't see why there were two blocks for the tests: first one being one loop, the 2nd being a nested loop. I started playing with that and testing in v1.14.2 to see which of the new tests worked before. I liked the nested loop so tried to make that the only loop to see what would happen. The revised new test passes in v1.14.2 but now fails in this PR. I committed anyway, and I'm thinking coerceAs could be enhanced now to make these pass again, since it seems to be singleton 0 and NA which could be coerced without warning from complex. Maybe there'll still be some warnings which should be tested by the tests. I'll keep going on this, hence WIP.

[ben-schwen]

I guess thats also kind of my fault. I only exchanged the coercing for the cases: INTSXP and REALSXP.
Changing it also for LGLSXP, CPLSXP and STRSXP should convert most errors into warnings.

Whats then still missing is the conversion from CPLSXP to other types. Is dropping imaginary part a valid approach for this? Maybe with a warning?

[mattdowle]

Looking at the coerceVector(source, SXP) in memrecycle made me realize that's not calling the as.character method; e.g. the below behavior in 1.14.2 looks wrong. I was expecting this to have been reported but I didn't find any issue. Does anyone know any issue numbers about this? integer64 (and nanotime) was turned off with an error asking user to as.character . So I'm about to remove the coerceInt64toStr function I just added in this PR and just change memrecycle to call as.character at R level to catch all cases.

> DT = data.table(A=letters[1:3])
> DT
        A
   <char>
1:      a
2:      b
3:      c
> DT[2, A:=as.Date("2020-03-04")]
> DT
        A
   <char>
1:      a
2:  18325
3:      c
> 

[mattdowle]

Tiny % drop in project coverage (and hence coverage fail on project vs 100% coverage of diff) is correct due to removal of covered code.