Skip to content

Commit

Permalink
ARROW-18174: [R] Fix compile of altrep.cpp on some builds (apache#14530)
Browse files Browse the repository at this point in the history
After ARROW-17187 (apache#14271), we get at least two nightly build failures because of how cpp11 objects are constructed in some new test functions. This PR makes the constructors less ambiguous so that no compilers give us trouble!

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
  • Loading branch information
paleolimbot authored Oct 31, 2022
1 parent c7d9730 commit e91d228
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 16 deletions.
26 changes: 13 additions & 13 deletions r/src/altrep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1098,9 +1098,9 @@ void test_arrow_altrep_set_string_elt(sexp x, int i, std::string value) {
}

// [[arrow::export]]
logicals test_arrow_altrep_is_materialized(sexp x) {
sexp test_arrow_altrep_is_materialized(sexp x) {
if (!is_arrow_altrep(x)) {
return logicals({NA_LOGICAL});
return Rf_ScalarLogical(NA_LOGICAL);
}

sexp data_class_sym = CAR(ATTRIB(ALTREP_CLASS(x)));
Expand All @@ -1120,7 +1120,7 @@ logicals test_arrow_altrep_is_materialized(sexp x) {
result = arrow::r::altrep::AltrepFactor::IsMaterialized(x);
}

return logicals({result});
return Rf_ScalarLogical(result);
}

// [[arrow::export]]
Expand Down Expand Up @@ -1163,19 +1163,19 @@ sexp test_arrow_altrep_copy_by_element(sexp x) {
R_xlen_t n = Rf_xlength(x);

if (TYPEOF(x) == INTSXP) {
writable::integers out(Rf_xlength(x));
cpp11::writable::integers out(Rf_xlength(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = INTEGER_ELT(x, i);
}
return out;
} else if (TYPEOF(x) == REALSXP) {
writable::doubles out(Rf_xlength(x));
cpp11::writable::doubles out(Rf_xlength(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = REAL_ELT(x, i);
}
return out;
} else if (TYPEOF(x) == STRSXP) {
writable::strings out(Rf_xlength(x));
cpp11::writable::strings out(Rf_xlength(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = STRING_ELT(x, i);
}
Expand All @@ -1194,8 +1194,8 @@ sexp test_arrow_altrep_copy_by_region(sexp x, R_xlen_t region_size) {
R_xlen_t n = Rf_xlength(x);

if (TYPEOF(x) == INTSXP) {
writable::integers out(Rf_xlength(x));
writable::integers buf_shelter(region_size);
cpp11::writable::integers out(Rf_xlength(x));
cpp11::writable::integers buf_shelter(region_size);
int* buf = INTEGER(buf_shelter);
for (R_xlen_t i = 0; i < n; i++) {
if ((i % region_size) == 0) {
Expand All @@ -1205,8 +1205,8 @@ sexp test_arrow_altrep_copy_by_region(sexp x, R_xlen_t region_size) {
}
return out;
} else if (TYPEOF(x) == REALSXP) {
writable::doubles out(Rf_xlength(x));
writable::doubles buf_shelter(region_size);
cpp11::writable::doubles out(Rf_xlength(x));
cpp11::writable::doubles buf_shelter(region_size);
double* buf = REAL(buf_shelter);
for (R_xlen_t i = 0; i < n; i++) {
if ((i % region_size) == 0) {
Expand All @@ -1229,21 +1229,21 @@ sexp test_arrow_altrep_copy_by_dataptr(sexp x) {
R_xlen_t n = Rf_xlength(x);

if (TYPEOF(x) == INTSXP) {
writable::integers out(Rf_xlength(x));
cpp11::writable::integers out(Rf_xlength(x));
int* ptr = reinterpret_cast<int*>(DATAPTR(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = ptr[i];
}
return out;
} else if (TYPEOF(x) == REALSXP) {
writable::doubles out(Rf_xlength(x));
cpp11::writable::doubles out(Rf_xlength(x));
double* ptr = reinterpret_cast<double*>(DATAPTR(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = ptr[i];
}
return out;
} else if (TYPEOF(x) == STRSXP) {
writable::strings out(Rf_xlength(x));
cpp11::writable::strings out(Rf_xlength(x));
SEXP* ptr = reinterpret_cast<SEXP*>(DATAPTR(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = ptr[i];
Expand Down
2 changes: 1 addition & 1 deletion r/src/arrowExports.cpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions r/src/compute-exec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,9 @@ class ExecPlanReader : public arrow::RecordBatchReader {
}

plan_status_ = PLAN_FINISHED;
plan_.reset();
sink_gen_ = arrow::MakeEmptyGenerator<std::optional<compute::ExecBatch>>();
// A previous version of this called plan_.reset() and reset
// sink_gen_ to an empty generator; however, this caused
// crashes on some platforms.
}
};

Expand Down

0 comments on commit e91d228

Please sign in to comment.