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

Fix typo EXPORT -> EXPOSED #910

Merged
merged 2 commits into from Oct 7, 2018

Conversation

Projects
None yet
2 participants
@rstub
Copy link
Contributor

rstub commented Oct 7, 2018

No description provided.

@rstub

This comment has been minimized.

Copy link
Contributor Author

rstub commented Oct 7, 2018

Ooops, actually it is the other way round. I will update the PR.

@rstub rstub force-pushed the rstub:patch-1 branch from d6233c2 to 3c880fa Oct 7, 2018

@rstub rstub changed the title Fix typo EXPOSED -> EXPORT Fix typo EXPORT -> EXPOSED Oct 7, 2018

@rstub

This comment has been minimized.

Copy link
Contributor Author

rstub commented Oct 7, 2018

BTW, I also get some (pedantic) warnings:

expose_class.cpp:4:19: warning: extra ‘;’ [-Wpedantic]
 RCPP_EXPOSED_AS(A);
                   ^

Should I remove the semicolons in the documentation in this PR?

@eddelbuettel

This comment has been minimized.

Copy link
Member

eddelbuettel commented Oct 7, 2018

Thanks for the PR. We generally like to hear about things first in an issue ticket. So what is this fixing? Documentation issues only? Apart from the -pedantic ?

@rstub

This comment has been minimized.

Copy link
Contributor Author

rstub commented Oct 7, 2018

Only documentation issues: The docs use RCPP_EXPORT_AS and RCPP_EXPORT_WRAP where it should read RCPP_EXPOSED_AS and RCPP_EXPOSED_WRAP.

@eddelbuettel

This comment has been minimized.

Copy link
Member

eddelbuettel commented Oct 7, 2018

So do I have this right that

  1. two instances of RCPP_EXPORT_WRAP are being correcteded to RCPP_EXPOSED_WRAP in vignettes/Rcpp-extending.Rmd, and
  2. I do not see a file expose_class.cpp -- is that your running example?
@rstub

This comment has been minimized.

Copy link
Contributor Author

rstub commented Oct 7, 2018

i) yes, plus the same for RCPP_EXPORT_AS -> RCPP_EXPOSED_AS
ii) yes, that is my example code. Sorry for the confusion.

@eddelbuettel

This comment has been minimized.

Copy link
Member

eddelbuettel commented Oct 7, 2018

  1. Sounds good.
  2. But caused by our macro, or caused by you typing an extra ;?
@rstub

This comment has been minimized.

Copy link
Contributor Author

rstub commented Oct 7, 2018

  1. Caused by me copying the extra ; from the documentation. ;-) The examples for both RCPP_EXPOSED_AS and RCPP_EXPOSED_WRAP have this extra ;. And looking at the source for these macros in inst/include/Rcpp/macros/module.h, I think it is not possible to define these macros in a way that makes ; after it not superfluous. So my suggestion would be to remove the ; from the two relevant lines.
@eddelbuettel

This comment has been minimized.

Copy link
Member

eddelbuettel commented Oct 7, 2018

Yep! Can you add that change to the Rmd as well?

Thanks for catching this (in the process of yet another nice SO answer -- thanks for all those too!) !

@eddelbuettel eddelbuettel merged commit 1b4a29e into RcppCore:master Oct 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.