Skip to content

Fix R_UnboundValue NOTE #1466

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

Open
Enchufa2 wants to merge 6 commits into
base: master
Choose a base branch
from
+41 −63
Open

Fix R_UnboundValue NOTE #1466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
44e6a1a
remove unneeded check using non-API R_UnboundValue
Enchufa2 Mar 29, 2026
ebf1f89
use R_NilValue with R_getVarEx to avoid non-API R_UnboundValue + some…
Enchufa2 Mar 29, 2026
e904602
use R_NilValue instead of non-API R_UnboundValue
Enchufa2 Mar 29, 2026
d6b08fc
use R_BindingTypeUnbound instead of non-API R_UnboundValue
Enchufa2 Mar 29, 2026
505eb9c
update ChangeLog
Enchufa2 Mar 29, 2026
3f35dbb
only need to check for PROMSXP in the old path
Enchufa2 Mar 29, 2026
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
8 changes: 8 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
2026-03-29 Iñaki Ucar <[email protected]>

* inst/include/Rcpp/api/meat/Rcpp_eval.h: Remove check for non-API
R_UnboundValue, which is never returned anyway from Rf_findFun
* inst/include/Rcpp/Function.h: Use alternative to R_UnboundValue
* inst/include/Rcpp/Promise.h: Idem
* inst/include/Rcpp/Environment.h: Idem + some refactoring

2026-03-26 Dirk Eddelbuettel <[email protected]>

* inst/bib/Rcpp.bib: Refreshed a few more references
Expand Down
69 changes: 18 additions & 51 deletions inst/include/Rcpp/Environment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// Environment.h: Rcpp R/C++ interface class library -- access R environments
//
// Copyright (C) 2009-2013 Dirk Eddelbuettel and Romain François
// Copyright (C) 2014-2026 Dirk Eddelbuettel, Romain François and Kevin Ushey
// Copyright (C) 2014-2025 Dirk Eddelbuettel, Romain François and Kevin Ushey
// Copyright (C) 2026 Dirk Eddelbuettel, Romain François, Kevin Ushey and Iñaki Ucar
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -95,20 +96,8 @@ namespace Rcpp{
* @return a SEXP (possibly R_NilValue)
*/
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 */
if( TYPEOF(res) == PROMSXP){
res = internal::Rcpp_eval_impl( res, env ) ; // #nocov
}
return res ;
Symbol nameSym = Rf_install(name.c_str());
return get(nameSym);
Copy link
Copy Markdown
Member

@eddelbuettel eddelbuettel Mar 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

That simplification seems almost too good to be true. Why did we not do that earlier? 😆

}

/**
Expand All @@ -122,16 +111,12 @@ namespace Rcpp{
SEXP env = Storage::get__() ;
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVarInFrame( env, name ) ;
if (res == R_UnboundValue) return R_NilValue;
if (TYPEOF(res) == PROMSXP)
res = internal::Rcpp_eval_impl(res, env);
#else
SEXP res = R_getVarEx(name, env, FALSE, R_UnboundValue);
SEXP res = R_getVarEx(name, env, FALSE, R_NilValue);
#endif

if( res == R_UnboundValue ) return R_NilValue ;

/* We need to evaluate if it is a promise */
if( TYPEOF(res) == PROMSXP){
res = internal::Rcpp_eval_impl( res, env ) ;
}
return res ;
}

Expand All @@ -144,21 +129,8 @@ 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) ;

/* We need to evaluate if it is a promise */
if( TYPEOF(res) == PROMSXP){
res = internal::Rcpp_eval_impl( res, env ) ;
}
return res ;
Symbol nameSym = Rf_install(name.c_str());
return find(nameSym);
}

/**
Expand All @@ -171,19 +143,13 @@ namespace Rcpp{
SEXP env = Storage::get__() ;
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVar( name, env ) ;
if (res == R_UnboundValue) throw binding_not_found(name.c_str());
if (TYPEOF(res) == PROMSXP)
res = internal::Rcpp_eval_impl(res, env);
#else
SEXP res = R_getVarEx(name, env, TRUE, R_UnboundValue);
SEXP res = R_getVarEx(name, env, TRUE, R_NilValue);
if (res == R_NilValue) throw binding_not_found(name.c_str());
#endif
if( res == R_UnboundValue ) {
// Pass on the const char* to the RCPP_EXCEPTION_CLASS's
// const std::string& requirement
throw binding_not_found(name.c_str()) ;
}

/* We need to evaluate if it is a promise */
if( TYPEOF(res) == PROMSXP){
res = internal::Rcpp_eval_impl( res, env ) ;
}
return res ;
}

