Skip to content

Switch from Rf_findVarInFrame to R_getVarEx #1423

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.

Sign up for GitHub

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
eddelbuettel merged 3 commits into from
Dec 16, 2025
+39 −10
Merged

Switch from Rf_findVarInFrame to R_getVarEx #1423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3e5a1c
Switch from Rf_findVarInFrame to R_getVarEx
eddelbuettel Dec 12, 2025
daeaa11
Micro whitespace edit following rebase [ci skip]
eddelbuettel Dec 12, 2025
3f4ff02
Adjust two 'inherits'-alike arguments following review comments
eddelbuettel Dec 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
* inst/tinytest/test_system.R: Wrap suppressMessages() around three
tests for long-obsolete linker and compiler flags

* inst/include/Rcpp/Environment.h: Replace use of Rf_findVarInFrame
with R_getVarEx (or R_getVar) if R 4.5.0 or later is used
* inst/include/Rcpp/Function.h: Idem
* src/barrier.cpp: Idem

2025-12-10 Dirk Eddelbuettel <edd@debian.org>

* DESCRIPTION (Version, Date): Roll micro version and date
Expand Down
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: Rcpp
Title: Seamless R and C++ Integration
Version: 1.1.0.9
Date: 2025-12-10
Date: 2025-12-13
Authors@R: c(person("Dirk", "Eddelbuettel", role = c("aut", "cre"), email = "edd@debian.org",
comment = c(ORCID = "0000-0001-6419-907X")),
person("Romain", "Francois", role = "aut",
Expand Down
27 changes: 22 additions & 5 deletions inst/include/Rcpp/Environment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
//

// Environment.h: Rcpp R/C++ interface class library -- access R environments
//
// Copyright (C) 2009 - 2013 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2014 - 2020 Dirk Eddelbuettel, Romain Francois and Kevin Ushey
// Copyright (C) 2014 - 2025 Dirk Eddelbuettel, Romain Francois and Kevin Ushey
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -98,8 +97,11 @@ namespace Rcpp{
SEXP get(const std::string& name) const {
SEXP env = Storage::get__() ;
SEXP nameSym = Rf_install(name.c_str());
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVarInFrame( env, nameSym ) ;

#else
SEXP res = R_getVarEx(nameSym, env, FALSE, R_UnboundValue);
#endif
if( res == R_UnboundValue ) return R_NilValue ;

/* We need to evaluate if it is a promise */
Expand All @@ -118,7 +120,11 @@ namespace Rcpp{
*/
SEXP get(Symbol name) const {
SEXP env = Storage::get__() ;
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVarInFrame( env, name ) ;
#else
SEXP res = R_getVarEx(name, env, FALSE, R_UnboundValue);
#endif

if( res == R_UnboundValue ) return R_NilValue ;

Expand All @@ -140,7 +146,11 @@ namespace Rcpp{
SEXP find( const std::string& name) const{
SEXP env = Storage::get__() ;
SEXP nameSym = Rf_install(name.c_str());
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVar( nameSym, env ) ;
#else
SEXP res = R_getVarEx(nameSym, env, TRUE, R_UnboundValue);
#endif

if( res == R_UnboundValue ) throw binding_not_found(name) ;

Expand All @@ -159,8 +169,11 @@ namespace Rcpp{
*/
SEXP find(Symbol name) const{
SEXP env = Storage::get__() ;
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVar( name, env ) ;

#else
SEXP res = R_getVarEx(name, env, TRUE, R_UnboundValue);
#endif
if( res == R_UnboundValue ) {
// Pass on the const char* to the RCPP_EXCEPTION_CLASS's
// const std::string& requirement
Expand All @@ -184,7 +197,11 @@ namespace Rcpp{
*/
bool exists( const std::string& name ) const {
SEXP nameSym = Rf_install(name.c_str());
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVarInFrame( Storage::get__() , nameSym ) ;
#else
SEXP res = R_getVarEx(nameSym, Storage::get__(), FALSE, R_UnboundValue);
#endif
return res != R_UnboundValue ;
}

Expand Down
9 changes: 6 additions & 3 deletions inst/include/Rcpp/Function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
//

// Function.h: Rcpp R/C++ interface class library -- functions (also primitives and builtins)
//
// Copyright (C) 2010 - 2013 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2010 - 2025 Dirk Eddelbuettel and Romain Francois
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -70,7 +69,11 @@ namespace Rcpp{
}

Function_Impl(const std::string& name, const std::string& ns) {
#if R_VERSION < R_Version(4,5,0)
Shield<SEXP> env(Rf_findVarInFrame(R_NamespaceRegistry, Rf_install(ns.c_str())));
#else
Shield<SEXP> env(R_getVarEx(Rf_install(ns.c_str()), R_NamespaceRegistry, FALSE, R_UnboundValue));
#endif
if (env == R_UnboundValue) {
stop("there is no namespace called \"%s\"", ns);
}
Expand Down
6 changes: 5 additions & 1 deletion src/barrier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// barrier.cpp: Rcpp R/C++ interface class library -- write barrier
//
// Copyright (C) 2010 - 2020 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2021 - 2022 Dirk Eddelbuettel, Romain Francois and Iñaki Ucar
// Copyright (C) 2021 - 2025 Dirk Eddelbuettel, Romain Francois and Iñaki Ucar
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -150,7 +150,11 @@ SEXP get_rcpp_cache() {
Rcpp::Shield<SEXP> call(Rf_lang2(getNamespaceSym, RcppString));
Rcpp::Shield<SEXP> RCPP(Rf_eval(call, R_GlobalEnv));

#if R_VERSION < R_Version(4,5,0)
Rcpp_cache = Rf_findVarInFrame(RCPP, Rf_install(".rcpp_cache"));
#else
Rcpp_cache = R_getVar(Rf_install(".rcpp_cache"), RCPP, TRUE);
Copy link
Contributor

@kevinushey kevinushey Dec 13, 2025

Choose a reason for hiding this comment

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

Suggested change
Rcpp_cache = R_getVar(Rf_install(".rcpp_cache"), RCPP, TRUE);
Rcpp_cache = R_getVar(Rf_install(".rcpp_cache"), RCPP, FALSE);

Copy link
Member Author

@eddelbuettel eddelbuettel Dec 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This one I was more torn about. Note the comment just above the code:

        /**
         * Get an object from the environment or one of its
         * parents
         *
         * @param name name of the object
         *
         */
        SEXP find( const std::string& name) const{

So does is walk through the other envs?

Also note that lines 139 to 162 (for find(std::string)) and 164 to 188 (for find(Symbol)) should be symmetric. Both TRUE, or both FALSE?

Copy link
Contributor

@kevinushey kevinushey Dec 15, 2025

Choose a reason for hiding this comment

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

I'm not sure if I'm missing something, but isn't this code just trying to get a reference to the .rcpp_cache variable in the Rcpp namespace?

Copy link
Contributor

@kevinushey kevinushey Dec 15, 2025

Choose a reason for hiding this comment

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

I think the prior edits are preserving that behavior you suggest, though -- get => inherits = FALSE; find => inherits = TRUE.

Copy link
Member Author

@eddelbuettel eddelbuettel Dec 15, 2025

Choose a reason for hiding this comment

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

I think that is where we are at right now (i.e.: get has a FALSE, find has a TRUE) and that is due to one correction made after your comment made days ago (as noted above). And that seems fine.

(The 'just look for .rcpp_cache' is in a different file and seemingly a debugging-only helper.)

#endif
Rcpp_cache_know = true;
}
return Rcpp_cache;
Expand Down