Skip to content

Commit c7ac808

Browse files
lionel-lionel-
committed
Revert "shielding #789 behind a new #define to effectively disable it now"
This reverts commit 81c8339.
1 parent 6886a43 commit c7ac808

File tree

8 files changed

+43
-109
lines changed

8 files changed

+43
-109
lines changed

‎ChangeLog‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,35 @@
1+
2+
2018-05-31 Lionel Henry <lionel@rstudio.com>
3+
4+
* inst/include/Rcpp/api/meat/Rcpp_eval.h: Unguard Rcpp_fast_eval().
5+
6+
Unlike Rcpp_eval(), Rcpp_fast_eval() does not evaluate R code within
7+
tryCatch() in order to avoid the catching overhead. R longjumps are now
8+
correctly intercepted and rethrown. Following this change the C++ stack
9+
is now safely unwound when a longjump is detected while calling into R
10+
code. This includes the following cases: caught condition of any class,
11+
long return, restart jump, debugger exit.
12+
13+
Rcpp_eval() also uses the protect-unwind API under the hood in order to
14+
gain safety. It is fully backward-compatibile and still catches errors
15+
and interrupts to rethrow them as typed C++ exceptions. If you don't
16+
need to catch those, consider using Rcpp_fast_eval() instead to avoid
17+
the catching overhead.
18+
19+
These improvements are only available for R 3.5.0 and greater. You also
20+
need to explicitly define `RCPP_PROTECTED_EVAL` before including Rcpp.h.
21+
When compiled with old versions of R, Rcpp_fast_eval() always falls back
22+
to Rcpp_eval(). This is in contrast to internal::Rcpp_eval_impl() which
23+
falls back to Rf_eval() and which is used in performance-sensititive
24+
places.
25+
26+
Note that Rcpp_fast_eval() behaves a bit differently to Rcpp_eval(). The
27+
former has the semantics of the C function Rf_eval() whereas the latter
28+
behaves like the R function base::eval(). This has subtle implications
29+
for control flow. For instance evaluating a return() expression within a
30+
frame environment returns from that frame rather than from the
31+
Rcpp_eval() call.
32+
133
2018-05-09 Dirk Eddelbuettel <edd@debian.org>
234

335
* DESCRIPTION: Release 0.12.17

‎inst/include/Rcpp/Environment.h‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,7 @@ namespace Rcpp{
109109

110110
/* We need to evaluate if it is a promise */
111111
if( TYPEOF(res) == PROMSXP){
112-
#if defined(RCPP_USE_UNWIND_PROTECT)
113-
res = internal::Rcpp_eval_impl(res, env);
114-
#else
115-
res = Rf_eval(res, env);
116-
#endif
112+
res = internal::Rcpp_eval_impl( res, env ) ;
117113
}
118114
return res ;
119115
}
@@ -133,11 +129,7 @@ namespace Rcpp{
133129

134130
/* We need to evaluate if it is a promise */
135131
if( TYPEOF(res) == PROMSXP){
136-
#if defined(RCPP_USE_UNWIND_PROTECT)
137-
res = internal::Rcpp_eval_impl(res, env);
138-
#else
139-
res = Rf_eval(res, env);
140-
#endif
132+
res = internal::Rcpp_eval_impl( res, env ) ;
141133
}
142134
return res ;
143135
}
@@ -159,11 +151,7 @@ namespace Rcpp{
159151

160152
/* We need to evaluate if it is a promise */
161153
if( TYPEOF(res) == PROMSXP){
162-
#if defined(RCPP_USE_UNWIND_PROTECT)
163-
res = internal::Rcpp_eval_impl(res, env);
164-
#else
165-
res = Rf_eval(res, env);
166-
#endif
154+
res = internal::Rcpp_eval_impl( res, env ) ;
167155
}
168156
return res ;
169157
}
@@ -186,11 +174,7 @@ namespace Rcpp{
186174

187175
/* We need to evaluate if it is a promise */
188176
if( TYPEOF(res) == PROMSXP){
189-
#if defined(RCPP_USE_UNWIND_PROTECT)
190-
res = internal::Rcpp_eval_impl(res, env);
191-
#else
192-
res = Rf_eval(res, env);
193-
#endif
177+
res = internal::Rcpp_eval_impl( res, env ) ;
194178
}
195179
return res ;
196180
}

‎inst/include/Rcpp/Language.h‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,18 +145,10 @@ namespace Rcpp{
145145
}
146146

