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

Added find() & get() to environment (closes #326) #513

Merged
merged 1 commit into from
Jul 22, 2016
Merged

Added find() & get() to environment (closes #326) #513

merged 1 commit into from
Jul 22, 2016

Conversation

coatless
Copy link
Contributor

Modified load order of Rcpp.h to allow for symbol object to be used within Environment.h by placing Symbol.h before Environment.h

Used the .c_str() on symbol to ensure exception call could be made. (Assuming the symbol name is not NULL).

Fixed a space in changelog

@coatless
Copy link
Contributor Author

coatless commented Jul 22, 2016

The Travis-CI check being green is a bit misleading as the travis-log contains:

* checking foreign function calls ... NOTE
Registration problem:
  symbol ‘symbol’ in the local frame:
   .Call(symbol)
See chapter ‘System and foreign language interfaces’ in the ‘Writing R
Extensions’ manual.

My local check on Rcpp does not have this error listed.

* checking foreign function calls ... OK

@eddelbuettel
Copy link
Member

We have been getting that warning for a long time; it is grandfathered as "technically correct" (we do make that call, but it is Rcpp internals and needed for some functionality). I guess it depends if you call with --as-cran and/or which checking variables you have set. We can discuss off-line, or I just email you my ~/.R/check.environ.

...
* checking replacement functions ... OK
* checking foreign function calls ... NOTE
Registration problem:
  symbol ‘symbol’ in the local frame:
   .Call(symbol)
See chapter ‘System and foreign language interfaces’ in the ‘Writing R Extensions’ manual.
* checking R code for possible problems ... OK
* checking Rd files ... OK
...

@eddelbuettel
Copy link
Member

Looks like a nice PR. As it re-orders headers I'll be prudent and run a rev.dep check just in case.

@kevinushey
Copy link
Contributor

LGTM as well, modulo the check to ensure re-ordered headers aren't an issue.

@eddelbuettel
Copy link
Member

Looks like it passed the reverse depends with flying colors.

@coatless
Copy link
Contributor Author

The Registration problem seems to be triggered by:

Rcpp/R/Module.R

Line 199 in 0dd0fb6

xp <- .Call( symbol )

I think there is a way to actually suppress / remove the warning.

@eddelbuettel eddelbuettel merged commit 75c3f6d into RcppCore:master Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants