`R_ParseVector()` may jump

[romainfrancois]

When parsing fails, SEXP R_ParseVector(SEXP text, int n, ParseStatus *status, SEXP srcfile) is supposed to return R_NilValue and set status to PARSE_ERROR

 *	status = PARSE_NULL       - there was no statement to parse
 *		 PARSE_OK	  - complete statement
 *		 PARSE_INCOMPLETE - incomplete statement
 *		 PARSE_ERROR      - syntax error
 *		 PARSE_EOF	  - end of file

and when it returns an ParseStatus_PARSE_ERROR.

However, there are cases where R_ParseVector() actually signals an R condition, aka jumps, via R_Parse() > R_Parse1() > raiseParseError() :

static SEXP R_Parse1(ParseStatus *status)
{
    Status = 1; /* safety */
    switch(yyparse()) {
    [...]
	case 3:                     /* Valid expr '\n' terminated */
	case 4:                     /* Valid expr ';' terminated */
	    if (checkForPlaceholder(R_PlaceholderToken, R_CurrentExpr)) {
	        YYLTYPE lloc;
	        lloc.first_line = ParseState.xxlineno;
		lloc.first_column = ParseState.xxcolno;
	        if (Status == 3) lloc.first_line--;
		raiseParseError("invalidPlaceholder", R_CurrentExpr,
		                NO_VALUE, NULL, &lloc,
			  _("invalid use of pipe placeholder (%s:%d:%d)"));
            }
	    if (checkForPipeBind(R_CurrentExpr)) {
	        YYLTYPE lloc;
	        lloc.first_line = ParseState.xxlineno;
		lloc.first_column = ParseState.xxcolno;
	        if (Status == 3) lloc.first_line--;
		raiseParseError("invalidPipeBind", R_CurrentExpr, 
		                NO_VALUE, NULL, &lloc,
			  _("invalid use of pipe bind symbol (%s:%d:%d)"));
	    }
	    *status = PARSE_OK;
	    break;
	}
	break;
    case 1:                     /* Syntax error / incomplete */
	*status = PARSE_ERROR;
	if (EndOfFile) *status = PARSE_INCOMPLETE;
	break;
    case 2:
	error(_("out of memory while parsing"));
    }
    return R_CurrentExpr;
}

I don't know if that means we need to use R_UnwindProtect() around R_ParseVector() or perhaps use parse(text =) in R code and then catch the condition with R means.

For example, if I add this test to exec.rs in harp :

    #[test]
    fn test_parse(){ r_test! {
        let mut ps : ParseStatus = 0;
        let code = RObject::from("x + _");
        R_ParseVector(*code, -1, &mut ps, R_NilValue);

        assert_eq!(ps, ParseStatus_PARSE_ERROR);
    }}

it's not happy:

[romainfrancois]

Enregistrement.de.l.ecran.2023-01-20.a.15.25.03.mov

[jmcphers]

Notes from Slack conversation -- we should be able to use R_UnwindProtect here since we are willing to have a minimum R version in line with Tidyverse's (~3.6)

[kevinushey]

It would be nice if we had a "friendly" interface to R_UnwindProtect(), e.g.

r_unwind_protect! {
  // some R code that might longjmp
}

On the RStudio side, we typically used R_ToplevelExec() in these sorts of situations.

https://github.com/rstudio/rstudio/blob/2088e487f2c590900d0ad4dcc5e0d61b3cda7ff8/src/cpp/r/RExec.cpp#L266-L284

[romainfrancois]

It feels like R_UnwindProtect() is designed for the case of R code > .Call() > C/C++/(rust?) > R api that may jump, so that r api is unwound then the c/c++/rust is unwound.

But from the docs and how cpp11 uses it, it looks like then you have to continue unwinding.

[kevinushey]

You're right. In that context, toplevelexec might be the right choice.

[kevinushey]

I believe this was closed via #112 -- is that correct @romainfrancois? If so please feel free to close out this issue.

[romainfrancois]

Almost, I just need to make sure that r_parse_vector() is used instead of directly R_ParseVector()