147147
SEXP fast_eval() const {
148-
#if defined(RCPP_USE_UNWIND_PROTECT)
149-
return internal::Rcpp_eval_impl( Storage::get__(), R_GlobalEnv);
150-
#else
151-
return Rf_eval(Storage::get__(), R_GlobalEnv);
152-
#endif
148+
return internal::Rcpp_eval_impl( Storage::get__(), R_GlobalEnv) ;
153149
}
154150
SEXP fast_eval(SEXP env ) const {
155-
#if defined(RCPP_USE_UNWIND_PROTECT)
156151
return internal::Rcpp_eval_impl( Storage::get__(), env) ;
157-
#else
158-
return Rf_eval(Storage::get__(), env);
159-
#endif
160152
}
161153

162154
void update( SEXP x){

‎inst/include/Rcpp/api/meat/Rcpp_eval.h‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,11 @@
2121
#include <Rcpp/Interrupt.h>
2222
#include <Rversion.h>
2323

24-
// outer definition from RcppCommon.h
25-
#if defined(RCPP_USE_UNWIND_PROTECT)
26-
#if (defined(RCPP_PROTECTED_EVAL) && defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0))
27-
// file-local and only used here
28-
#define RCPP_USE_PROTECT_UNWIND
29-
#endif
24+
#if (defined(RCPP_PROTECTED_EVAL) && defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0))
25+
#define RCPP_USE_PROTECT_UNWIND
3026
#endif
3127

28+
3229
namespace Rcpp {
3330
namespace internal {
3431

@@ -112,11 +109,7 @@ inline SEXP Rcpp_eval(SEXP expr, SEXP env) {
112109
SET_TAG(CDDR(call), ::Rf_install("error"));
113110
SET_TAG(CDDR(CDR(call)), ::Rf_install("interrupt"));
114111

115-
#if defined(RCPP_USE_UNWIND_PROTECT)
116-
Shield<SEXP> res(::Rf_eval(call, R_GlobalEnv)) // execute the call
117-
#else
118112
Shield<SEXP> res(internal::Rcpp_eval_impl(call, R_GlobalEnv));
119-
#endif
120113

121114
// check for condition results (errors, interrupts)
122115
if (Rf_inherits(res, "condition")) {
@@ -125,12 +118,7 @@ inline SEXP Rcpp_eval(SEXP expr, SEXP env) {
125118

126119
Shield<SEXP> conditionMessageCall(::Rf_lang2(::Rf_install("conditionMessage"), res));
127120

128-
#if defined(RCPP_USE_UNWIND_PROTECT)
129-
Shield<SEXP> conditionMessage(internal::Rcpp_eval_impl(conditionMessageCall,
130-
R_GlobalEnv));
131-
#else
132-
Shield<SEXP> conditionMessage(::Rf_eval(conditionMessageCall, R_GlobalEnv));
133-
#endif
121+
Shield<SEXP> conditionMessage(internal::Rcpp_eval_impl(conditionMessageCall, R_GlobalEnv));
134122
throw eval_error(CHAR(STRING_ELT(conditionMessage, 0)));
135123
}
136124

‎inst/include/Rcpp/exceptions.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ namespace Rcpp {
111111
throw Rcpp::exception(message.c_str());
112112
} // #nocov end
113113

114-
#if defined(RCPP_USE_UNWIND_PROTECT)
115114
namespace internal {
116115

117116
struct LongjumpException {
@@ -127,7 +126,6 @@ namespace Rcpp {
127126
}
128127

129128
} // namespace internal
130-
#endif
131129

132130
} // namespace Rcpp
133131

‎inst/include/Rcpp/macros/macros.h‎

Lines changed: 1 addition & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@
3636
#endif
3737

3838
#ifndef VOID_END_RCPP
39-
// longer form with Rcpp::internal::LongjumpException first, alternate below #else
40-
#if defined(RCPP_USE_UNWIND_PROTECT)
39+
#define VOID_END_RCPP \
4140
} \
4241
catch( Rcpp::internal::InterruptedException &__ex__) { \
4342
rcpp_output_type = 1 ; \
@@ -67,42 +66,13 @@
6766
SEXP expr = PROTECT( Rf_lang2( stop_sym , rcpp_output_condition ) ) ; \
6867
Rf_eval( expr, R_GlobalEnv ) ; \
6968
}
70-
#else
71-
#define VOID_END_RCPP \
72-
} \
73-
catch( Rcpp::internal::InterruptedException &__ex__) { \
74-
rcpp_output_type = 1 ; \
75-
} \
76-
catch(Rcpp::exception& __ex__) { \
77-
rcpp_output_type = 2 ; \
78-
rcpp_output_condition = PROTECT(rcpp_exception_to_r_condition(__ex__)) ; \
79-
} \
80-
catch( std::exception& __ex__ ){ \
81-
rcpp_output_type = 2 ; \
82-
rcpp_output_condition = PROTECT(exception_to_r_condition(__ex__)) ; \
83-
} \
84-
catch( ... ){ \
85-
rcpp_output_type = 2 ; \
86-
rcpp_output_condition = PROTECT(string_to_try_error("c++ exception (unknown reason)")) ; \
87-
} \
88-
if( rcpp_output_type == 1 ){ \
89-
Rf_onintr() ; \
90-
} \
91-
if( rcpp_output_type == 2 ){ \
92-
SEXP stop_sym = Rf_install( "stop" ) ; \
93-
SEXP expr = PROTECT( Rf_lang2( stop_sym , rcpp_output_condition ) ) ; \
94-
Rf_eval( expr, R_GlobalEnv ) ; \
95-
}
96-
#endif
9769
#endif
9870

9971
#ifndef END_RCPP
10072
#define END_RCPP VOID_END_RCPP return R_NilValue;
10173
#endif
10274

10375
#ifndef END_RCPP_RETURN_ERROR
104-
// longer form with Rcpp::internal::LongjumpException first, alternate below #else
105-
#if defined(RCPP_USE_UNWIND_PROTECT)
10676
#define END_RCPP_RETURN_ERROR \
10777
} \
10878
catch (Rcpp::internal::InterruptedException &__ex__) { \
@@ -119,20 +89,6 @@
11989
return string_to_try_error("c++ exception (unknown reason)"); \
12090
} \
12191
return R_NilValue;
122-
#else
123-
#define END_RCPP_RETURN_ERROR \
124-
} \
125-
catch (Rcpp::internal::InterruptedException &__ex__) { \
126-
return Rcpp::internal::interruptedError(); \
127-
} \
128-
catch (std::exception &__ex__) { \
129-
return exception_to_try_error(__ex__); \
130-
} \
131-
catch (...) { \
132-
return string_to_try_error("c++ exception (unknown reason)"); \
133-
} \
134-
return R_NilValue;
135-
#endif
13692
#endif
13793

13894
#define Rcpp_error(MESSAGE) throw Rcpp::exception(MESSAGE, __FILE__, __LINE__)

‎inst/include/RcppCommon.h‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,6 @@
2626
// #define RCPP_DEBUG_LEVEL 1
2727
// #define RCPP_DEBUG_MODULE_LEVEL 1
2828

29-
// PR #798 by Lionel seems to have created some side-effects possibly related to
30-
// UnwinProtect is currently implement in R-devel. This #define needs to be set to
31-
// enable it, in most cases you want to be disabled.
32-
// #define RCPP_USE_UNWIND_PROTECT 1
33-
// so here _explicitly_ disable it for now
34-
#ifdef RCPP_USE_UNWIND_PROTECT
35-
#undef RCPP_USE_UNWIND_PROTECT
36-
#endif
37-
3829
#include <Rcpp/r/headers.h>
3930

4031
/**
@@ -83,10 +74,9 @@ namespace Rcpp {
8374

8475
namespace Rcpp {
8576

77+
SEXP Rcpp_fast_eval(SEXP expr_, SEXP env = R_GlobalEnv);
8678
SEXP Rcpp_eval(SEXP expr_, SEXP env = R_GlobalEnv);
8779

88-
// from PR#789
89-
SEXP Rcpp_fast_eval(SEXP expr_, SEXP env = R_GlobalEnv);
9080
namespace internal {
9181
SEXP Rcpp_eval_impl(SEXP expr, SEXP env = R_GlobalEnv);
9282
}

‎inst/unitTests/runit.stack.R‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@
1818
# along with Rcpp. If not, see <http://www.gnu.org/licenses/>.
1919

2020
.runThisTest <- Sys.getenv("RunAllRcppTests") == "yes"
21-
.onLinux <- .Platform$OS.type == "unix" && unname(Sys.info()["sysname"]) == "Linux"
22-
23-
## As of release 0.12.15, the stack unwinding is experimental and not used
24-
## See the #define in RcppCommon.h to change it
25-
26-
.runThisTest <- FALSE
2721

2822

2923
if (.runThisTest) {

0 commit comments

Comments
 (0)