paleolimbot commented on code in PR #14271:
URL: https://github.com/apache/arrow/pull/14271#discussion_r989169036
##########
r/src/altrep.cpp:
##########
@@ -800,41 +864,35 @@ struct AltrepVectorString : public
AltrepVectorBase<AltrepVectorString<Type>> {
return Representation(alt);
}
- BEGIN_CPP11
-
const auto& chunked_array = GetChunkedArray(alt);
SEXP data2 = PROTECT(Rf_allocVector(STRSXP, chunked_array->length()));
MARK_NOT_MUTABLE(data2);
- RStringViewer r_string_viewer;
+ R_xlen_t i = 0;
+ RStringViewer& r_string_viewer = string_viewer();
+ r_string_viewer.reset_null_was_stripped();
+ r_string_viewer.set_strip_out_nuls(GetBoolOption("arrow.skip_nul", false));
+ for (const auto& array : chunked_array->chunks()) {
+ r_string_viewer.SetArray(array);
- // r_string_viewer.Convert() might jump so we have to
- // wrap it in unwind_protect() to:
- // - correctly destruct the C++ objects
- // - resume the unwinding
- cpp11::unwind_protect([&]() {
- R_xlen_t i = 0;
- for (const auto& array : chunked_array->chunks()) {
- r_string_viewer.SetArray(array);
-
- auto ni = array->length();
- for (R_xlen_t j = 0; j < ni; j++, i++) {
- SET_STRING_ELT(data2, i, r_string_viewer.Convert(j));
- }
+ auto ni = array->length();
+ for (R_xlen_t j = 0; j < ni; j++, i++) {
+ SET_STRING_ELT(data2, i, r_string_viewer.Convert(j));
Review Comment:
`Convert()` definitely still jumps...the gist of this change is to make the
entire frame jump-safe (to my understanding, this means everything is trivially
destructible). That doesn't mean I didn't miss something (or that my
understanding of this is incorrect!).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
