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

Possible Rcpp hotfix release to appease compiler nags w.r.t. deprecated pre-C++11 idioms #1201

Closed
eddelbuettel opened this issue Mar 9, 2022 · 6 comments

Comments

@eddelbuettel
Copy link
Member

Kurt Hornik emailed me because of an (unrelated) RcppArmadillo issue (involving a possible need for -latomic on Debian, that is still TBD) and noted that newer compilers nag a bit with respect to std::unary_function() and std::binary_function(). See for example in the results for RcppRedis which I looked at yesterday when it updated:

Result: WARN
    Found the following significant warnings:
     /home/hornik/tmp/R.check/r-devel-gcc/Work/build/Packages/Rcpp/include/Rcpp/Language.h:180:36: warning: ‘template<class _Arg, class _Result> struct std::unary_function’ is deprecated [-Wdeprecated-declarations]
     /home/hornik/tmp/R.check/r-devel-gcc/Work/build/Packages/Rcpp/include/Rcpp/Language.h:197:37: warning: ‘template<class _Arg1, class _Arg2, class _Result> struct std::binary_function’ is deprecated [-Wdeprecated-declarations]
     /home/hornik/tmp/R.check/r-devel-gcc/Work/build/Packages/Rcpp/include/Rcpp/StringTransformer.h:30:47: warning: ‘template<class _Arg, class _Result> struct std::unary_function’ is deprecated [-Wdeprecated-declarations]

When he emailed me earlier yesterday he only mentioned the thrid of these for StringTransformer and I quickly played with fix -- this is in fact simple enough thanks to std::function -- using std::function<bool(int)> instead of the deprecated std::unary_function<int, bool> is all it took in another example. I think replacing std::binary_function will be equally simple.

The question now is how to best implement the check as we need to keep the old code for compatibility's sake. Shall we just use

#if __cplusplus >= 201103L

to flip all C++11 or later compilation to use std::function()? Or are there any downsides I am not seeing?

(We will of course test fully at CRAN).

While we are at this we should probably also cherry-pick in #1193 which fixed a minor hoopla w.r.t. to C++98.

So these two changes, and we call it Rcpp 1.0.8-1 ? (And I will make sure to turn the unit test for three vs four version numbers off.)
Or do we have preference for four dots and call it 1.0.8.1 ?

@Enchufa2
Copy link
Member

Enchufa2 commented Mar 9, 2022

Can't we just drop the inheritance on unary|binary_function? Unless I'm missing something, there's no need if there's a call operator defined that is already unary/binary.

As for the version, my vote goes for four dots.

@eddelbuettel
Copy link
Member Author

Can't we just drop the inheritance on unary|binary_function?

I wondered about that too. And I don't feel I have a good handle on why it was needed in the past, but flipping to std::function seems closer.

As for the version, my vote goes for four dots.

The only problem with that is that we already use that form for 'interim test releases' i.e. the micro-snapshots between the releases. It's a bit murky to use the same form both ways. Part of me wants a clearer distinction.

@Enchufa2
Copy link
Member

Enchufa2 commented Mar 9, 2022

Can't we just drop the inheritance on unary|binary_function?

I wondered about that too. And I don't feel I have a good handle on why it was needed in the past, but flipping to std::function seems closer.

Most probably it was never needed. I would try a full revdep check without them, and I wouldn't expect any trouble. I mean, the definition of unary|binary_function is just a couple of typedefs, and we do not even use them!

I checked other projects just in case, and they just remove them (example).

As for the version, my vote goes for four dots.

The only problem with that is that we already use that form for 'interim test releases' i.e. the micro-snapshots between the releases. It's a bit murky to use the same form both ways. Part of me wants a clearer distinction.

Fine for me.

@eddelbuettel
Copy link
Member Author

I would try a full revdep check without them

I don't have time or energy for that. I will make the minimal change to std::function<> and test that. More extensive changes are welcomed with proper testing, I am not volunteering at this point.

Fine for me.

Checking shows we have really made an 1.0.8.1 since January so maybe the standard '.hotfix' form works.

@eddelbuettel
Copy link
Member Author

Or maybe we do it two step. Get a narrower fix out now, and if anyone (you?) feels motivated to dig deeper prior to 1.0.9 we do that. It looks like a fairly marginal feature either way so probably not the issue to go all in on (and I do want to get back to UNWIND_PROTECT).

@eddelbuettel
Copy link
Member Author

I pushed a somewhat minimal fix (to two header files) plus the needed updates to mark it as a new version 1.0.8.2 (because .1 is currently committed) and started a full reverse-depends check (using Debian testing, i.e. standard compiler). I will email Kurt and he can test it against the newer gcc/g++-12 (which I could activate too). This will likely be a PR "shortly".

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

No branches or pull requests

2 participants