Skip to content
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.

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

Noisy installations of Rcpp 1.0.1 due to changes in RcppExports.cpp #951

Closed
eddelbuettel opened this issue Mar 17, 2019 · 6 comments

Comments

@eddelbuettel
Copy link
Member

commented Mar 17, 2019

[...]
/usr/local/lib/R-devel/lib/R/library/Rcpp/include/Rcpp/macros/macros.h:36:17: warning: unused variable ‘stop_sym’ [-Wunused-variable]
     static SEXP stop_sym = Rf_install("stop");                                                   \
                 ^~~~~~~~
/usr/local/lib/R-devel/lib/R/library/Rcpp/include/Rcpp/macros/macros.h:36:17: note: in definition of macro ‘BEGIN_RCPP’
     static SEXP stop_sym = Rf_install("stop");                                                   \
                 ^~~~~~~~
RcppExports.cpp: In function ‘SEXPREC* _RQuantLib_europeanOptionArraysEngine_try(SEXP, SEXP)’:
/usr/local/lib/R-devel/lib/R/library/Rcpp/include/Rcpp/macros/macros.h:32:9: warning: unused variable ‘nprot’ [-Wunused-variable]
     int nprot = 0;                                                                               \
         ^~~~~
/usr/local/lib/R-devel/lib/R/library/Rcpp/include/Rcpp/macros/macros.h:32:9: note: in definition of macro ‘BEGIN_RCPP’
     int nprot = 0;                                                                               \
         ^~~~~
/usr/local/lib/R-devel/lib/R/library/Rcpp/include/Rcpp/macros/macros.h:36:17: warning: unused variable ‘stop_sym’ [-Wunused-variable]
     static SEXP stop_sym = Rf_install("stop");                                                   \
                 ^~~~~~~~
/usr/local/lib/R-devel/lib/R/library/Rcpp/include/Rcpp/macros/macros.h:36:17: note: in definition of macro ‘BEGIN_RCPP’
     static SEXP stop_sym = Rf_install("stop");                                                   \
                 ^~~~~~~~

@romainfrancois @lionel- @kevinushey Any better idea than inserting a fake check or use?
Seems silly to waste cycles on this. Can we add a sentinel function just to fake the compiler?

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2019

I think I have a fix. The 'other' macro END_RCPP_RETURN_ERROR needs both symbols added.

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2019

modified   inst/include/Rcpp/macros/macros.h
@@ -98,7 +98,10 @@
   catch (...) {                                                                \
     return string_to_try_error("c++ exception (unknown reason)");              \
   }                                                                            \
-  return R_NilValue;
+  UNPROTECT(nprot);                                                            \
+  return R_NilValue;                                                           \
+  if (stop_sym == R_NilValue) /* never reached but */                          \
+    return R_NilValue;        /* suppresses warning*/
 #endif
 
 #define Rcpp_error(MESSAGE) throw Rcpp::exception(MESSAGE, __FILE__, __LINE__)
@kevinushey

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

I'll take a look as well.

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

commented Mar 18, 2019

That actually takes care of things and I'll PR it later once I get home...

@kevinushey

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

Sounds good (can confirm your fix looks good as well)

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

commented Mar 18, 2019

@kevinushey Now committed and put into PR #953

eddelbuettel added a commit that referenced this issue Mar 19, 2019

Merge pull request #953 from RcppCore/bugfix/rcpp_end_nprot_stop
use nprot and reference stop_sym (closes #951)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.