Expand All @@ -199,10 +165,11 @@ namespace Rcpp{
SEXP nameSym = Rf_install(name.c_str());
#if R_VERSION < R_Version(4,5,0)
SEXP res = Rf_findVarInFrame( Storage::get__() , nameSym ) ;
return res != R_UnboundValue;
#else
SEXP res = R_getVarEx(nameSym, Storage::get__(), FALSE, R_UnboundValue);
SEXP res = R_getVarEx(nameSym, Storage::get__(), FALSE, R_NilValue);
return res != R_NilValue;
#endif
return res != R_UnboundValue ;
}

/**
Expand Down
7 changes: 4 additions & 3 deletions inst/include/Rcpp/Function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@

// Function.h: Rcpp R/C++ interface class library -- functions (also primitives and builtins)
//
// Copyright (C) 2010 - 2026 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2010 - 2025 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2026 Dirk Eddelbuettel, Romain François and Iñaki Ucar
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -72,9 +73,9 @@ namespace Rcpp{
#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));
Shield<SEXP> env(R_getVarEx(Rf_install(ns.c_str()), R_NamespaceRegistry, FALSE, R_NilValue));
#endif
if (env == R_UnboundValue) {
if (env == R_NilValue) {
stop("there is no namespace called \"%s\"", ns);
}
get_function(name, env);
Expand Down
14 changes: 10 additions & 4 deletions inst/include/Rcpp/Promise.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
//
// Promise.h: Rcpp R/C++ interface class library -- promises (PROMSXP)
//
// Copyright (C) 2010 - 2013 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2010 - 2025 Dirk Eddelbuettel and Romain François
// Copyright (C) 2026 Dirk Eddelbuettel, Romain François and Iñaki Ucar
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -48,13 +49,18 @@ namespace Rcpp{
* Return the result of the PRVALUE macro on the promise
*/
SEXP value() const{
SEXP val = PRVALUE( Storage::get__() ) ;
if( val == R_UnboundValue ) throw unevaluated_promise() ;
return val ;
if (!was_evaluated()) throw unevaluated_promise();
return PRVALUE(Storage::get__());
}

bool was_evaluated() const {
#if R_VERSION < R_Version(4,6,0)
return PRVALUE(Storage::get__()) != R_UnboundValue ;
#else
SEXP env = environment();
R_BindingType_t bt = R_GetBindingType(Storage::get__(), env);
return bt != R_BindingTypeUnbound;
Copy link
Copy Markdown
Member

@eddelbuettel eddelbuettel Mar 29, 2026

Choose a reason for hiding this comment

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

Nice, and legit per WRE's r-devel version

#endif
}

/**
Expand Down
6 changes: 1 addition & 5 deletions inst/include/Rcpp/api/meat/Rcpp_eval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright (C) 2013 - 2025 Romain Francois
// Copyright (C) 2026 Romain Francois and Dirk Eddelbuettel
// Copyright (C) 2026 Romain Francois, Dirk Eddelbuettel and Iñaki Ucar
//
// This file is part of Rcpp.
//
Expand Down Expand Up @@ -56,10 +56,6 @@ inline SEXP Rcpp_eval(SEXP expr, SEXP env) {
// 'identity' function used to capture errors, interrupts
Shield<SEXP> identity(Rf_findFun(::Rf_install("identity"), R_BaseNamespace));

if (identity == R_UnboundValue) {
stop("Failed to find 'base::identity()'");
}

Copy link
Copy Markdown
Member

@eddelbuettel eddelbuettel Mar 29, 2026

Choose a reason for hiding this comment

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

Just for kicks: when/why did we need this? When could that have been true?

Copy link
Copy Markdown
Member Author

@Enchufa2 Enchufa2 Mar 29, 2026

Choose a reason for hiding this comment

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

git-blame says that this is from 20 years ago, and Rcpp is 18 years old, so... never.

Copy link
Copy Markdown
Member

@eddelbuettel eddelbuettel Mar 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

No, it says @kevinushey did this in 2015:

454de74f5 (Kevin Ushey       2015-07-13 17:45:01 -0700 59)     if (identity == R_UnboundValue) {
454de74f5 (Kevin Ushey       2015-07-13 17:45:01 -0700 60)         stop("Failed to find 'base::identity()'");
1c90562c8 (Kevin Ushey       2015-07-13 15:43:09 -0700 61)     }

(i.e. I was asking why we added this / why we can now simply remove it).

// define the evalq call -- the actual R evaluation we want to execute
Shield<SEXP> evalqCall(Rf_lang3(::Rf_install("evalq"), expr, env));

Expand Down
Loading