-
Notifications
You must be signed in to change notification settings - Fork 974
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
Issues with memory management in fmelt #6248
Conversation
Generated via commit ea67015 Download link for the artifact containing the test results: ↓ atime-results.zip Time taken to finish the standard R installation steps: 12 minutes and 10 seconds Time taken to run |
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.
great. I wrote this code and I guess forgot a PROTECT, thanks for looking at rchk results and providing a fix. add a NEWS item?
does this one fix handle all three rchk issues in fmelt.c?
I'm not used to reading / understanding these messages, are some redundant with each other? |
I'm also not sure, but yes that's my read. I'll add NEWS in a bit |
As identified by
rchk
on CRAN:https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/data.table.out
Note that the
cols_to_int_or_list()
output is indeedPROTECT()
'd properly in other nearby branches. This makes sense becausemeasure_int_or_list
is a newSEXP
not yet on the stack that must be marked against gc removal.