Στις Τετ 10 Οκτ 2012, ο/η Dirk Eddelbuettel έγραψε:
> There is a boolean in RInside.cpp that you need to flip. Eventually, we
> could (if it is useful) support a flag at instantiation of the RObject.
[snip]
> | I browsed the code, and a few lines down I see:
> |
> | case PARSE_ERROR:
> | Rf_error("Parse Error: \"%s\"\n", line.c_str());
> | UNPROTECT(2);
> | return 1;
> | break;
> |
> | thus it appears that parse errors are already handled. Even better:
> | handled with a return code (I don't like exceptions that much). So I
> | guess the only problem is that we're not in interactive mode, correct?
>
> In theory. In practice, I think, it just doesn't get there.
>
> | What kind of work/extensions do you suggest this code needs? (I could
> | try to work a little on that, and learn a little about the insides of
> | R in the process).
>
> I think it is pretty much this switch block we need to get to work
> properly.
>
> Dirk
Hi Dirk,
I studied RInside.cpp, made a few trial-and-error tests, and it appears to
work for me now - parseEval() doesn't crash when fed with syntactically
incorrect R code.
I am attaching a patch I made with quilt (interactive.diff), so you can see
what I did.
Turns out, that boolean you were talking about (on line 152) did no good.
Leaving it to false results in a crash ("Execution halted"), switching it to
true results in a crash plus a segmentation fault!
Turns out also that the switch statement is indeed evaluated. It gets there.
What halts execution is indeed the Rf_error() function; changing that to
Rf_warning() solves the problem, and allows parseEval() to return 1 and
parseEvalQ() to throw its exception.
One also needs to call mb_m.rewind() afterwards, or the MemBuf is not cleared
and subsequent calls to parseEval() fail as well.
As a result I added a boolean to RInside.h (interactiveMode) along with a
setter function (setInteractive(bool)), in order to enable this behaviour
instead of the old one.
Now it works perfectly for me!
As an alternative, we could get rid of the boolean altogether and also get rid
of the Rf_error() or Rf_warning(). I don't think it can ever be good to "halt
execution" when dealing with syntactically incorrect R code. Returning an
error code or throwing an exception is better. And the Rf_warning() is also
unnecessary, if case PARSE_NULL: and case PARSE_ERROR: return their own error
code (eg. 2 and 3). No code using RInside should be affected by this, since
until now the error code was not returned and execution was halted at
Rf_error().
So if you aggree with that, you can apply instead the other patch I'm
attaching (simplified.diff). I guess that's the best solution.
Hope this helps a bit! It sure solved my problem.
Regards,
Theodore
Index: RInside/inst/include/RInside.h
===================================================================
--- RInside.orig/inst/include/RInside.h 2010-07-09 23:59:14.000000000 +0300
+++ RInside/inst/include/RInside.h 2012-10-10 19:25:37.302217289 +0300
@@ -32,6 +32,7 @@
Rcpp::Environment global_env ;
bool verbose_m; // private switch
+ bool interactiveMode; // if true, parseEval() will not cause crash when evaluating a line with syntax errors
void init_tempdir(void);
void init_rand(void);
@@ -56,6 +57,8 @@
int parseEval(const std::string & line, SEXP &ans); // parse line, return in ans; error code rc
void parseEvalQ(const std::string & line); // parse line, no return (throws on error)
+ void setInteractive(const bool interactive) { interactiveMode = interactive; }
+
class Proxy {
public:
Proxy(SEXP xx): x(xx) { };
Index: RInside/src/RInside.cpp
===================================================================
--- RInside.orig/src/RInside.cpp 2012-09-08 04:00:46.000000000 +0300
+++ RInside/src/RInside.cpp 2012-10-10 19:30:08.427279004 +0300
@@ -57,7 +57,7 @@
RInside::RInside()
#ifdef RINSIDE_CALLBACKS
- : callbacks(0)
+ : interactiveMode(false), callbacks(0)
#endif
{
initialize( 0, 0, false );
@@ -99,7 +99,7 @@
RInside::RInside(const int argc, const char* const argv[], const bool loadRcpp)
#ifdef RINSIDE_CALLBACKS
-: callbacks(0)
+: interactiveMode(false), callbacks(0)
#endif
{
initialize( argc, argv, loadRcpp );
@@ -333,14 +333,28 @@
// need to read another line
break;
case PARSE_NULL:
- Rf_error("%s: ParseStatus is null (%d)\n", programName, status);
- UNPROTECT(2);
- return 1;
+ if (interactiveMode) {
+ Rf_warning("%s: ParseStatus is null (%d)\n", programName, status);
+ UNPROTECT(2);
+ mb_m.rewind();
+ return 1;
+ } else {
+ Rf_error("%s: ParseStatus is null (%d)\n", programName, status);
+ UNPROTECT(2);
+ return 1;
+ }
break;
case PARSE_ERROR:
- Rf_error("Parse Error: \"%s\"\n", line.c_str());
- UNPROTECT(2);
- return 1;
+ if (interactiveMode) {
+ Rf_warning("Parse Error: \"%s\"\n", line.c_str());
+ UNPROTECT(2);
+ mb_m.rewind();
+ return 1;
+ } else {
+ Rf_error("Parse Error: \"%s\"\n", line.c_str());
+ UNPROTECT(2);
+ return 1;
+ }
break;
case PARSE_EOF:
Rf_error("%s: ParseStatus is eof (%d)\n", programName, status);
Index: RInside/src/RInside.cpp
===================================================================
--- RInside.orig/src/RInside.cpp 2012-10-11 00:30:43.228525248 +0300
+++ RInside/src/RInside.cpp 2012-10-11 00:32:01.676564524 +0300
@@ -333,14 +333,14 @@
// need to read another line
break;
case PARSE_NULL:
- Rf_error("%s: ParseStatus is null (%d)\n", programName, status);
UNPROTECT(2);
- return 1;
+ mb_m.rewind();
+ return 2;
break;
case PARSE_ERROR:
- Rf_error("Parse Error: \"%s\"\n", line.c_str());
UNPROTECT(2);
- return 1;
+ mb_m.rewind();
+ return 3;
break;
case PARSE_EOF:
Rf_error("%s: ParseStatus is eof (%d)\n", programName, status);
_______________________________________________
Rcpp-devel mailing list
[email protected]
https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
