-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
Warning from -Wdelete-non-virtual-dtor when using a .finalizer function #1305
Comments
Good question. Modules has not seen a lot of love those last few years. If your repo with added finalizer public? Not sure we have (m)any other examples of finalizers in use (though I have not looked yet). |
I think you're right; the class has virtual methods but no virtual destructor: Rcpp/inst/include/Rcpp/Module.h Lines 299 to 304 in 03fd09a
We should probably add it, or just simplify and remove the virtual inheritance used here (since FunctionFinalizer is the only thing that inherits from CppFinalizer) |
What do you think about e.g. rule of four adding the missing ones as defaults? (I am spitballing here...) |
Thanks for the quick response.
It's currently in a branch of my fork here, but will be merged sometime soon. I have a couple of other exposed classes that I would add finalizers for, but didn't have it working in the past so spending a little more time to check it out now.
FWIW, I really like modules and |
Do you want to give modifying the set of defaults and see where it leads you? |
update to the above, the class with finalizer has been merged in main: |
That's actually a pretty nice example for modules use! |
It's a nice API to be able to wrap and use from R. Class And as side note, |
Sorry, I didn't catch that before. My rationale, originally, was that a custom finalizer would run upon gc() but the class destructor may be less predictable. If the above is correct, and the destructor does the necessary clean-up, then a custom finalizer may not be needed. Makes sense. It seems to work fine either way for class VSIFile, and the destructor is called during garbage collection when not using .finalizer. I'll omit the custom finalizer in this case. |
Hey @ctoney just circling back and as you (per CRANberries) have a new version on CRAN: are you all good here? What if any action items do we have left? |
Intended to circle back and let you know it is on CRAN but got distracted by a couple of things. My issue is resolved, but by not using the custom finalizer. The object's destructor is called at gc(), since Rcpp Modules registers a default finalizer as noted above (but I missed that previously). That meets my need and the cleanup code is very simple. The compiler warning appears though, if a custom .finalizer is defined on the exposed class, in case that's of interest. Thanks again. |
Using modules with an
RCPP_EXPOSED_CLASS
that has a.finalizer
function, a compiler warning is emitted:Is this just because
Rcpp::CppFinalizer
does not have virtual destructor? Or more likely involves something on my end?Thanks.
g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
The text was updated successfully, but these errors were encountered: