GH-44924: [R] Remove usage of cpp11's HAS_UNWIND_PROTECT #45261
Conversation
| bool CanRunWithCapturedR() { | ||
| #if defined(HAS_UNWIND_PROTECT) | ||
| return MainRThread::GetInstance().Executor() == nullptr; | ||
| #else | ||
| return false; | ||
| #endif |
There was a problem hiding this comment.
Do we need this function at all anymore? Or another way: does MainRThread::GetInstance().Executor() == nullptr ever return false?
There was a problem hiding this comment.
🤷 I have no idea tbh. Just following the prompt from the cpp11 author ^^
There was a problem hiding this comment.
ah, the header explains it, I'll update the comment but from how I read it we still need to check for the executor
// Unwind protection was added in R 3.5 and some calls here use it
// and crash R in older versions (ARROW-16201). Implementation provided
// in safe-call-into-r-impl.cpp so that we can skip some tests
// when this feature is not provided. This also checks that there
// is not already an event loop registered (via MainRThread::Executor()),
// because only one of these can exist at any given time.
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
github-actions
bot
added
awaiting merge
Rationale for this change
The macro is no longer required on R >= 4.0 which is our minimum version.
What changes are included in this PR?
Remove use of HAS_UNWIND_PROTECT
Are these changes tested?
ci
Are there any user-facing changes?
no
HAS_UNWIND_PROTECT#44924