Skip to content

Commit be921b1

Browse files
committed
Drop remaining uses of TRUELENGTH from assign.c
- Don't SET_TRUELENGTH by hand. All of our resizable vectors now have the GROWABLE_BIT set, so when they are duplicated, TRUELENGTH is reset to 0. - Use a combination of R_isResizable and R_maxLength to replace other uses of TRUELENGTH.
1 parent 332bfba commit be921b1

File tree

2 files changed

+9
-13
lines changed

2 files changed

+9
-13
lines changed

src/assign.c

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,9 @@ static int _selfrefok(SEXP x, Rboolean checkNames, Rboolean verbose) {
7171
tag = R_ExternalPtrTag(v);
7272
if (!(isNull(tag) || isString(tag))) internal_error(__func__, ".internal.selfref tag is neither NULL nor a character vector"); // # nocov
7373
names = getAttrib(x, R_NamesSymbol);
74-
if (names!=tag && isString(names) && !ALTREP(names)) // !ALTREP for #4734
75-
SET_TRUELENGTH(names, LENGTH(names));
76-
// R copied this vector not data.table; it's not actually over-allocated. It looks over-allocated
77-
// because R copies the original vector's tl over despite allocating length.
7874
prot = R_ExternalPtrProtected(v);
7975
if (TYPEOF(prot) != EXTPTRSXP) // Very rare. Was error(_(".internal.selfref prot is not itself an extptr")).
8076
return 0; // # nocov ; see http://stackoverflow.com/questions/15342227/getting-a-random-internal-selfref-error-in-data-table-for-r
81-
if (x!=R_ExternalPtrAddr(prot) && !ALTREP(x))
82-
SET_TRUELENGTH(x, LENGTH(x)); // R copied this vector not data.table, it's not actually over-allocated
8377
return checkNames ? names==tag : x==R_ExternalPtrAddr(prot);
8478
}
8579

@@ -201,7 +195,7 @@ SEXP alloccol(SEXP dt, R_len_t n, Rboolean verbose)
201195
// if (TRUELENGTH(getAttrib(dt,R_NamesSymbol))!=tl)
202196
// internal_error(__func__, "tl of dt passes checks, but tl of names (%d) != tl of dt (%d)", tl, TRUELENGTH(getAttrib(dt,R_NamesSymbol))); // # nocov
203197

204-
tl = TRUELENGTH(dt);
198+
tl = R_maxLength(dt);
205199
// R <= 2.13.2 and we didn't catch uninitialized tl somehow
206200
if (tl<0) internal_error(__func__, "tl of class is marked but tl<0"); // # nocov
207201
if (tl>0 && tl<l) internal_error(__func__, "tl (%d) < l (%d) but tl of class is marked", tl, l); // # nocov
@@ -251,11 +245,11 @@ SEXP shallowwrapper(SEXP dt, SEXP cols) {
251245
if (!selfrefok(dt, FALSE)) {
252246
int n = isNull(cols) ? length(dt) : length(cols);
253247
return(shallow(dt, cols, n));
254-
} else return(shallow(dt, cols, TRUELENGTH(dt)));
248+
} else return(shallow(dt, cols, R_maxLength(dt)));
255249
}
256250

257251
SEXP truelength(SEXP x) {
258-
return ScalarInteger(isNull(x) ? 0 : TRUELENGTH(x));
252+
return ScalarInteger(isNull(x) || !R_isResizable(x) ? 0 : R_maxLength(x));
259253
}
260254

261255
SEXP selfrefokwrapper(SEXP x, SEXP verbose) {
@@ -450,10 +444,10 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
450444
// modify DT by reference. Other than if new columns are being added and the allocVec() fails with
451445
// out-of-memory. In that case the user will receive hard halt and know to rerun.
452446
if (length(newcolnames)) {
453-
oldtncol = TRUELENGTH(dt); // TO DO: oldtncol can be just called tl now, as we won't realloc here any more.
447+
if (!R_isResizable(dt)) error(_("This data.table has either been loaded from disk (e.g. using readRDS()/load()) or constructed manually (e.g. using structure()). Please run setDT() or setalloccol() on it first (to pre-allocate space for new columns) before assigning by reference to it.")); // #2996
448+
oldtncol = R_maxLength(dt); // TO DO: oldtncol can be just called tl now, as we won't realloc here any more.
454449

455450
if (oldtncol<oldncol) {
456-
if (oldtncol==0) error(_("This data.table has either been loaded from disk (e.g. using readRDS()/load()) or constructed manually (e.g. using structure()). Please run setDT() or setalloccol() on it first (to pre-allocate space for new columns) before assigning by reference to it.")); // #2996
457451
internal_error(__func__, "oldtncol(%d) < oldncol(%d)", oldtncol, oldncol); // # nocov
458452
}
459453
if (oldtncol>oldncol+10000L) warning(_("truelength (%d) is greater than 10,000 items over-allocated (length = %d). See ?truelength. If you didn't set the datatable.alloccol option very large, please report to data.table issue tracker including the result of sessionInfo()."),oldtncol, oldncol);
@@ -463,9 +457,9 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
463457
error(_("It appears that at some earlier point, names of this data.table have been reassigned. Please ensure to use setnames() rather than names<- or colnames<-. Otherwise, please report to data.table issue tracker.")); // # nocov
464458
// Can growVector at this point easily enough, but it shouldn't happen in first place so leave it as
465459
// strong error message for now.
466-
else if (TRUELENGTH(names) != oldtncol)
460+
else if (R_maxLength(names) != oldtncol)
467461
// Use (long long) to cast R_xlen_t to a fixed type to robustly avoid -Wformat compiler warnings, see #5768, PRId64 didn't work
468-
internal_error(__func__, "selfrefnames is ok but tl names [%lld] != tl [%d]", (long long)TRUELENGTH(names), oldtncol); // # nocov
462+
internal_error(__func__, "selfrefnames is ok but maxLength(names) [%lld] != maxLength(dt) [%d]", (long long)R_maxLength(names), oldtncol); // # nocov
469463
if (!selfrefok(dt, verbose)) // #6410 setDT(dt) and subsequent attr<- can lead to invalid selfref
470464
error(_("It appears that at some earlier point, attributes of this data.table have been reassigned. Please use setattr(DT, name, value) rather than attr(DT, name) <- value. If that doesn't apply to you, please report your case to the data.table issue tracker."));
471465
R_resizeVector(dt, oldncol+LENGTH(newcolnames));

src/data.table.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@
104104
}
105105
# define R_duplicateAsResizable(x) R_duplicateAsResizable_(x)
106106
# define R_resizeVector(x, newlen) SETLENGTH(x, newlen)
107+
# define R_maxLength(x) TRUELENGTH(x)
108+
# define R_isResizable(x) ((bool)(IS_GROWABLE(x)))
107109
#endif
108110

109111
// init.c

0 commit comments

Comments
 (0)