Fix -Wformat issues - #5769

(PS it would be great if we could add these compiler warnings to CI to prevent regressions on this in the future)

I poked around in r-devel a bit to see how they handle R_xlen_t in printf() formatted messages, but didn't see anything.

Therefore the approach in 0d57fd8 seems like the best expedient solution for now -- on most installations it will be a no-op (long -> long cast), and wherever R_xlen_t is just int, it'll cast int -> long.

See comment on the issue thread for why %lld was chosen for R_xlen_t.

(PS it would be great if we could add these compiler warnings to CI to prevent regressions on this in the future)

If you would happen to that docker that reproduces that please share. It's probably just compiler version and flags but will make it easier to add.

I added -Wformat -Wformat-security to ~/.R/Makevars and used gcc 13.2.0 (same as CRAN's gcc) but I am unable to reproduce those warnings from CRAN. Must be some extra flag then.

This is what we just talked about with @eddelbuettel about lack of CRAN reproducibility...

You need r-devel. I fixed two packages in two days. Works like a charm if you have r-devel and the Makevars change.

I rebuilt each day, but you should not have to. I got my first email about it while I was traveling -- Thu or Fri.

edd@rob:~/git/rcppcnpy(master)$ RD --version
R Under development (unstable) (2023-11-26 r85638) -- "Unsuffered Consequences"
Copyright (C) 2023 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under the terms of the
GNU General Public License versions 2 or 3.
For more information about these matters see
https://www.gnu.org/licenses/.

edd@rob:~/git/rcppcnpy(master)$ 

h/t @HughParsonage for this r-package-devel thread getting directly to the issue:

https://stat.ethz.ch/pipermail/r-package-devel/2023q4/010123.html

That points out R itself has a new macro R_PRIdXLEN_T:

https://github.com/r-devel/r-svn/blob/a9ded8dd775dc0aa75b26a5c88c11983d6d884f9/src/include/Rinternals.h#L75-L79

But we're miles away from being able to depend on such a recent addition to R. We could define the macro ourselves; it will be fragile but seems like no better choice at the moment if we want to avoid long long.

From the thread you linked it sounds long long is good enough, will work across different R versions.

I eyeballed the thread too but didn't like it much. I prefer to a) not depend on a particular version or b) make the code more complicated via an #if ... for a given architecture when c) a simple cast takes care of it. So I am with @jangorecki here: a simple cast does it. I used unsigned int to convert a (C++_ sizeof() that made R on Windows bark. All quiet now.

thanks all. was only wary of long long given the comments in the CRAN release notes. Agree it's best otherwise.

Considering it was commented out in CRAN release we should be good

testing PR via

sudo docker run -it --rm rocker/r-devel /bin/bash
mkdir -p ~/.R
echo 'CFLAGS=-Wformat' > ~/.R/Makevars
echo 'CXXFLAGS=-Wformat' >> ~/.R/Makevars

## optional
wget https://cran.r-project.org/src/contrib/data.table_1.14.8.tar.gz
RD CMD INSTALL data.table_1.14.8.tar.gz
# warnings reproduced

## PR
rm -fr format-warnings.zip data.table-format-warnings
wget https://github.com/Rdatatable/data.table/archive/refs/heads/format-warnings.zip
unzip format-warnings.zip
RD CMD build data.table-format-warnings --no-build-vignettes
RD CMD INSTALL data.table_1.14.9.tar.gz