-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-43194: [R] R_existsVarInFrame isn't available earlier than R 4.2 #43243
Conversation
@github-actions crossbow submit test-r-versions |
|
Revision: 7414f23 Submitted crossbow builds: ursacomputing/crossbow @ actions-edac7fb428
|
@github-actions crossbow submit test-r-rstudio-r-base-4.1-opensuse155 r-binary-packages |
Revision: 7414f23 Submitted crossbow builds: ursacomputing/crossbow @ actions-fa92c5b35b
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; I see the job failing on the nightlies but passing above after this change here.
Do we want to cherry-pick this into the release?
Revision: 7414f23 Submitted crossbow builds: ursacomputing/crossbow @ actions-4dc7fa4731
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 0372617. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…43243) ### Rationale for this change `R_existsVarInFrame` doesn't exist before R 4.2, so we need to fall back to `Rf_findVarInFrame3` if it is not defined. Resolves #43194 ### What changes are included in this PR? `ifdef`s ### Are these changes tested? Yes, in our extended CI `test-r-versions`, `test-r-rstudio-r-base-4.1-opensuse155` ### Are there any user-facing changes? No * GitHub Issue: #43194 Authored-by: Jonathan Keane <[email protected]> Signed-off-by: Jonathan Keane <[email protected]>
A follow on to #43243 `#ifdef function declaration` does not work, instead we must use the `#if (R_VERSION >= R_Version(4, 2, 0))` pattern to only include code for specific versions. Lead-authored-by: Jonathan Keane <[email protected]> Co-authored-by: Neal Richardson <[email protected]> Signed-off-by: Jonathan Keane <[email protected]>
A follow on to #43243 `#ifdef function declaration` does not work, instead we must use the `#if (R_VERSION >= R_Version(4, 2, 0))` pattern to only include code for specific versions. Lead-authored-by: Jonathan Keane <[email protected]> Co-authored-by: Neal Richardson <[email protected]> Signed-off-by: Jonathan Keane <[email protected]>
Rationale for this change
R_existsVarInFrame
doesn't exist before R 4.2, so we need to fall back toRf_findVarInFrame3
if it is not defined.Resolves #43194
What changes are included in this PR?
ifdef
sAre these changes tested?
Yes, in our extended CI
test-r-versions
,test-r-rstudio-r-base-4.1-opensuse155
Are there any user-facing changes?
No
R_existsVarInFrame
isn't available earlier than R 4.2 #43194