[WIP] investigate -Wconversion warnings (close#391)

[thirdwing]

I spent some time investigating the -Wconversion warnings and fixed most of them.

Places I fixed:

(1) string_elt and vector_elt should accept R_xlen_t (https://github.com/wch/r-source/blob/trunk/src/include/Rinternals.h#L557-L558). This is something we really need to fix.

(2) length of vector should be size_t not int.

(3) the hashing functions should return unsigned int as we defined RCPP_HASH (https://github.com/RcppCore/Rcpp/blob/master/inst/include/Rcpp/hash/IndexHash.h#L46)

(4) I think Range sugar should accept R_xlen_t as start and end.

Places I don't fix:

(1) warnings in rowSums.h (https://github.com/RcppCore/Rcpp/blob/master/inst/include/Rcpp/sugar/functions/rowSums.h#L70). I think it is fine.

(2) Date and Datetime. I think @eddelbuettel knows these classes far better than me, so I didn't touch them.

Please take a look @kevinushey @eddelbuettel

[eddelbuettel]

Wow! Thanks a lot for all this.

I am wondering if we should break this into several separate PRs?

[eddelbuettel]

In short and without having looked in great detail yet:

(1) Yes, maybe in a separate PR?

(2) In principle, yes. In practice, won't that force a change on a bazillion lines of user code currently using int ?

(3) Yes.

(4) Yes.

These are all excellent points but maybe time to disentangle?

[thirdwing]

It might be better to break into several small PRs. I will do that later.

Let's wait for some opinions from Kevin.

BTW, I know you are working on a new Date class, so I didn't touch them.

[eddelbuettel]

Sounds good. The branch I am currently in updates the vector version for DateVector and DatetimeVector which are very silly and old. The 'single value' Date and Datetime may stay as is.

[kevinushey]

Nice work! Looks good to me here as well.

[kevinushey]

I believe we shouldn't be breaking any user code here -- I think we get automatic integral conversion from int to size_t from the compiler, so at worst users would be seeing some conversion warnings. Does that sound correct?

(some users might still be grumpy about it, though)

[thirdwing]

@kevinushey the size_t changes in this PR are mostly like this. No access from users.

[kevinushey]

I'm thinking more like https://github.com/RcppCore/Rcpp/pull/556/files#diff-a1af5c14c54d6d6907a0ca6f99d61286R69 (although that's int to R_xlen_t, so I misspoke earlier)

[eddelbuettel]

Yeah, I was thinking about what X.size() returns. Internal changes are fine, methinks.

[thirdwing]

Actually there are still tens of warnings about conversion between R_xlen_t and size_t.

They might not be defined as the same on some platforms.

[kevinushey]

Yes, R_xlen_t is defined as ptrdiff_t (https://cran.r-project.org/doc/manuals/r-release/R-ints.html#Long-vectors), which is a signed type. I would expect warnings of that form, and I'm not sure if we want to do anything about it (unless we decide to adopt R_xlen_t as our internal size type over std::size_t)

[eddelbuettel]

I'm closing this one now as we are taking this one topic at a time which seems a little easier.