Skip to content

Use lsInteral3 instead of lsInternal #1362

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

Merged
merged 2 commits into from
Mar 16, 2025
Merged

Conversation

eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Mar 13, 2025

This came to our attention via a CC in email from @kurthornik in the context of helping the @RfastOfficial team. The change is benign as R has Rf_lsInternal3 (with three arguments, we were not using the third for 'sorted or not') and asking 'blame' reveals this signature is at least 11 years old so we do not need a version check methinks.

This change is so cosmetic and we are not changing exposed APIs here so I may for once not bother to wait for the 2 1/2 days of a full reverse dependency check. (And the first batch of CI from the commit before the PR was formed is all green too.)

Checklist

  • Code compiles correctly
  • R CMD check still passes all tests
  • Preferably, new tests were added which fail without the change
  • Document the changes by file in ChangeLog

Copy link
Contributor

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@eddelbuettel
Copy link
Member Author

In that context just spotted section 'Some Backports' in the R-devel version of WRE. We may want to borrow this as needed:

#if R_VERSION < R_Version(4, 4, 1)
#define allocLang Rf_allocLang

SEXP Rf_allocLang(int n)
{
    if (n > 0)
          return LCONS(R_NilValue, Rf_allocList(n - 1));
    else
          return R_NilValue;
}
#endif

#if R_VERSION < R_Version(4, 5, 0)
# define isDataFrame(x) Rf_isFrame(x)
# define R_ClosureFormals(x) FORMALS(x)
# define R_ClosureEnv(x) CLOENV(x)
# define R_ParentEnv(x) ENCLOS(x)

SEXP R_mkClosure(SEXP formals, SEXP body, SEXP env)
{
    SEXP fun = Rf_allocSExp(CLOSXP);
    SET_FORMALS(fun, formals);
    SET_BODY(fun, body);
    SET_CLOENV(fun, env);
    return fun;
}

void CLEAR_ATTRIB(SEXP x)
{
    SET_ATTRIB(x, R_NilValue);
    SET_OBJECT(x, 0);
    UNSET_S4_OBJECT(x);
}
#endif

@eddelbuettel
Copy link
Member Author

I turned the rev.dep beast on ("old habits die hard") but may not wait for its conclusion.

@eddelbuettel
Copy link
Member Author

Rev.dep is a quarter done and no issues. I let it run a little longer but may merge before it is done and then rebase the other branch too.

@eddelbuettel eddelbuettel merged commit 0905d92 into master Mar 16, 2025
18 checks passed
@eddelbuettel eddelbuettel deleted the feature/lsInternal branch March 16, 2025 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants