Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use STRING_PTR_RO, not STRING_PTR #6312

Merged
merged 3 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/assign.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
if (sourceIsFactor) { sourceLevels=PROTECT(getAttrib(source, R_LevelsSymbol)); protecti++; }
if (!sourceIsFactor || !R_compute_identical(sourceLevels, targetLevels, 0)) { // !sourceIsFactor for test 2115.6
const int nTargetLevels=length(targetLevels), nSourceLevels=length(sourceLevels);
const SEXP *targetLevelsD=STRING_PTR(targetLevels), *sourceLevelsD=STRING_PTR(sourceLevels);
const SEXP *targetLevelsD=STRING_PTR_RO(targetLevels), *sourceLevelsD=STRING_PTR_RO(sourceLevels);
SEXP newSource = PROTECT(allocVector(INTSXP, length(source))); protecti++;
savetl_init();
for (int k=0; k<nTargetLevels; ++k) {
Expand Down Expand Up @@ -835,7 +835,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
newSourceD[i] = val==NA_INTEGER ? NA_INTEGER : -TRUELENGTH(sourceLevelsD[val-1]); // retains NA factor levels here via TL(NA_STRING); e.g. ordered factor
}
} else {
const SEXP *sourceD = STRING_PTR(source);
const SEXP *sourceD = STRING_PTR_RO(source);
for (int i=0; i<nSource; ++i) { // convert source integers to refer to target levels
const SEXP val = sourceD[i];
newSourceD[i] = val==NA_STRING ? NA_INTEGER : -TRUELENGTH(val);
Expand Down Expand Up @@ -1103,7 +1103,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
} break;
case STRSXP :
if (sourceIsFactor) {
const SEXP *ld = STRING_PTR(PROTECT(getAttrib(source, R_LevelsSymbol))); protecti++;
const SEXP *ld = STRING_PTR_RO(PROTECT(getAttrib(source, R_LevelsSymbol))); protecti++;
BODY(int, INTEGER, SEXP, val==NA_INTEGER ? NA_STRING : ld[val-1], SET_STRING_ELT(target, off+i, cval))
} else {
if (!isString(source)) {
Expand All @@ -1125,7 +1125,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
source = PROTECT(coerceVector(source, STRSXP)); protecti++;
}
}
BODY(SEXP, STRING_PTR, SEXP, val, SET_STRING_ELT(target, off+i, cval))
BODY(SEXP, STRING_PTR_RO, SEXP, val, SET_STRING_ELT(target, off+i, cval))
}
case VECSXP :
case EXPRSXP : { // #546 #4350
Expand All @@ -1140,12 +1140,12 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
// the UNPROTECT can be at the end of the CAST before the SET_VECTOR_ELT, because SET_VECTOR_ELT will protect it and there's no other code in between
// the PROTECT is now needed because of the call to LOGICAL() which could feasibly gc inside it.
// copyMostAttrib is inside CAST so as to be outside loop. See the history in #4350 and its follow up
case RAWSXP: BODY(Rbyte, RAW, SEXP, PROTECT(allocVector(RAWSXP, 1));RAW(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case LGLSXP: BODY(int, LOGICAL, SEXP, PROTECT(allocVector(LGLSXP, 1));LOGICAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case INTSXP: BODY(int, INTEGER, SEXP, PROTECT(allocVector(INTSXP, 1));INTEGER(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case REALSXP: BODY(double, REAL, SEXP, PROTECT(allocVector(REALSXP, 1));REAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case CPLXSXP: BODY(Rcomplex, COMPLEX, SEXP, PROTECT(allocVector(CPLXSXP, 1));COMPLEX(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case STRSXP: BODY(SEXP, STRING_PTR, SEXP, PROTECT(allocVector(STRSXP, 1));SET_STRING_ELT(cval, 0, val);copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case RAWSXP: BODY(Rbyte, RAW, SEXP, PROTECT(allocVector(RAWSXP, 1));RAW(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case LGLSXP: BODY(int, LOGICAL, SEXP, PROTECT(allocVector(LGLSXP, 1));LOGICAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case INTSXP: BODY(int, INTEGER, SEXP, PROTECT(allocVector(INTSXP, 1));INTEGER(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case REALSXP: BODY(double, REAL, SEXP, PROTECT(allocVector(REALSXP, 1));REAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case CPLXSXP: BODY(Rcomplex, COMPLEX, SEXP, PROTECT(allocVector(CPLXSXP, 1));COMPLEX(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case STRSXP: BODY(SEXP, STRING_PTR_RO, SEXP, PROTECT(allocVector(STRSXP, 1));SET_STRING_ELT(cval, 0, val);copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
case VECSXP:
case EXPRSXP: BODY(SEXP, SEXPPTR_RO, SEXP, val, SET_VECTOR_ELT(target,off+i,cval))
default: COERCE_ERROR("list");
Expand Down
6 changes: 3 additions & 3 deletions src/between.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S
break;

case STRSXP: {
const SEXP *lp = STRING_PTR(lower);
const SEXP *up = STRING_PTR(upper);
const SEXP *xp = STRING_PTR(x);
const SEXP *lp = STRING_PTR_RO(lower);
const SEXP *up = STRING_PTR_RO(upper);
const SEXP *xp = STRING_PTR_RO(x);
#define LCMP (strcmp(CHAR(ENC2UTF8(l)),CHAR(ENC2UTF8(elem)))<=-open)
#define UCMP (strcmp(CHAR(ENC2UTF8(elem)),CHAR(ENC2UTF8(u)))<=-open)
// TODO if all ascii can be parallel, otherwise ENC2UTF8 could allocate
Expand Down
4 changes: 2 additions & 2 deletions src/bmerge.c
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
case STRSXP : {
// op[col]==EQ checked up front to avoid an if() here and non-thread-safe error()
// not sure why non-EQ is not supported for STRSXP though as it seems straightforward (StrCmp returns sign to indicate GT or LT)
const SEXP *icv = STRING_PTR(ic);
const SEXP *xcv = STRING_PTR(xc);
const SEXP *icv = STRING_PTR_RO(ic);
const SEXP *xcv = STRING_PTR_RO(xc);
const SEXP ival = ENC2UTF8(icv[ir]);
#undef ISNAT
#undef WRAP
Expand Down
6 changes: 3 additions & 3 deletions src/chmatch.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
if (TYPEOF(x) == SYMSXP) {
if (xlen!=1)
error(_("Internal error: length of SYMSXP is %d not 1"), xlen); // # nocov
sym = PRINTNAME(x); // so we can do &sym to get a length 1 (const SEXP *)STRING_PTR(x) and save an alloc for coerce to STRSXP
sym = PRINTNAME(x); // so we can do &sym to get a length 1 (const SEXP *)STRING_PTR_RO(x) and save an alloc for coerce to STRSXP
} else if (!isString(x) && !isSymbol(x) && !isNull(x)) {
if (chin && !isVectorAtomic(x)) {
return ScalarLogical(FALSE);
Expand Down Expand Up @@ -40,9 +40,9 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
if (isSymbol(x)) {
xd = &sym;
} else {
xd = STRING_PTR(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
xd = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
}
const SEXP *td = STRING_PTR(PROTECT(coerceUtf8IfNeeded(table))); nprotect++;
const SEXP *td = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(table))); nprotect++;
if (xlen==1) {
ansd[0] = nomatch;
for (int i=0; i<tablelen; ++i) {
Expand Down
2 changes: 1 addition & 1 deletion src/cj.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ SEXP cj(SEXP base_list) {
}
} break;
case STRSXP: {
const SEXP *sourceP = STRING_PTR(source);
const SEXP *sourceP = STRING_PTR_RO(source);
int start = 0;
for (int i=0; i<ncopy; ++i) {
for (int j=0; j<thislen; ++j) {
Expand Down
4 changes: 2 additions & 2 deletions src/coalesce.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
}
} break;
case STRSXP: {
const SEXP *xP = STRING_PTR(first);
const SEXP *xP = STRING_PTR_RO(first);
SEXP finalVal=NA_STRING;
int k=0;
for (int j=0; j<nval; ++j) {
Expand All @@ -152,7 +152,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
finalVal = tt;
break;
}
valP[k++] = STRING_PTR(item);
valP[k++] = STRING_PTR_RO(item);
}
const bool final = (finalVal!=NA_STRING);
for (int i=0; i<nrow; ++i) {
Expand Down
3 changes: 3 additions & 0 deletions src/data.table.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
#endif
#include <Rinternals.h>
#define SEXPPTR_RO(x) ((const SEXP *)DATAPTR_RO(x)) // to avoid overhead of looped STRING_ELT and VECTOR_ELT
#ifndef STRING_PTR_RO
#define STRING_PTR_RO STRING_PTR
#endif
#include <stdint.h> // for uint64_t rather than unsigned long long
#include <stdbool.h>
#include "types.h"
Expand Down
10 changes: 5 additions & 5 deletions src/fifelse.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b, SEXP na) {
}
} break;
case STRSXP : {
const SEXP *restrict pa = na_a ? NULL : STRING_PTR(a);
const SEXP *restrict pb = na_b ? NULL : STRING_PTR(b);
const SEXP *restrict pna = na_n ? NULL : STRING_PTR(na);
const SEXP *restrict pa = na_a ? NULL : STRING_PTR_RO(a);
const SEXP *restrict pb = na_b ? NULL : STRING_PTR_RO(b);
const SEXP *restrict pna = na_n ? NULL : STRING_PTR_RO(na);
const SEXP na = NA_STRING;
for (int64_t i=0; i<len0; ++i) {
SET_STRING_ELT(
Expand Down Expand Up @@ -358,8 +358,8 @@ SEXP fcaseR(SEXP na, SEXP rho, SEXP args) {
}
} break;
case STRSXP: {
const SEXP *restrict pouts = STRING_PTR(outs);
const SEXP pna = nonna ? STRING_PTR(na)[0] : NA_STRING;
const SEXP *restrict pouts = STRING_PTR_RO(outs);
const SEXP pna = nonna ? STRING_PTR_RO(na)[0] : NA_STRING;
for (int64_t j=0; j<len2; ++j) {
const int64_t idx = imask ? j : p[j];
if (pcons[idx]==1) {
Expand Down
6 changes: 3 additions & 3 deletions src/fmelt.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,13 @@ static SEXP combineFactorLevels(SEXP factorLevels, SEXP target, int * factorType
SEXP ans = PROTECT(allocVector(INTSXP, nrow));
SEXP *levelsRaw = (SEXP *)R_alloc(maxlevels, sizeof(SEXP)); // allocate for worst-case all-unique levels
int *ansd = INTEGER(ans);
const SEXP *targetd = STRING_PTR(target);
const SEXP *targetd = STRING_PTR_RO(target);
savetl_init();
// no alloc or any fail point until savetl_end()
int nlevel=0;
for (int i=0; i<nitem; ++i) {
const SEXP this = VECTOR_ELT(factorLevels, i);
const SEXP *thisd = STRING_PTR(this);
const SEXP *thisd = STRING_PTR_RO(this);
const int thisn = length(this);
for (int k=0; k<thisn; ++k) {
SEXP s = thisd[k];
Expand Down Expand Up @@ -745,7 +745,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
counter += thislen;
}
} else {
const SEXP *s = STRING_PTR(thiscol); // to reduce overhead of STRING_ELT() inside loop below. Read-only hence const.
const SEXP *s = STRING_PTR_RO(thiscol); // to reduce overhead of STRING_ELT() inside loop below. Read-only hence const.
for (int j=0; j<data->lmax; ++j) {
for (int k=0; k<data->nrow; ++k) {
SET_STRING_ELT(target, j*data->nrow + k, s[k]);
Expand Down
14 changes: 7 additions & 7 deletions src/forder.c
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ static void cradix(SEXP *x, int n)
free(cradix_xtmp); cradix_xtmp=NULL;
}

static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int *out_na_count)
static void range_str(const SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int *out_na_count)
// group numbers are left in truelength to be fetched by WRITE_KEY
{
int na_count=0;
Expand Down Expand Up @@ -323,7 +323,7 @@ static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int
SEXP *ustr3 = (SEXP *)malloc(ustr_n * sizeof(SEXP));
if (!ustr3)
STOP(_("Failed to alloc ustr3 when converting strings to UTF8")); // # nocov
memcpy(ustr3, STRING_PTR(ustr2), ustr_n*sizeof(SEXP));
memcpy(ustr3, STRING_PTR_RO(ustr2), ustr_n*sizeof(SEXP));
// need to reset ustr_maxlen because we need ustr_maxlen for utf8 strings
ustr_maxlen = 0;
for (int i=0; i<ustr_n; i++) {
Expand All @@ -342,7 +342,7 @@ static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int
int *tl = (int *)malloc(ustr_n * sizeof(int));
if (!tl)
STOP(_("Failed to alloc tl when converting strings to UTF8")); // # nocov
SEXP *tt = STRING_PTR(ustr2);
const SEXP *tt = STRING_PTR_RO(ustr2);
for (int i=0; i<ustr_n; i++) tl[i] = TRUELENGTH(tt[i]); // fetches the o in ustr3 into tl which is ordered by ustr
for (int i=0; i<ustr_n; i++) SET_TRUELENGTH(ustr3[i], 0); // reset to 0 tl of the UTF8 (and possibly non-UTF in ustr too)
for (int i=0; i<ustr_n; i++) SET_TRUELENGTH(ustr[i], tl[i]); // put back the o into ustr's tl
Expand Down Expand Up @@ -552,7 +552,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
break;
case STRSXP :
// need2utf8 now happens inside range_str on the uniques
range_str(STRING_PTR(x), nrow, &min, &max, &na_count);
range_str(STRING_PTR_RO(x), nrow, &min, &max, &na_count);
break;
default:
STOP(_("Column %d passed to [f]order is type '%s', not yet supported."), col+1, type2char(TYPEOF(x)));
Expand Down Expand Up @@ -693,7 +693,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
}
break;
case STRSXP : {
SEXP *xd = STRING_PTR(x);
const SEXP *xd = STRING_PTR_RO(x);
#pragma omp parallel for num_threads(getDTthreads(nrow, true))
for (int i=0; i<nrow; i++) {
uint64_t elem=0;
Expand Down Expand Up @@ -1326,7 +1326,7 @@ SEXP issorted(SEXP x, SEXP by)
}
break;
case STRSXP : {
SEXP *xd = STRING_PTR(x);
const SEXP *xd = STRING_PTR_RO(x);
i = 0;
while (i<n && xd[i]==NA_STRING) i++;
if (i==n) break; // xd consists only of NA_STRING #5070
Expand Down Expand Up @@ -1369,7 +1369,7 @@ SEXP issorted(SEXP x, SEXP by)
break;
case STRSXP:
types[j] = 3;
ptrs[j] = (const char *)STRING_PTR(col);
ptrs[j] = (const char *)STRING_PTR_RO(col);
break;
default:
STOP(_("type '%s' is not yet supported"), type2char(TYPEOF(col))); // # nocov
Expand Down
4 changes: 2 additions & 2 deletions src/frank.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ SEXP dt_na(SEXP x, SEXP cols) {
}
break;
case STRSXP: {
const SEXP *sv = STRING_PTR(v);
const SEXP *sv = STRING_PTR_RO(v);
for (int j=0; j<n; ++j) ians[j] |= (sv[j] == NA_STRING);
}
break;
Expand Down Expand Up @@ -209,7 +209,7 @@ SEXP anyNA(SEXP x, SEXP cols) {
while(j<n && iv[j]!=NA_INTEGER) j++;
} break;
case STRSXP: {
const SEXP *sv = STRING_PTR(v);
const SEXP *sv = STRING_PTR_RO(v);
while (j<n && sv[j]!=NA_STRING) j++;
} break;
case REALSXP:
Expand Down
2 changes: 1 addition & 1 deletion src/fwriteR.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ int getMaxStringLen(const SEXP *col, const int64_t n) {
int getMaxCategLen(SEXP col) {
col = getAttrib(col, R_LevelsSymbol);
if (!isString(col)) error(_("Internal error: col passed to getMaxCategLen is missing levels"));
return getMaxStringLen( STRING_PTR(col), LENGTH(col) );
return getMaxStringLen( STRING_PTR_RO(col), LENGTH(col) );
}

const char *getCategString(SEXP col, int64_t row) {
Expand Down
8 changes: 4 additions & 4 deletions src/gsumm.c
Original file line number Diff line number Diff line change
Expand Up @@ -763,8 +763,8 @@ static SEXP gminmax(SEXP x, SEXP narm, const bool min)
break;
case STRSXP: {
ans = PROTECT(allocVector(STRSXP, ngrp));
const SEXP *ansd = STRING_PTR(ans);
const SEXP *xd = STRING_PTR(x);
const SEXP *ansd = STRING_PTR_RO(ans);
const SEXP *xd = STRING_PTR_RO(x);
if (!LOGICAL(narm)[0]) {
const SEXP init = min ? char_maxString : R_BlankString; // char_maxString == "\xFF\xFF..." in init.c
for (int i=0; i<ngrp; ++i) SET_STRING_ELT(ans, i, init);
Expand Down Expand Up @@ -978,7 +978,7 @@ static SEXP gfirstlast(SEXP x, const bool first, const int w, const bool headw)
int64_t *ansd=(int64_t *)REAL(ans); DO(int64_t, REAL, NA_INTEGER64, ansd[ansi++]=val) }
else { double *ansd=REAL(ans); DO(double, REAL, NA_REAL, ansd[ansi++]=val) } break;
case CPLXSXP: { Rcomplex *ansd=COMPLEX(ans); DO(Rcomplex, COMPLEX, NA_CPLX, ansd[ansi++]=val) } break;
case STRSXP: DO(SEXP, STRING_PTR, NA_STRING, SET_STRING_ELT(ans,ansi++,val)) break;
case STRSXP: DO(SEXP, STRING_PTR_RO, NA_STRING, SET_STRING_ELT(ans,ansi++,val)) break;
case VECSXP: DO(SEXP, SEXPPTR_RO, ScalarLogical(NA_LOGICAL), SET_VECTOR_ELT(ans,ansi++,val)) break;
default:
error(_("Type '%s' is not supported by GForce head/tail/first/last/`[`. Either add the namespace prefix (e.g. utils::head(.)) or turn off GForce optimization using options(datatable.optimize=1)"), type2char(TYPEOF(x)));
Expand Down Expand Up @@ -1266,7 +1266,7 @@ SEXP gshift(SEXP x, SEXP nArg, SEXP fillArg, SEXP typeArg) {
case REALSXP: { double *ansd=REAL(tmp); SHIFT(double, REAL, ansd[ansi++]=val); } break;
// integer64 is shifted as if it's REAL; and assigning fill=NA_INTEGER64 is ok as REAL
case CPLXSXP: { Rcomplex *ansd=COMPLEX(tmp); SHIFT(Rcomplex, COMPLEX, ansd[ansi++]=val); } break;
case STRSXP: { SHIFT(SEXP, STRING_PTR, SET_STRING_ELT(tmp,ansi++,val)); } break;
case STRSXP: { SHIFT(SEXP, STRING_PTR_RO, SET_STRING_ELT(tmp,ansi++,val)); } break;
//case VECSXP: { SHIFT(SEXP, SEXPPTR_RO, SET_VECTOR_ELT(tmp,ansi++,val)); } break;
default:
error(_("Type '%s' is not supported by GForce gshift. Either add the namespace prefix (e.g. data.table::shift(.)) or turn off GForce optimization using options(datatable.optimize=1)"), type2char(TYPEOF(x)));
Expand Down
Loading
Loading