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

two (short) new entries for Rcpp-FAQ vignette #1009

Merged
merged 2 commits into from Nov 6, 2019

Conversation

@eddelbuettel
Copy link
Member

eddelbuettel commented Oct 28, 2019

Two simple additions which provide a start.

@kevinushey @jjallaire if you have comments / extensions maybe commit straight into branch?


## Can we use exceptions and stop() across shared libraries?

No, sadly. A known limitation coming from the operating system, Windows in

This comment has been minimized.

Copy link
@jjallaire

jjallaire Oct 28, 2019

Member

I think this might in it's present form be too cautionary. In all Rcpp attribute generated code (including the export of C++ interfaces to other packages) we handle this gracefully. The only place I'm aware of in Rcpp where this bites is specifically in Rcpp Module methods on Windows only. I think we might be better off just adding docs to modules saying you can't call stop() from within methods (they should always return something even if just an error object).

This comment has been minimized.

Copy link
@eddelbuettel

eddelbuettel Oct 28, 2019

Author Member

Hm, I thought we still bound by the general restriction of not being able to throw/catch across distinct DLLs? But maybe I am indeed to pessimistic in that view as it would blow up the use case of, eg, RQuantLib catching QuantLib errors.

Easy to rewrite as "Mostly. A known remaining limitation ..." I can take a stab at it later.

This comment has been minimized.

Copy link
@kevinushey

kevinushey Oct 28, 2019

Contributor

Cross-DLL exceptions work as long as:

  1. You are linking to a non-static C++ standard library;
  2. You are using the same standard library in all compiled objects.

Unfortunately, 1. is not true on Windows as R opts to link to libstdc++ statically; there was some discussion about whether this could be changed for Rtools 4.0 but it sounds like that won't happen.

In some cases, 2. is also not true -- e.g. on macOS users might end up mixing the version of libc++ bundled with R in some dylibs, and the system version of libc++ in others. Usually this means that the exception type info isn't properly handled across module boundaries.

This comment has been minimized.

Copy link
@kevinushey

kevinushey Oct 28, 2019

Contributor

(and of course, Rcpp attributes help us as, if one calls an attributes-generated function, any C++ exceptions are effectively shielded by try-catch and so are not propagated into a separate DLL)

This comment has been minimized.

Copy link
@eddelbuettel

eddelbuettel Nov 3, 2019

Author Member

@jjallaire @kevinushey Thanks for the very helpful comments.

I just rebased and force-pushed this branch adding one refining commit. The diff in the commit now shows an expanded and refined answer to this new FAQ entry based on your comments.

@jjallaire

This comment has been minimized.

Copy link
Member

jjallaire commented Oct 28, 2019

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

eddelbuettel commented Oct 28, 2019

Yes I meant that in general it just works (at least on not-on-Windows). I.e. at work we have a large-ish standard C++ codebase, exceptions are generally forbidden and it asserts and exits on error so for the calling-from-R-case I redefine these as Rcpp::stop() and it works just fine.

Just not sure about Windows which I have the benefit of mostly not seeing anymore ....

@eddelbuettel eddelbuettel force-pushed the feature/rcpp_faq_addition branch from 2e62d19 to 8a2c83d Nov 3, 2019
@eddelbuettel eddelbuettel force-pushed the feature/rcpp_faq_addition branch from 8a2c83d to 05a1d51 Nov 4, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 4, 2019

Codecov Report

Merging #1009 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1009   +/-   ##
=======================================
  Coverage   82.46%   82.46%           
=======================================
  Files          63       63           
  Lines        3166     3166           
=======================================
  Hits         2611     2611           
  Misses        555      555

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee0a5b8...05a1d51. Read the comment docs.

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

eddelbuettel commented Nov 4, 2019

@kevinushey @jjallaire if/when you have a chance glance over the current state of the new paragraphs and especially the updated / rewritten second one reflecting your really useful feedback.

Otherwise looks like we are good to go. May toss this at CRAN soon as 1.0.3.

@eddelbuettel

This comment has been minimized.

Copy link
Member Author

eddelbuettel commented Nov 6, 2019

Ok, I will take "no further comment" in the generous sense of "no further complaints" 😀

Merging now and going to prepare a 1.0.3 release.

@eddelbuettel eddelbuettel merged commit 3ea1730 into master Nov 6, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@eddelbuettel eddelbuettel deleted the feature/rcpp_faq_addition branch Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.