Skip to content

Commit

Permalink
pass -Wstrict-prototypes for CRAN (#5477)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdowle authored Oct 6, 2022
1 parent f0e6da5 commit 7f9ff44
Show file tree
Hide file tree
Showing 14 changed files with 181 additions and 198 deletions.
3 changes: 2 additions & 1 deletion .dev/cc.R
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ cc = function(test=FALSE, clean=FALSE, debug=FALSE, omp=!debug, cc_dir, path=Sys
if (debug) {
ret = system(sprintf("MAKEFLAGS='-j CC=%s PKG_CFLAGS=-f%sopenmp CFLAGS=-std=c99\\ -O0\\ -ggdb\\ -pedantic' R CMD SHLIB -d -o data_table.so *.c", CC, OMP))
} else {
ret = system(sprintf("MAKEFLAGS='-j CC=%s CFLAGS=-f%sopenmp\\ -std=c99\\ -O3\\ -pipe\\ -Wall\\ -pedantic\\ -fno-common' R CMD SHLIB -o data_table.so *.c", CC, OMP))
ret = system(sprintf("MAKEFLAGS='-j CC=%s CFLAGS=-f%sopenmp\\ -std=c99\\ -O3\\ -pipe\\ -Wall\\ -pedantic\\ -Wstrict-prototypes\\ -isystem\\ /usr/share/R/include\\ -fno-common' R CMD SHLIB -o data_table.so *.c", CC, OMP))
# the -isystem suppresses strict-prototypes warnings from R's headers. Look at the output to see what -I is and pass the same path to -isystem.
# TODO add -Wextra too?
}
if (ret) return()
Expand Down
4 changes: 3 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,12 @@

## NOTES

1. gcc 12.1 (May 2022) now detects and warns about an always-false condition in `fread` which caused a small efficiency saving never to be invoked. Thanks to CRAN for testing latest versions of compilers.
1. gcc 12.1 (May 2022) now detects and warns about an always-false condition (`-Waddress`) in `fread` which caused a small efficiency saving never to be invoked. Thanks to CRAN for testing latest versions of compilers.

2. `update.dev.pkg()` has been renamed `update_dev_pkg()` to get out of the way of the `stats::update` generic function, [#5421](https://github.com/Rdatatable/data.table/pull/5421). This is a utility function which upgrades the version of `data.table` to the latest commit in development which has passed all tests. As such we don't expect any backwards compatibility concerns. Its manual page was causing an intermittent hang/crash from `R CMD check` on Windows-only on CRAN which we hope will be worked around by changing its name.

3. Internal C code now passes `-Wstrict-prototypes` to satisfy the warnings now displayed on CRAN.


# data.table [v1.14.2](https://github.com/Rdatatable/data.table/milestone/24?closed=1) (27 Sep 2021)

Expand Down
4 changes: 2 additions & 2 deletions src/assign.c
Original file line number Diff line number Diff line change
Expand Up @@ -1190,7 +1190,7 @@ SEXP allocNAVectorLike(SEXP x, R_len_t n) {
static SEXP *saveds=NULL;
static R_len_t *savedtl=NULL, nalloc=0, nsaved=0;

void savetl_init() {
void savetl_init(void) {
if (nsaved || nalloc || saveds || savedtl) {
error(_("Internal error: savetl_init checks failed (%d %d %p %p). please report to data.table issue tracker."), nsaved, nalloc, saveds, savedtl); // # nocov
}
Expand Down Expand Up @@ -1231,7 +1231,7 @@ void savetl(SEXP s)
nsaved++;
}

void savetl_end() {
void savetl_end(void) {
// Can get called if nothing has been saved yet (nsaved==0), or even if _init() hasn't been called yet (pointers NULL). Such
// as to clear up before error. Also, it might be that nothing needed to be saved anyway.
for (int i=0; i<nsaved; i++) SET_TRUELENGTH(saveds[i],savedtl[i]);
Expand Down
83 changes: 77 additions & 6 deletions src/data.table.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ extern size_t __typeorder[100]; // __ prefix otherwise if we use these names dir

long long DtoLL(double x);
double LLtoD(long long x);
int GetVerbose();
int GetVerbose(void);

// cj.c
SEXP cj(SEXP base_list);
Expand All @@ -125,14 +125,14 @@ SEXP growVector(SEXP x, R_len_t newlen);
SEXP allocNAVector(SEXPTYPE type, R_len_t n);
SEXP allocNAVectorLike(SEXP x, R_len_t n);
void writeNA(SEXP v, const int from, const int n, const bool listNA);
void savetl_init(), savetl(SEXP s), savetl_end();
void savetl_init(void), savetl(SEXP s), savetl_end(void);
int checkOverAlloc(SEXP x);

// forder.c
int StrCmp(SEXP x, SEXP y);
uint64_t dtwiddle(double x);
SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, SEXP naArg);
int getNumericRounding_C();
int getNumericRounding_C(void);

// reorder.c
SEXP reorder(SEXP x, SEXP order);
Expand Down Expand Up @@ -189,12 +189,12 @@ double iquickselect(int *x, int n);
double i64quickselect(int64_t *x, int n);

// fread.c
double wallclock();
double wallclock(void);

// openmp-utils.c
void initDTthreads();
void initDTthreads(void);
int getDTthreads(const int64_t n, const bool throttle);
void avoid_openmp_hang_within_fork();
void avoid_openmp_hang_within_fork(void);

// froll.c
void frollmean(unsigned int algo, double *x, uint64_t nx, ans_t *ans, int k, int align, double fill, bool narm, int hasna, bool verbose);
Expand Down Expand Up @@ -262,3 +262,74 @@ SEXP substitute_call_arg_namesR(SEXP expr, SEXP env);

//negate.c
SEXP notchin(SEXP x, SEXP table);

// functions called from R level .Call/.External and registered in init.c
// these now live here to pass -Wstrict-prototypes, #5477
// all arguments must be SEXP since they are called from R level
// where there are no arguments, it must be (void) not () to be a strict prototype
SEXP setattrib(SEXP, SEXP, SEXP);
SEXP assign(SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP copy(SEXP);
SEXP alloccolwrapper(SEXP, SEXP, SEXP);
SEXP selfrefokwrapper(SEXP, SEXP);
SEXP truelength(SEXP);
SEXP setcharvec(SEXP, SEXP, SEXP);
SEXP chmatch_R(SEXP, SEXP, SEXP);
SEXP chmatchdup_R(SEXP, SEXP, SEXP);
SEXP chin_R(SEXP, SEXP);
SEXP freadR(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP fwriteR(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP rbindlist(SEXP, SEXP, SEXP, SEXP);
SEXP setlistelt(SEXP, SEXP, SEXP);
SEXP address(SEXP);
SEXP expandAltRep(SEXP);
SEXP fmelt(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP fcast(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP issorted(SEXP, SEXP);
SEXP gforce(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP gsum(SEXP, SEXP);
SEXP gmean(SEXP, SEXP);
SEXP gmin(SEXP, SEXP);
SEXP gmax(SEXP, SEXP);
SEXP setNumericRounding(SEXP);
SEXP getNumericRounding(void);
SEXP binary(SEXP);
SEXP subsetDT(SEXP, SEXP, SEXP);
SEXP convertNegAndZeroIdx(SEXP, SEXP, SEXP, SEXP);
SEXP frank(SEXP, SEXP, SEXP, SEXP);
SEXP lookup(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP overlaps(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP whichwrapper(SEXP, SEXP);
SEXP shift(SEXP, SEXP, SEXP, SEXP);
SEXP transpose(SEXP, SEXP, SEXP, SEXP);
SEXP anyNA(SEXP, SEXP);
SEXP setlevels(SEXP, SEXP, SEXP);
SEXP rleid(SEXP, SEXP);
SEXP gmedian(SEXP, SEXP);
SEXP gtail(SEXP, SEXP);
SEXP ghead(SEXP, SEXP);
SEXP glast(SEXP);
SEXP gfirst(SEXP);
SEXP gnthvalue(SEXP, SEXP);
SEXP dim(SEXP);
SEXP gvar(SEXP, SEXP);
SEXP gsd(SEXP, SEXP);
SEXP gprod(SEXP, SEXP);
SEXP gshift(SEXP, SEXP, SEXP, SEXP);
SEXP nestedid(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP setDTthreads(SEXP, SEXP, SEXP, SEXP);
SEXP getDTthreads_R(SEXP);
SEXP nqRecreateIndices(SEXP, SEXP, SEXP, SEXP, SEXP);
SEXP fsort(SEXP, SEXP);
SEXP inrange(SEXP, SEXP, SEXP, SEXP);
SEXP hasOpenMP(void);
SEXP uniqueNlogical(SEXP, SEXP);
SEXP dllVersion(void);
SEXP initLastUpdated(SEXP);
SEXP allNAR(SEXP);
SEXP test_dt_win_snprintf(void);
SEXP dt_zlib_version(void);
SEXP startsWithAny(SEXP, SEXP, SEXP);
SEXP convertDate(SEXP, SEXP);
SEXP fastmean(SEXP);

10 changes: 5 additions & 5 deletions src/forder.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ static char msg[1001];
* Therefore, using <<if (!malloc()) STOP(_("helpful context msg"))>> approach to cleanup() on error.
*/

static void free_ustr() {
static void free_ustr(void) {
for(int i=0; i<ustr_n; i++)
SET_TRUELENGTH(ustr[i],0);
free(ustr); ustr=NULL;
ustr_alloc=0; ustr_n=0; ustr_maxlen=0;
}

static void cleanup() {
static void cleanup(void) {
free(gs); gs=NULL;
gs_alloc = 0;
gs_n = 0;
Expand Down Expand Up @@ -110,7 +110,7 @@ static void push(const int *x, const int n) {
gs_thread_n[me] += n;
}

static void flush() {
static void flush(void) {
if (!retgrp) return;
int me = omp_get_thread_num();
int n = gs_thread_n[me];
Expand Down Expand Up @@ -373,12 +373,12 @@ SEXP setNumericRounding(SEXP droundArg)
return R_NilValue;
}

SEXP getNumericRounding()
SEXP getNumericRounding(void)
{
return ScalarInteger(dround);
}

int getNumericRounding_C()
int getNumericRounding_C(void)
// for use in uniqlist.c
{
return dround;
Expand Down
2 changes: 1 addition & 1 deletion src/fread.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static double NAND;
static double INFD;

// NAN and INFINITY constants are float, so cast to double once up front.
void init() {
void init(void) {
NAND = (double)NAN;
INFD = (double)INFINITY;
}
Expand Down
53 changes: 27 additions & 26 deletions src/fwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,17 @@ inline void write_chars(const char *x, char **pch)
*pch = ch;
}

void writeBool8(int8_t *col, int64_t row, char **pch)
void writeBool8(const void *col, int64_t row, char **pch)
{
int8_t x = col[row];
int8_t x = ((const int8_t *)col)[row];
char *ch = *pch;
*ch++ = '0'+(x==1);
*pch = ch-(x==INT8_MIN); // if NA then step back, to save a branch
}

void writeBool32(int32_t *col, int64_t row, char **pch)
void writeBool32(const void *col, int64_t row, char **pch)
{
int32_t x = col[row];
int32_t x = ((const int32_t *)col)[row];
char *ch = *pch;
if (x==INT32_MIN) { // TODO: when na=='\0' as recommended, use a branchless writer
write_chars(na, &ch);
Expand All @@ -80,9 +80,9 @@ void writeBool32(int32_t *col, int64_t row, char **pch)
*pch = ch;
}

void writeBool32AsString(int32_t *col, int64_t row, char **pch)
void writeBool32AsString(const void *col, int64_t row, char **pch)
{
int32_t x = col[row];
int32_t x = ((const int32_t *)col)[row];
char *ch = *pch;
if (x == INT32_MIN) {
write_chars(na, &ch);
Expand All @@ -106,10 +106,10 @@ static inline void reverse(char *upp, char *low)
}
}

void writeInt32(int32_t *col, int64_t row, char **pch)
void writeInt32(const void *col, int64_t row, char **pch)
{
char *ch = *pch;
int32_t x = col[row];
int32_t x = ((const int32_t *)col)[row];
if (x == INT32_MIN) {
write_chars(na, &ch);
} else {
Expand All @@ -122,10 +122,10 @@ void writeInt32(int32_t *col, int64_t row, char **pch)
*pch = ch;
}

void writeInt64(int64_t *col, int64_t row, char **pch)
void writeInt64(const void *col, int64_t row, char **pch)
{
char *ch = *pch;
int64_t x = col[row];
int64_t x = ((const int64_t *)col)[row];
if (x == INT64_MIN) {
write_chars(na, &ch);
} else {
Expand Down Expand Up @@ -177,7 +177,7 @@ void genLookups() {
}
*/

void writeFloat64(double *col, int64_t row, char **pch)
void writeFloat64(const void *col, int64_t row, char **pch)
{
// hand-rolled / specialized for speed
// *pch is safely the output destination with enough space (ensured via calculating maxLineLen up front)
Expand All @@ -187,7 +187,7 @@ void writeFloat64(double *col, int64_t row, char **pch)
// ii) no C libary calls such as sprintf() where the fmt string has to be interpretted over and over
// iii) no need to return variables or flags. Just writes.
// iv) shorter, easier to read and reason with in one self contained place.
double x = col[row];
double x = ((const double *)col)[row];
char *ch = *pch;
if (!isfinite(x)) {
if (isnan(x)) {
Expand Down Expand Up @@ -301,9 +301,9 @@ void writeFloat64(double *col, int64_t row, char **pch)
*pch = ch;
}

void writeComplex(Rcomplex *col, int64_t row, char **pch)
void writeComplex(const void *col, int64_t row, char **pch)
{
Rcomplex x = col[row];
Rcomplex x = ((const Rcomplex *)col)[row];
char *ch = *pch;
writeFloat64(&x.r, 0, &ch);
if (!ISNAN(x.i)) {
Expand Down Expand Up @@ -340,8 +340,8 @@ static inline void write_time(int32_t x, char **pch)
*pch = ch;
}

void writeITime(int32_t *col, int64_t row, char **pch) {
write_time(col[row], pch);
void writeITime(const void *col, int64_t row, char **pch) {
write_time(((const int32_t *)col)[row], pch);
}

static inline void write_date(int32_t x, char **pch)
Expand Down Expand Up @@ -394,15 +394,16 @@ static inline void write_date(int32_t x, char **pch)
*pch = ch;
}

void writeDateInt32(int32_t *col, int64_t row, char **pch) {
write_date(col[row], pch);
void writeDateInt32(const void *col, int64_t row, char **pch) {
write_date(((const int32_t *)col)[row], pch);
}

void writeDateFloat64(double *col, int64_t row, char **pch) {
write_date(isfinite(col[row]) ? (int)(col[row]) : INT32_MIN, pch);
void writeDateFloat64(const void *col, int64_t row, char **pch) {
double x = ((const double *)col)[row];
write_date(isfinite(x) ? (int)(x) : INT32_MIN, pch);
}

void writePOSIXct(double *col, int64_t row, char **pch)
void writePOSIXct(const void *col, int64_t row, char **pch)
{
// Write ISO8601 UTC by default to encourage ISO standards, stymie ambiguity and for speed.
// R internally represents POSIX datetime in UTC always. Its 'tzone' attribute can be ignored.
Expand All @@ -411,7 +412,7 @@ void writePOSIXct(double *col, int64_t row, char **pch)
// All positive integers up to 2^53 (9e15) are exactly representable by double which is relied
// on in the ops here; number of seconds since epoch.

double x = col[row];
double x = ((const double *)col)[row];
char *ch = *pch;
if (!isfinite(x)) {
write_chars(na, &ch);
Expand Down Expand Up @@ -464,9 +465,9 @@ void writePOSIXct(double *col, int64_t row, char **pch)
*pch = ch;
}

void writeNanotime(int64_t *col, int64_t row, char **pch)
void writeNanotime(const void *col, int64_t row, char **pch)
{
int64_t x = col[row];
int64_t x = ((const int64_t *)col)[row];
char *ch = *pch;
if (x == INT64_MIN) {
write_chars(na, &ch);
Expand Down Expand Up @@ -549,12 +550,12 @@ static inline void write_string(const char *x, char **pch)

void writeString(const void *col, int64_t row, char **pch)
{
write_string(getString(col, row), pch);
write_string(getString((const SEXP *)col, row), pch);
}

void writeCategString(const void *col, int64_t row, char **pch)
{
write_string(getCategString(col, row), pch);
write_string(getCategString((const SEXP *)col, row), pch);
}

#ifndef NOZLIB
Expand Down
Loading

0 comments on commit 7f9ff44

Please sign in to comment.