-
-
Notifications
You must be signed in to change notification settings - Fork 219
Emit message if R.h included, can turn to warning #1411
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a compile-time check to detect when R headers (R.h, Rinterface.h, Rinternals.h, or Rdefines.h) are included before Rcpp headers, which can cause hard-to-debug errors. The check emits a pragma message directing users to issue #1410 for more information, and provides an escape hatch via the RCPP_NO_R_HEADERS_CHECK macro.
- New header file
check_r_headers.hthat detects pre-included R headers - Integration of the check into
RcppCommon.has the first include - Copyright year updates for 2025
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| inst/include/Rcpp/r/check_r_headers.h | New header file that checks for R headers included before Rcpp and emits pragma messages when detected |
| inst/include/RcppCommon.h | Includes the new check_r_headers.h file and updates copyright year to 2025 |
| ChangeLog | Documents the addition of the new header check feature and the RCPP_NO_R_HEADERS_CHECK escape hatch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Enchufa2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good robot. :) LGTM!
|
Yes, as uncool it is to admit it, I now turn them on all the time. It is especially good at the nitpicky stuff. |
|
Ok, having waited the polite 24 hours for veto or comment (hi @kevinushey we know you're busy so no worries) I will now fold this in. |
This addresses #1410 and adds a message if
R.h(or one of three other headers) is seen beforeRcpp.his included.Checklist
R CMD checkstill passes all tests